Skip to content

Conversation

@LukasReschke
Copy link
Member

This adds a "backend" type filter to the index REST route which is a pre-requisite for #12620

For example when calling index.php/settings/users/users?offset=0&limit=10&gid=&pattern=&backend=OC_User_Database only users within the backend OC_User_Database would be shown. (requires sending a CSRF token as well)

@LukasReschke
Copy link
Member Author

@MorrisJobke

@LukasReschke LukasReschke mentioned this pull request Dec 9, 2014
23 tasks
@LukasReschke
Copy link
Member Author

Mentioning @nickvergessen as he always likes to review stuff.

@LukasReschke LukasReschke force-pushed the add-filter-for-backend-to-rest-index branch 2 times, most recently from b620659 to aa6c7c6 Compare December 10, 2014 11:02
@LukasReschke
Copy link
Member Author

@nickvergessen Rebased on master and added unit-test as suggested for the $backend parameter.

@LukasReschke LukasReschke force-pushed the add-filter-for-backend-to-rest-index branch from aa6c7c6 to d3d2ca3 Compare December 10, 2014 11:05
This adds a "backend" type filter to the index REST route which is a pre-requisite for #12620

For example when calling `index.php/settings/users/users?offset=0&limit=10&gid=&pattern=&backend=OC_User_Database` only users within the backend `OC_User_Database` would be shown. (requires sending a CSRF token as well)

Depends upon #12711
@LukasReschke LukasReschke force-pushed the add-filter-for-backend-to-rest-index branch from d3d2ca3 to 5dc6406 Compare December 10, 2014 11:07
@LukasReschke LukasReschke mentioned this pull request Dec 10, 2014
@LukasReschke
Copy link
Member Author

CI is at #12755

@LukasReschke
Copy link
Member Author

From #12755 (comment):

🚀 Test PASSed. 🚀
Refer to this link for build results (access rights to CI server needed):
https://ci.owncloud.org//job/pull-request-analyser-ng-simple/4213/
🚀 Test PASSed. 🚀


@MorrisJobke @nickvergessen Please review.

@MorrisJobke MorrisJobke added this to the 8.0-current milestone Dec 10, 2014
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not \OCP\UserInterface (and change it in every backend;) I know you will love it )?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed the PHPDoc accordingly. Will update specific interfaces later as modifications on them are anyways required.

@ghost
Copy link

ghost commented Dec 11, 2014

Refer to this link for build results (access rights to CI server needed):
https://ci.owncloud.org//job/pull-request-analyser-ng-simple/4286/

Build result: FAILURE

[...truncated 16 lines...]Fetching upstream changes from https://github.com/owncloud/core.gitusing GIT_SSH to set credentials using .gitcredentials to set credentials > git config --local credential.helper store --file=/tmp/git791989861574526983.credentials # timeout=10 > git fetch --tags --progress https://github.com/owncloud/core.git +refs/pull/:refs/remotes/origin/pr/ > git config --local --remove-section credential # timeout=10 > git rev-parse origin/pr/12726/merge^{commit} # timeout=10 > git branch -a --contains 16c9a2c22869e9424913f02b648a4ea668d6779f # timeout=10 > git rev-parse remotes/origin/pr/12726/merge^{commit} # timeout=10Checking out Revision 16c9a2c22869e9424913f02b648a4ea668d6779f (origin/pr/12726/merge) > git config core.sparsecheckout # timeout=10 > git checkout -f 16c9a2c22869e9424913f02b648a4ea668d6779fFirst time build. Skipping changelog. > git remote # timeout=10 > git submodule init # timeout=10 > git submodule sync # timeout=10 > git config --get remote.origin.url # timeout=10 > git submodule update --init --recursiveTriggering pull-request-analyser-ng-simple » SLAVEpull-request-analyser-ng-simple » SLAVE completed with result FAILUREStarted calculate disk usage of buildFinished Calculation of disk usage of build in 0 secondsStarted calculate disk usage of workspaceFinished Calculation of disk usage of workspace in 34 second
💣 Test FAILed. 💣

@MorrisJobke
Copy link
Contributor

👍

@LukasReschke
Copy link
Member Author

@owncloud-bot Retest this please.

@LukasReschke
Copy link
Member Author

@nickvergessen Care to review? :-)

@ghost
Copy link

ghost commented Dec 12, 2014

Refer to this link for build results (access rights to CI server needed):
https://ci.owncloud.org//job/pull-request-analyser-ng-simple/4380/

Build result: FAILURE

[...truncated 16 lines...]Fetching upstream changes from https://github.com/owncloud/core.gitusing GIT_SSH to set credentials using .gitcredentials to set credentials > git config --local credential.helper store --file=/tmp/git8165423963444798092.credentials # timeout=10 > git fetch --tags --progress https://github.com/owncloud/core.git +refs/pull/:refs/remotes/origin/pr/ > git config --local --remove-section credential # timeout=10 > git rev-parse origin/pr/12726/merge^{commit} # timeout=10 > git branch -a --contains 7dbaa9612384e38158a6e2454f254fdde11c626b # timeout=10 > git rev-parse remotes/origin/pr/12726/merge^{commit} # timeout=10Checking out Revision 7dbaa9612384e38158a6e2454f254fdde11c626b (origin/pr/12726/merge) > git config core.sparsecheckout # timeout=10 > git checkout -f 7dbaa9612384e38158a6e2454f254fdde11c626bFirst time build. Skipping changelog. > git remote # timeout=10 > git submodule init # timeout=10 > git submodule sync # timeout=10 > git config --get remote.origin.url # timeout=10 > git submodule update --init --recursiveTriggering pull-request-analyser-ng-simple » SLAVEpull-request-analyser-ng-simple » SLAVE completed with result FAILUREStarted calculate disk usage of buildFinished Calculation of disk usage of build in 0 secondsStarted calculate disk usage of workspaceFinished Calculation of disk usage of workspace in 27 second
💣 Test FAILed. 💣

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please also add a test for a backend that does not have users

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@nickvergessen
Copy link
Contributor

Looks good, but I'd like to have a second test which then receives an empty result

@MorrisJobke
Copy link
Contributor

Test looks good 👍

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comparison on classes instead of names ? Is that safe / faster than string comparison ? (very minor)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Honestly: No idea :-)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code comment would be appreciated. This is confusing but I'm pretty sure you understood it while touching it 😉

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might need to make it clear that the input array is in form $uid => $displayName now

@PVince81
Copy link
Contributor

Seems to work fine with the LDAP zombie army 👍

@PVince81
Copy link
Contributor

(the order of users is wrong but I get this was already a problem before) CC @blizzz

@ghost
Copy link

ghost commented Dec 12, 2014

Refer to this link for build results (access rights to CI server needed):
https://ci.owncloud.org//job/pull-request-analyser-ng-simple/4429/

Build result: FAILURE

[...truncated 17 lines...]using GIT_SSH to set credentials using .gitcredentials to set credentials > git config --local credential.helper store --file=/tmp/git6533418843201547087.credentials # timeout=10 > git fetch --tags --progress https://github.com/owncloud/core.git +refs/pull/:refs/remotes/origin/pr/ > git config --local --remove-section credential # timeout=10 > git rev-parse origin/pr/12726/merge^{commit} # timeout=10 > git branch -a --contains 96051d925d902562801572c9a62960119715c07a # timeout=10 > git rev-parse remotes/origin/pr/12726/merge^{commit} # timeout=10Checking out Revision 96051d925d902562801572c9a62960119715c07a (origin/pr/12726/merge) > git config core.sparsecheckout # timeout=10 > git checkout -f 96051d925d902562801572c9a62960119715c07a > git rev-list b4f10be9e41b1c1af4d5a6f8d65d8b4d3caaa870 # timeout=10First time build. Skipping changelog. > git remote # timeout=10 > git submodule init # timeout=10 > git submodule sync # timeout=10 > git config --get remote.origin.url # timeout=10 > git submodule update --init --recursiveTriggering pull-request-analyser-ng-simple » SLAVEpull-request-analyser-ng-simple » SLAVE completed with result FAILUREStarted calculate disk usage of buildFinished Calculation of disk usage of build in 0 secondsStarted calculate disk usage of workspaceFinished Calculation of disk usage of workspace in 2 second
💣 Test FAILed. 💣

@MorrisJobke
Copy link
Contributor

jenkins PR #12822

@scrutinizer-notifier
Copy link

The inspection completed: 25 new issues, 9 updated code elements

@scrutinizer-notifier
Copy link

The inspection completed: 25 new issues, 9 updated code elements

@MorrisJobke
Copy link
Contributor

The failing tests also appear on other branches and are unrelated (files cache) -> merge

MorrisJobke added a commit that referenced this pull request Dec 13, 2014
…t-index

Add filter for backend to rest index
@MorrisJobke MorrisJobke merged commit efb495b into master Dec 13, 2014
@MorrisJobke MorrisJobke deleted the add-filter-for-backend-to-rest-index branch December 13, 2014 07:50
@MorrisJobke MorrisJobke mentioned this pull request Dec 22, 2014
27 tasks
@lock lock bot locked as resolved and limited conversation to collaborators Aug 15, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants