-
Notifications
You must be signed in to change notification settings - Fork 2.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow getting the share list filtered by share type via API #38000
Conversation
Missing items:
Code has been checked via web UI and partially via API |
Could also add a couple of API acceptance tests. There is a step: That could have more specific steps to get just the user, group or public link shares. The scenario(s) could create a few shares of different types, then do a "get" filtering by share type. |
@jvillafanez Let us include this in 10.6 |
@jvillafanez there are some JavaScript tests that failed: https://drone.owncloud.com/owncloud/core/27205/8/6 |
Failing test fixed, and new unit tests added. This should be ready now. |
f748fcd
to
6d873a9
Compare
When I query as
The filtering did not happen. But when I query as
Is this filtering-by-share-type supposed to work for |
b3a51c4
to
c7fc31c
Compare
ToDo:
|
How will clients know that the server is capable of providing a filtered list of shares by share type? |
I'd vote for API docs.
I think those options are the ones that require the least changes both in the client and the server. |
Doc ticket opened in owncloud/docs#2825 |
0eb4eb2
to
b319f0b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
API acceptance tests are done for the interesting combinations - looks good.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally, code looks okay. All the changes are in controller and all tests are passed. So, affected parts should be limited.
I just only suggested a small change that can simplify some foreach blocks. @jvillafanez I would like to hear your thoughts about that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Syntax error needs fixing - @jvillafanez
bfa1836
to
95fc0aa
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Kudos, SonarCloud Quality Gate passed!
|
Description
Allow filtering by share type while getting the shares. This should speed up a bit the web UI because the server will need to fetch and send less data.
Changes are backwards-compatible: if no filter is sent, all the shares will be returned, same as before.
Related Issue
https://github.com/owncloud/enterprise/issues/4107
Motivation and Context
The "share by link" view is expected to perform faster since there are less requests done to the DB to fetch data. Furthermore, the view doesn't need to ask for remote shares, which could slow down things if there are problems with the remote server.
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Checklist: