share api expanded by tags #26583
share api expanded by tags #26583
Conversation
|
@mjobst-necls, thanks for your PR! By analyzing the history of the files in this pull request, we identified @PVince81, @rullzer and @LukasReschke to be potential reviewers. |
|
The Tags::getTagsForObjects() function is not very performant, because a query will be executed for each object id. We could modify this function in that way, that just one select with |
| @@ -183,7 +184,8 @@ | |||
| url: OC.linkToOCS('apps/files_sharing/api/v1') + 'remote_shares', | |||
| /* jshint camelcase: false */ | |||
| data: { | |||
| format: 'json' | |||
| format: 'json', | |||
| show_tags: true | |||
There was a problem hiding this comment.
call it include_tags because the API doesn't really show anything but just returns a result. What the API consumer does with it is up to them. Also adjust the variable names everywhere accordingly 😄
I think it's only used by the file picker dialog now (try for example to select an avatar with it), it doesn't require any tags for that.
file picker
It used to, yes. There was a time where the whole files app was using ajax/*.php calls including that list.php. We changed it to use the Webdav API since OC 9.0 because why have duplicate endpoints.
It is already possible by marking the mount point of the remote share as favorite, which itself also has a fileid.
Yes, sounds good.
Yes, file_source points to the fileid from the oc_filecache.
Hmm I thought that one was already optimized. If you have ideas to make it better, let's do it in a separate PR then. Thanks a lot, I really appreciate the effort 😄 |
|
Ok. This should work now. There still exist a few little bugs (Expiration date/Shared With disappear, Size is not shown correctly on details view), but they will be fixed by #26552 after some modifications. |
Added missing function description
Added missing function description
implicit boolean conversion to !empty()
|
Ok, it seems that jenkins error is not caused by this PR. |
|
Result:
|
|
Fantastic work ! Code looks great and also works. 👍 Let me kick Jenkins again... somehow that random failure always appear from times to times (#25830) |
|
Really nice! 👍 |
* share api expanded by tags * Modified files_sharing JS Unit tests * modified tests. renamed request parameter. refactoring * Update Share20OCS.php Added missing function description * Update Helper.php Added missing function description * Update Helper.php implicit boolean conversion to !empty() * Update Share20OCSTest.php
|
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Description
Modification of the share api. The share api adds the file tags to the response if "show_tags" is existing and has a positive value in the request.
Attention: This Issue is not solved yet by this first commits. Additional information is needed. I'm also not yet confident with the naming conventions at all.
Is the request parameter name "show_tags" ok?
Check the modified populateTags() function. That function was only used in the /apps/files/ajax/list.php
2a) Is that list.php still needed?
2b) How can this file be tested?
2c) The populateTags() function returned the same value as passed by as parameter. Did this function really do its job correct?
Should it be/Is it possible to mark remote shares as favorite?
The favorite indicator should be shown in the share views. The tags can now be retrieved by the share api. Do you think my approach regarding adding the entries to the allowedList (see Cannot set a file as favorite from the star icon in the sidebar in tags menu #26437) would be ok by now
Is 'file_source' the correct field to identify the object in shares?
Additional Tests have to be added. I will do this when all questions have been answered.
Related Issue
#19753
#19753 should be done before #26437
Motivation and Context
The share views should be enhanced by the favorite indicator and actions.
How Has This Been Tested?
By simple api calls.
Screenshots (if appropriate):
Types of changes
Checklist: