Hotfix/sharing: some more bugs with sharing and mirror file creation #161
Conversation
… this should be a working happy path.
| @@ -226,11 +226,12 @@ func (s *Space) OpenSharedFile(ctx context.Context, hash, password, filename str | |||
| }, nil | |||
| } | |||
|
|
|||
| func (s *Space) ShareFilesViaPublicKey(ctx context.Context, paths []domain.FullPath, pubkeys []crypto.PubKey) error { | |||
| func (s *Space) ShareFilesViaPublicKey(ctx context.Context, paths *[]domain.FullPath, pubkeys []crypto.PubKey) error { | |||
There was a problem hiding this comment.
Maybe add a comment on this function that paths would be modified. Personally, I think it would have been better if the function returned the modified paths instead of mutating the input value, but eitherways a comment should help make it more apparent.
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| if mf != nil { | ||
| return nil, errMirrorFileAlreadyExists |
There was a problem hiding this comment.
I wonder: should we return err or just return mf, nil?
There was a problem hiding this comment.
Will leave this as-is for now and can update later since it's working at least. If we need a create or get type usage if this later we can always update.
| } | ||
|
|
||
| if path.Bucket == "" || path.Bucket == t.GetDefaultBucketSlug() { | ||
| if ep.Bucket == "" || ep.Bucket == t.GetDefaultBucketSlug() { |
There was a problem hiding this comment.
Do shared with me files have ep.Bucket == ""? Because if that's the case it might unintendedly fall into this block for shared-with-me files
There was a problem hiding this comment.
So shared with me files will always have a bucket name. But the weird part is that the mirror bucket name is also called personal so it would erroneous fall into the second clause. I made both clauses so that it would default to the personal bucket if it was empty or if personal was supplied explicitly. For now I'm thinking to keep it clean we can change the name of the mirrored buckets to something like shared so it doesn't fall into this.
There was a problem hiding this comment.
@maurycy do you see any issues with this idea? Like if I used personal_mirror for the mirror bucket names. As long as we are not referring the mirror bucket name hardcoded as personal elsewhere that should be fine.
There was a problem hiding this comment.
Not sure if I understand. Do you mean adding a prefix, eg: shared_, personal_?
There was a problem hiding this comment.
Fixed in 1742bb0. Mirror buckets now use a different name so it wouldn't fall into this slot unless it was actually a personal bucket (or empty in which case it is personal).
Change Log
targetBucketPathinstead oftargetPathso we don't pass in emtpy string for root filespathsin as a pointer soShareFilesViaPublicKeycan modify it for enhancing dataFindMirrorFileByPathAndBucketSlugdon't return empty struct so nil could be checked for when trying to create