feat: Added an ability to share by a link a file inside a shared drive#4681
feat: Added an ability to share by a link a file inside a shared drive#4681
Conversation
shepilov
force-pushed
the
feat/shared_drive_share_by_link
branch
from
d964e8e to
835811d
Compare
shepilov
force-pushed
the
feat/shared_drive_share_by_link
branch
from
835811d to
5fcd5bf
Compare
| type APIPermission struct { | ||
| *permission.Permission | ||
| included []jsonapi.Object | ||
| PublicLink string `json:"public_link,omitempty"` |
There was a problem hiding this comment.
Oops, it was initial idea to custruct URL for the shared resource on backend
| // getSharingDir returns the directory linked to the drive sharing. | ||
| func getSharingDir(inst *instance.Instance, s *sharing.Sharing) (*vfs.DirDoc, error) { | ||
| dir, err := s.GetSharingDir(inst) | ||
| if err != nil { | ||
| return nil, jsonapi.NotFound(errors.New("shared drive not found")) | ||
| } | ||
| return dir, nil | ||
| } |
There was a problem hiding this comment.
[nit] This function is only used by ChangesFeed and doesn't really do much more than calling s.GetSharingDir(inst) directly in ChangedFeed would.
There was a problem hiding this comment.
yes, I think I was going to use the validateSharedDrivePermission function and then forgot about it
| } | ||
|
|
||
| // CreateSharedDrivePermissionHandler creates a share-by-link permission for a file in a shared drive. | ||
| func CreateSharedDrivePermissionHandler(c echo.Context, inst *instance.Instance, s *sharing.Sharing) error { |
There was a problem hiding this comment.
[nit] I find the name of this function a bit confusing.
I think specifying the type of permission would be useful (e.g. CreateSharedDriveLinkPermissionHandler).
|
|
||
| // HandleCreateShareByLink is the common handler for creating share-by-link permissions. | ||
| // It handles parsing, optional additional validation, creation via CreateShareSet, and response formatting. | ||
| func HandleCreateShareByLink(c echo.Context, inst *instance.Instance, extraValidate ValidatePermissionsFn, skipValidation bool) error { |
There was a problem hiding this comment.
This name is a lot clearer! 👍
| var subdoc permission.Permission | ||
| if _, err := jsonapi.Bind(c.Request().Body, &subdoc); err != nil { | ||
| return err | ||
| } | ||
|
|
||
| // Run additional validation if provided (e.g., for shared drives) | ||
| if extraValidate != nil { | ||
| if err := extraValidate(subdoc.Permissions); err != nil { | ||
| return err | ||
| } | ||
| } |
There was a problem hiding this comment.
We usually return bad request errors only if there are no permission errors. I would expect to see those after the call to middlewares.GetPermission(c).
There was a problem hiding this comment.
It's just a code from the old createPermission function. I thin if we need to change the API, we need to check how it's used by the client
There was a problem hiding this comment.
You changed the order in which the code is called though.
| } | ||
|
|
||
| // RevokeSharedDrivePermission revokes a share-by-link permission for a file in a shared drive. | ||
| func RevokeSharedDrivePermission(c echo.Context, inst *instance.Instance, s *sharing.Sharing) error { |
There was a problem hiding this comment.
[nit] Same thing here: adding the type of permission to the function name would be helpful.
2 participants
Summary
Add share-by-link functionality for shared drives, allowing both owners and members to create and revoke public links for files within a shared drive.