EnterpriseyIntranet / nextcloud-API

NextCloud OCS API for Python
GNU General Public License v3.0
27 stars 27 forks source link

Group folders filter by mount point #40

Closed danil-topchiy closed 5 years ago

danil-topchiy commented 5 years ago

Add filter by mount point to get_group_folders method

danil-topchiy commented 5 years ago

Works for json format only and disabled it for xml for now as we don't parse it

codecov-io commented 5 years ago

Codecov Report

Merging #40 into master will increase coverage by 0.06%. The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #40      +/-   ##
==========================================
+ Coverage   92.91%   92.97%   +0.06%     
==========================================
  Files          17       17              
  Lines         536      541       +5     
==========================================
+ Hits          498      503       +5     
  Misses         38       38
Impacted Files Coverage Δ
src/nextcloud/api_wrappers/group_folders.py 100% <100%> (ø) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 762a2dd...5c73255. Read the comment docs.

scrutinizer-notifier commented 5 years ago

The inspection completed: 1 new issues, 1 updated code elements

matejak commented 5 years ago

Please make this a standalone function or method. The filtering functionality makes the original method more complicated (including the method's interface), and it basically just filters the original result after it is retrieved, i.e. it is not like that knowing the filter means that less data have to be downloaded. As other filtering methods may become available or desired in the future, the interface should be designed in a way that it doesn't break and it stays as lean as possible.

danil-topchiy commented 5 years ago

@matejak you right. Now I'm watching at this and think if we really need it as a standalone method or it's better to leave such filtering for client?

matejak commented 5 years ago

I agree, I am not against having another layer of convenience functionality in the module, but it has to be extensible without breaking the backward compatibility. In this context, I am closing the PR, we can pick it up later if the need for extensions appear.