Skip to content
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

Ajaxify user list for files external stable6 #10576

Merged
merged 5 commits into from
Oct 18, 2014

Conversation

butonic
Copy link
Member

@butonic butonic commented Aug 21, 2014

backport of #8507 to stable6 as separate PR because it introduces the select2 library

@butonic butonic added this to the 2014-sprint-02-current milestone Aug 21, 2014
@butonic butonic self-assigned this Aug 21, 2014
@ghost
Copy link

ghost commented Aug 21, 2014

🚀 Test Passed. 🚀
Refer to this link for build results: https://ci.owncloud.org/job/pull-request-analyser/6833/

@craigpg craigpg modified the milestones: 2014-sprint-03-next, 2014-sprint-02-current Sep 2, 2014
@LukasReschke
Copy link
Member

Same as #10558 (comment):

  • 3rdparty matches upstream
  • Permission checks are in place
  • No XSS and CSRF
  • Seems to work for me.

=> 👍

$tmpl = new OCP\Template('files_external', 'settings');
$tmpl->assign('isAdminPage', true);
$tmpl->assign('mounts', OC_Mount_Config::getSystemMountPoints());
$tmpl->assign('backends', OC_Mount_Config::getBackends());
$tmpl->assign('groups', OC_Group::getGroups());
$tmpl->assign('users', OCP\User::getUsers());
$tmpl->assign('userDisplayNames', OC_User::getDisplayNames());
Copy link
Contributor

Choose a reason for hiding this comment

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

this line needs to get removed (as it is in the original PR).

@blizzz
Copy link
Contributor

blizzz commented Sep 5, 2014

with my last commit 👍 someone wants to doublecheck me? @PVince81

@PVince81
Copy link
Contributor

PVince81 commented Sep 5, 2014

Do we really want to backport this big change that introduces a new library to stable6 ? @karlitschek please confirm

@ghost
Copy link

ghost commented Sep 5, 2014

🚀 Test Passed. 🚀
Refer to this link for build results: https://ci.owncloud.org/job/pull-request-analyser/7106/

@karlitschek
Copy link
Contributor

@blizzz @DeepDiver1975 and @PVince81 what are your opinions?
Unfortunately this is a quite grave problem if you have a lot of users.

@blizzz
Copy link
Contributor

blizzz commented Sep 5, 2014

Upon further testing, i saw another glitch:

  • adding a second external storage, all users applied to the first one disappear (only in the GUI, stays intact)

About the backport in general:

Apparently it is not possible to save it with the methods available in a reasonable amount of time and effort. The 3rd party lib is only added to files_external just as are all other code changes. It makes sense a to assign someone to do more intensive testing (inlcuding having LDAP users). OTOH, we already backported it to OC 7.

@PVince81
Copy link
Contributor

PVince81 commented Sep 5, 2014

@blizzz cannot reproduce on master with non-LDAP users. Do you see it there ??
Will try stable7.
Maybe that issue is specific to stable6 ?
We should raise a separate ticket for that and get it fixed on all affected branches.

@blizzz
Copy link
Contributor

blizzz commented Sep 5, 2014

No, on master everything was fine, I only saw it on s6. Did not test s7 about it, good thing to do.

@butonic
Copy link
Member Author

butonic commented Sep 8, 2014

@blizzz I also cannot reproduce the glitch of users disappearing in the GUI in stable6. Which browser? Did you reload the admin settings? I see the users flickering when adding a new external mount point, but they reappear immediately (ff & chrome).

@blizzz
Copy link
Contributor

blizzz commented Sep 15, 2014

FF 32. Yes, I reloaded and tried again, it was reproducible.

@craigpg craigpg modified the milestones: 2014-sprint-04-current, 2014-sprint-03 Sep 15, 2014
@craigpg craigpg modified the milestones: 2014-sprint-05-current, 2014-sprint-04 Sep 29, 2014
@ghost
Copy link

ghost commented Oct 6, 2014

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

@craigpg craigpg modified the milestones: 2014-sprint-06-current, 2014-sprint-05 Oct 12, 2014
@butonic butonic force-pushed the ajaxify_user_list_for_files_external_stable6 branch from 31ae00f to 2107915 Compare October 13, 2014 15:49
@scrutinizer-notifier
Copy link

The inspection completed: 3 new issues, 4 updated code elements

@ghost
Copy link

ghost commented Oct 15, 2014

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

@butonic
Copy link
Member Author

butonic commented Oct 16, 2014

@owncloud-bot retest this please

@ghost
Copy link

ghost commented Oct 16, 2014

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

Build result: FAILURE

GitHub pull request #10576 of commit 2107915 automatically merged.Building remotely on vm-slave-02 (SLAVE) in workspace /var/jenkins/workspace/pull-request-analyser-ng-simple@32 > git rev-parse --is-inside-work-tree # timeout=10Fetching changes from the remote Git repository > git config remote.origin.url https://github.com/owncloud/core.git # timeout=10Fetching upstream changes from https://github.com/owncloud/core.git > git --version # timeout=10 > git fetch --tags --progress https://github.com/owncloud/core.git +refs/pull/:refs/remotes/origin/pr/ > git rev-parse origin/pr/10576/merge^{commit} # timeout=10Checking out Revision 15135d8c610ef2d6ef7d97d42ac4f98a8b266075 (detached) > git config core.sparsecheckout # timeout=10 > git checkout -f 15135d8c610ef2d6ef7d97d42ac4f98a8b266075 > git rev-list 7d8283dbea1f018c6d1523a854ac0bcc168e5044 # timeout=10 > 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 » vm-slave-02Configuration pull-request-analyser-ng-simple » vm-slave-02 is still in the queue: Waiting for next available executor on vm-slave-02pull-request-analyser-ng-simple » vm-slave-02 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 35 second
💣 Test FAILed. 💣

@LukasReschke
Copy link
Member

@owncloud-bot Retest this please.

@ghost
Copy link

ghost commented Oct 16, 2014

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

Build result: FAILURE

GitHub pull request #10576 of commit 2107915 automatically merged.Building remotely on vm-slave-02 (SLAVE) in workspace /var/jenkins/workspace/pull-request-analyser-ng-simple@2 > git rev-parse --is-inside-work-tree # timeout=10Fetching changes from the remote Git repository > git config remote.origin.url https://github.com/owncloud/core.git # timeout=10Fetching upstream changes from https://github.com/owncloud/core.git > git --version # timeout=10 > git fetch --tags --progress https://github.com/owncloud/core.git +refs/pull/:refs/remotes/origin/pr/ > git rev-parse origin/pr/10576/merge^{commit} # timeout=10Checking out Revision 15135d8c610ef2d6ef7d97d42ac4f98a8b266075 (detached) > git config core.sparsecheckout # timeout=10 > git checkout -f 15135d8c610ef2d6ef7d97d42ac4f98a8b266075 > git rev-list d324eea120b43175efa4ee6bc384fe1d40d34626 # timeout=10 > 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 » vm-slave-02Configuration pull-request-analyser-ng-simple » vm-slave-02 is still in the queue: Waiting for next available executor on vm-slave-02pull-request-analyser-ng-simple » vm-slave-02 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 12 second
💣 Test FAILed. 💣

@DeepDiver1975
Copy link
Member

Jenkins is failing because there are no js unit test results

🚀 Test PASSed. 🚀

@butonic
Copy link
Member Author

butonic commented Oct 17, 2014

@PVince81 @blizzz good to go?

@blizzz
Copy link
Contributor

blizzz commented Oct 17, 2014

👍

@PVince81
Copy link
Contributor

Had a quick try with LDAP and it seems to work fine 👍

@PVince81
Copy link
Contributor

@karlitschek is there still room in 6.0.6 for this backport ? If yes, please click "Merge pull request".
Thanks.

karlitschek added a commit that referenced this pull request Oct 18, 2014
…xternal_stable6

Ajaxify user list for files external stable6
@karlitschek karlitschek merged commit de8c624 into stable6 Oct 18, 2014
@LukasReschke LukasReschke deleted the ajaxify_user_list_for_files_external_stable6 branch October 20, 2014 07:14
@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.

8 participants