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

Make it possible to filter by tags with REPORT method #26099

Merged
merged 1 commit into from Sep 20, 2016

Conversation

Projects
None yet
4 participants
@PVince81
Member

PVince81 commented Sep 12, 2016

Description

Enhanced the REPORT method on the Webdav endpoint and added a
"oc:favorite" filter rule. When set, it will return a flat list of
results filtered with only favorite files.

The web UI was also adjusted to use this REPORT method instead of the
private API endpoint.

Related Issue

Fixes #23263

Motivation and Context

This makes it possible for other clients like desktop and mobile to retrieve a flat filtered list of favorite files. The previous API was private and was not supposed to be used, but the REPORT method and the webdav endpoint are public. @davivel @nasli @dragotin @guruz

How Has This Been Tested?

  1. Favorite a few files in folders and subfolders, share one of them with link (for the share indicator).
  2. Go to the "Favorites" section in the web UI.
  3. Check the network console and see that it's using REPORT to get the results.
  4. Check that the displayed results look fine and also contain the share indicator from the previous link share (it's part of the Webdav response)

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

Please review @DeepDiver1975 @butonic @VicDeo @guruz @davivel @SergioBertolinSG @nasli

@PVince81 PVince81 added this to the 9.2 milestone Sep 12, 2016

@mention-bot

This comment has been minimized.

Show comment
Hide comment
@mention-bot

mention-bot Sep 12, 2016

@PVince81, thanks for your PR! By analyzing the annotation information on this pull request, we identified @LukasReschke, @nickvergessen and @DeepDiver1975 to be potential reviewers

@PVince81, thanks for your PR! By analyzing the annotation information on this pull request, we identified @LukasReschke, @nickvergessen and @DeepDiver1975 to be potential reviewers

@PVince81

This comment has been minimized.

Show comment
Hide comment
@PVince81

PVince81 Sep 12, 2016

Member

I just saw that we can actually have different report types using the report name.

The report here was called "filter-files". The question maybe is whether we want to separate all these into separate reports:

  • filter-systemtags
  • filter-favorites
  • get-files (for pagination)
  • search-files ?
Member

PVince81 commented Sep 12, 2016

I just saw that we can actually have different report types using the report name.

The report here was called "filter-files". The question maybe is whether we want to separate all these into separate reports:

  • filter-systemtags
  • filter-favorites
  • get-files (for pagination)
  • search-files ?
@PVince81

This comment has been minimized.

Show comment
Hide comment
@PVince81

PVince81 Sep 12, 2016

Member

The advantage of a single "filter-files" report is that you can combine filters and intersect results, but it also increases complexity because of this.

Member

PVince81 commented Sep 12, 2016

The advantage of a single "filter-files" report is that you can combine filters and intersect results, but it also increases complexity because of this.

@VicDeo

This comment has been minimized.

Show comment
Hide comment
@VicDeo

VicDeo Sep 14, 2016

Member

@PVince81
is it ok, that a response has several dummy parts like that in addition to the file list itself?

  <d:propstat>
   <d:prop>
    <oc:size/>
   </d:prop>
   <d:status>HTTP/1.1 404 Not Found</d:status>
  </d:propstat>
 </d:response>

If yes, consider my comment for 👍

Member

VicDeo commented Sep 14, 2016

@PVince81
is it ok, that a response has several dummy parts like that in addition to the file list itself?

  <d:propstat>
   <d:prop>
    <oc:size/>
   </d:prop>
   <d:status>HTTP/1.1 404 Not Found</d:status>
  </d:propstat>
 </d:response>

If yes, consider my comment for 👍

@PVince81

This comment has been minimized.

Show comment
Hide comment
@PVince81

PVince81 Sep 14, 2016

Member

@VicDeo yes, this happens when requesting a property that doesn't exist on all nodes.
The "oc:size" property only exists on folders, not files. So the file entries will tell so in the 404 block.
This is a standard PROPFIND response.

Member

PVince81 commented Sep 14, 2016

@VicDeo yes, this happens when requesting a property that doesn't exist on all nodes.
The "oc:size" property only exists on folders, not files. So the file entries will tell so in the 404 block.
This is a standard PROPFIND response.

@PVince81

This comment has been minimized.

Show comment
Hide comment
@PVince81

PVince81 Sep 16, 2016

Member

@DeepDiver1975 @butonic do we agree on the API-side of this ? Basically use multiple filters within the same report "filter-files".

Member

PVince81 commented Sep 16, 2016

@DeepDiver1975 @butonic do we agree on the API-side of this ? Basically use multiple filters within the same report "filter-files".

Make it possible to filter by tags with REPORT method
Enhanced the REPORT method on the Webdav endpoint and added a
"oc:favorite" filter rule. When set, it will return a flat list of
results filtered with only favorite files.

The web UI was also adjusted to use this REPORT method instead of the
private API endpoint.
@butonic

This comment has been minimized.

Show comment
Hide comment
@butonic

butonic Sep 20, 2016

Member

basically the report tesult is a list of files. What we want to change are the filter criteria, eg. by favorite, tag, mtime range, file name, file content ...

so yes, keep a single filter-files report.

Member

butonic commented Sep 20, 2016

basically the report tesult is a list of files. What we want to change are the filter criteria, eg. by favorite, tag, mtime range, file name, file content ...

so yes, keep a single filter-files report.

@PVince81

This comment has been minimized.

Show comment
Hide comment
@PVince81

PVince81 Sep 20, 2016

Member

Thanks, merging.

Pagination as it was mentionned is a topic for another time, see #13915.

Member

PVince81 commented Sep 20, 2016

Thanks, merging.

Pagination as it was mentionned is a topic for another time, see #13915.

@PVince81 PVince81 merged commit 812bae5 into master Sep 20, 2016

4 of 5 checks passed

VersionEye Some dependencies have no license.
Details
Scrutinizer 4 new issues, 2 updated code elements
Details
continuous-integration/jenkins/pr-head This commit looks good
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
licence/cla Contributor License Agreement is signed.
Details

@PVince81 PVince81 deleted the fav-report branch Sep 20, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment