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

Continuing the New User management #7151

Merged
merged 106 commits into from Jun 3, 2014

Conversation

Projects
None yet
@raghunayyar
Member

raghunayyar commented Feb 11, 2014

🚧 PR just for collaborating. Work In Progress, don't click merge yet.

First priority, basic functionality

  • the settings area should slide out smoothly, like in News. Also it should be possible to close it again by clicking on the same header area. And the header shouldn’t have a different color than the drawer. Best just try to extract it out from News and transfer the styles to core.
  • Search filter needs to rely on server response, because not necessarily all users are already in the list (by @blizzz )
  • the undo notification should disappear after 7 seconds and perform the the delete operation (by @DeepDiver1975 / @jancborchardt )
  • creating a group on the drop down menu dies not update the group list on the left (by @DeepDiver1975 )
  • creating a group on the group panel does not update the list (by @DeepDiver1975 )
  • delete of group does not trigger delete on the server (by @DeepDiver1975 )
  • adding a group should not be an input and button right away, but rather work like adding Groups in the Contacts app (inserting a new entry below) (by @jancborchardt)
  • the clickable area of the group actions (rename, delete) needs to be the whole row height and width of 44px
  • the rename button should be directly next to the name, just like in the Files app
  • the rename and delete icons should show half-transparent, and only dark on hover – maybe only use the .action class, just like Files and News do
  • create users fails with: Fatal error: Call to undefined method OCP\JSON::checkSubAdminUser() in /home/blizzz/owncloud/dev/master/settings/ajax/createuser.php on line 4 (by @blizzz)
  • filtering users oftentimes does not work, filtering admins almost never works (shows more people) (by @jancborchardt )
  • we need a spinner as feedback when switching groups (by @jancborchardt )
  • when a group is selected/shown, mark the entry in the sidebar as active. Just like in Notes. (by @jancborchardt )
  • The delete group action should only show up for the currently active group. That is, not on hover. (by @jancborchardt )
  • The »Create« buttons have too big of a font-size (18.33 px, cause of 0.8 em of the body) (by @jancborchardt )
  • »admin« is a set group, it shouldn’t be possible to delete (right @DeepDiver1975?) (by @jancborchardt )
  • In addition to deletion, we need a »rename« action on the group (by @jancborchardt )
  • We need search in form of a filter (by @jancborchardt )
  • selecting a group on the group panel does not update the user list (by @DeepDiver1975 )
  • rename Storage to Quota (by @blizzz)
  • groups should be sorted by number of people in them, not by alphabet (by @jancborchardt)
  • groups with no members should have no number, instead of a 0 (by @jancborchardt)
  • the sidebar should be position: fixed so it doesn’t move around when scrolling (should be in the core styles) (by @jancborchardt)
  • the app-navigation needs a higher z-index than the content, so that the content does not overlap the sidebar when you scroll horizontally (should be in the core styles, by @jancborchardt)
  • the »Create« button should say »Add user« to be more specific (by @jancborchardt)
  • the settings area should be a slide-out drawer like in the News app (by @jancborchardt)
  • What shall happen when clicking on a filtered group? – Show all users of that group, but show those first which also match the filter term. Maybe with a divider below. (by @blizzz )
  • Click on Everybody: No, it should display »Everybody« but filtered by the filter. Only resetting the filter should reset the filter. (by @blizzz )
  • To reset filter: »clickable x icon in the filter box to empty the filter«(by @blizzz )

Additional features

  • should be usable to manage huge amount of users (>20k) (by @blizzz )
  • pre-defined filters: Everyone, Users by Backend (Internal, LDAP, etc.), Not Grouped (users without group), Disabled Users. They shall be shown only if they contain rows, and be post-loaded using Ajax (by @blizzz )
  • Last Login requires core to store last login/session refresh (by @blizzz )
  • column for email (after Full Name), lets admin edit the email (by @blizzz )
  • optional columns (to activate by checkbox on the bottom left): Usage (displays Quota usage in %, after Quota), followed by Storage Location, Last Login (last col). Choice of which are displayed should be remembered (by @blizzz )
  • checkbox to send email to users after creation of the account (by @blizzz )
  • when encryption app is enabled and a recovery key is available: on password change ask for recovery key by prompt so that files are recovered. see current implementation as example on how it works. (by @blizzz \cc @schiesbn )

Big features which have to wait first

  • multiselect and edit of users (as in Files or Contacts-master app) (by @blizzz )
  • option to disable internal users (by @blizzz )
  • sortable columns (by @blizzz )
@raghunayyar

This comment has been minimized.

Show comment
Hide comment
@raghunayyar

raghunayyar Feb 11, 2014

Member

@jancborchardt here we go. ;)

Member

raghunayyar commented Feb 11, 2014

@jancborchardt here we go. ;)

@jancborchardt

This comment has been minimized.

Show comment
Hide comment
@jancborchardt

jancborchardt Feb 11, 2014

Member

@DeepDiver1975 @Raydiation @PVince81 check this out.

cc @owncloud/designers @MTRichards as well.

Member

jancborchardt commented Feb 11, 2014

@DeepDiver1975 @Raydiation @PVince81 check this out.

cc @owncloud/designers @MTRichards as well.

@jancborchardt jancborchardt added this to the ownCloud 7 milestone Feb 11, 2014

@jancborchardt

This comment has been minimized.

Show comment
Hide comment
@jancborchardt

jancborchardt Feb 11, 2014

Member

Good one @raghunayyar!

Feedback:

  • The delete group action should only show up for the currently active group.
  • The »Create« buttons have too big of a font-size (18.33 px, cause of 0.8 em of the body)
  • »admin« is a set group, it shouldn’t be possible to delete (right @DeepDiver1975?)

Additional features:

  • In addition to deletion, we need a »rename« action on the group
  • We need search in form of a filter
  • We need an email input field
Member

jancborchardt commented Feb 11, 2014

Good one @raghunayyar!

Feedback:

  • The delete group action should only show up for the currently active group.
  • The »Create« buttons have too big of a font-size (18.33 px, cause of 0.8 em of the body)
  • »admin« is a set group, it shouldn’t be possible to delete (right @DeepDiver1975?)

Additional features:

  • In addition to deletion, we need a »rename« action on the group
  • We need search in form of a filter
  • We need an email input field
@karlitschek

This comment has been minimized.

Show comment
Hide comment
@karlitschek

karlitschek Feb 11, 2014

Member

We have the additional request to do "user defined groups" any idea here?

Member

karlitschek commented Feb 11, 2014

We have the additional request to do "user defined groups" any idea here?

@jancborchardt

This comment has been minimized.

Show comment
Hide comment
@jancborchardt

jancborchardt Feb 11, 2014

Member

@karlitschek what does that mean exactly? Is there a writeup or issue on that?

@raghunayyar we should change »Storage« to »Quota« to prevent misunderstanding. Can you do that?

Member

jancborchardt commented Feb 11, 2014

@karlitschek what does that mean exactly? Is there a writeup or issue on that?

@raghunayyar we should change »Storage« to »Quota« to prevent misunderstanding. Can you do that?

@karlitschek

This comment has been minimized.

Show comment
Hide comment
@karlitschek

karlitschek Feb 11, 2014

Member

@jancborchardt Not clear yet. Needs to be defined

Member

karlitschek commented Feb 11, 2014

@jancborchardt Not clear yet. Needs to be defined

@DeepDiver1975

This comment has been minimized.

Show comment
Hide comment
@DeepDiver1975

DeepDiver1975 Feb 11, 2014

Member

The inspection completed: 5 new/changed issues

@raghunayyar please have a look at the scrutinizer inspection - it's worth having a look 😉

Member

DeepDiver1975 commented Feb 11, 2014

The inspection completed: 5 new/changed issues

@raghunayyar please have a look at the scrutinizer inspection - it's worth having a look 😉

@DeepDiver1975

This comment has been minimized.

Show comment
Hide comment
@DeepDiver1975

DeepDiver1975 Feb 11, 2014

Member

@raghunayyar did you test this? 😉

Uncaught SyntaxError: Unexpected token } users.js:601
Member

DeepDiver1975 commented Feb 11, 2014

@raghunayyar did you test this? 😉

Uncaught SyntaxError: Unexpected token } users.js:601
@DeepDiver1975

This comment has been minimized.

Show comment
Hide comment
@DeepDiver1975

DeepDiver1975 Feb 11, 2014

Member

Is search supposed to work?

I have a strange scroll behavior:
the group panel and the user list can be scrolled on few pixels up/down - no matter how big the window is

Member

DeepDiver1975 commented Feb 11, 2014

Is search supposed to work?

I have a strange scroll behavior:
the group panel and the user list can be scrolled on few pixels up/down - no matter how big the window is

@raghunayyar

This comment has been minimized.

Show comment
Hide comment
@raghunayyar

raghunayyar Feb 11, 2014

Member

@DeepDiver1975 it's an initial commit. I too am getting issues. Will definitely fix it this morning.

Member

raghunayyar commented Feb 11, 2014

@DeepDiver1975 it's an initial commit. I too am getting issues. Will definitely fix it this morning.

@MTRichards

This comment has been minimized.

Show comment
Hide comment
@MTRichards

MTRichards Feb 11, 2014

Contributor

The way it was explained to me:
As a user, I want to be able to log into ownCloud and share files with others. I also want to be able to create my own, private, user defined groups so that it is easier for me to share files quickly with an ad-hoc group of my peers.

Acceptance Criteria:

  • User can easily create user defined groups for their use
  • User can share files and folders with user defined groups the same was as with ownCloud defined users and groups
  • User can manage (rename, add, remove) their user defined groups
  • Other users cannot see the user defined groups

At least at a high level, that is what folks have explained to me.

Contributor

MTRichards commented Feb 11, 2014

The way it was explained to me:
As a user, I want to be able to log into ownCloud and share files with others. I also want to be able to create my own, private, user defined groups so that it is easier for me to share files quickly with an ad-hoc group of my peers.

Acceptance Criteria:

  • User can easily create user defined groups for their use
  • User can share files and folders with user defined groups the same was as with ownCloud defined users and groups
  • User can manage (rename, add, remove) their user defined groups
  • Other users cannot see the user defined groups

At least at a high level, that is what folks have explained to me.

@DeepDiver1975

This comment has been minimized.

Show comment
Hide comment
@DeepDiver1975

DeepDiver1975 Feb 11, 2014

Member

@MTRichards these groups are supposed to be used for sharing only? In addition these groups seem to be user specific - e.g. my group named 'family' will contain a different set of user compared to a different user.

Shall this group contain ownCloud users only or shall emails for public/pin-protected sharing be included as well?

I guess we can move this to the sharing next generation ticket - right?

Member

DeepDiver1975 commented Feb 11, 2014

@MTRichards these groups are supposed to be used for sharing only? In addition these groups seem to be user specific - e.g. my group named 'family' will contain a different set of user compared to a different user.

Shall this group contain ownCloud users only or shall emails for public/pin-protected sharing be included as well?

I guess we can move this to the sharing next generation ticket - right?

@MTRichards

This comment has been minimized.

Show comment
Hide comment
@MTRichards

MTRichards Feb 11, 2014

Contributor

@MTRichards these groups are supposed to be used for sharing only? In addition these groups seem to be user specific - e.g. my group named 'family' will contain a different set of user compared to a different user.

Yes and Yes!

Shall this group contain ownCloud users only or shall emails for public/pin-protected sharing be included as well?

From a user perspective, a group should be able to be a list of users, groups, or email addresses. It is just a shortcut when sharing a lot of the same files with people all the time.

I guess we can move this to the sharing next generation ticket - right?

Absolutely, but I think this is a tad lower on the list than a lot of the sharing elements we need.

Contributor

MTRichards commented Feb 11, 2014

@MTRichards these groups are supposed to be used for sharing only? In addition these groups seem to be user specific - e.g. my group named 'family' will contain a different set of user compared to a different user.

Yes and Yes!

Shall this group contain ownCloud users only or shall emails for public/pin-protected sharing be included as well?

From a user perspective, a group should be able to be a list of users, groups, or email addresses. It is just a shortcut when sharing a lot of the same files with people all the time.

I guess we can move this to the sharing next generation ticket - right?

Absolutely, but I think this is a tad lower on the list than a lot of the sharing elements we need.

@sfabel

This comment has been minimized.

Show comment
Hide comment
@sfabel

sfabel Feb 11, 2014

+1 on this feature - this would be useful for enterprise customers as well, trying to establish collaboration projects outside of the existing directory backend.

sfabel commented Feb 11, 2014

+1 on this feature - this would be useful for enterprise customers as well, trying to establish collaboration projects outside of the existing directory backend.

@raghunayyar

This comment has been minimized.

Show comment
Hide comment
@raghunayyar

raghunayyar Feb 12, 2014

Member

@DeepDiver1975 forced pushed the correction typos and the syntax errors along with the Initial Search Commit just now. Looking it into Scrutinizer now ;)

Member

raghunayyar commented Feb 12, 2014

@DeepDiver1975 forced pushed the correction typos and the syntax errors along with the Initial Search Commit just now. Looking it into Scrutinizer now ;)

@raghunayyar

This comment has been minimized.

Show comment
Hide comment
@raghunayyar

raghunayyar Feb 12, 2014

Member

@jancborchardt do you want to give an option of adding the email ID while a user is created? I mean, three inputs. Username, Password and Email ID. Or is there something I am missing here. Please explain! thanks!

Member

raghunayyar commented Feb 12, 2014

@jancborchardt do you want to give an option of adding the email ID while a user is created? I mean, three inputs. Username, Password and Email ID. Or is there something I am missing here. Please explain! thanks!

@DeepDiver1975

This comment has been minimized.

Show comment
Hide comment
@DeepDiver1975

DeepDiver1975 Feb 12, 2014

Member

@jancborchardt do you want to give an option of adding the email ID while a user is created? I mean, three inputs. Username, Password and Email ID. Or is there something I am missing here. Please explain! thanks!

email is not mandatory at the moment - and we shall keep this constraint.

Member

DeepDiver1975 commented Feb 12, 2014

@jancborchardt do you want to give an option of adding the email ID while a user is created? I mean, three inputs. Username, Password and Email ID. Or is there something I am missing here. Please explain! thanks!

email is not mandatory at the moment - and we shall keep this constraint.

@jancborchardt

This comment has been minimized.

Show comment
Hide comment
@jancborchardt

jancborchardt Feb 12, 2014

Member

Even though the email is not mandatory, I would introduce that field in the "adding a user" row. Password and groups are optional as well. But email is important for lots of communication, notification, password reset etc.

Just have the placeholder say "Email (optional)".

Member

jancborchardt commented Feb 12, 2014

Even though the email is not mandatory, I would introduce that field in the "adding a user" row. Password and groups are optional as well. But email is important for lots of communication, notification, password reset etc.

Just have the placeholder say "Email (optional)".

@raghunayyar

This comment has been minimized.

Show comment
Hide comment
@raghunayyar

raghunayyar Feb 12, 2014

Member

@jancborchardt @DeepDiver1975 can you guys test the search behaviour now? @kabum helped me fix it. ;)

Member

raghunayyar commented Feb 12, 2014

@jancborchardt @DeepDiver1975 can you guys test the search behaviour now? @kabum helped me fix it. ;)

@heavymanto

This comment has been minimized.

Show comment
Hide comment
@heavymanto

heavymanto Feb 13, 2014

hi, I'd like to have two new features

  1. a checkbox to enable / disable user
  2. Be able to set up the email of the user already created

thanks

heavymanto commented Feb 13, 2014

hi, I'd like to have two new features

  1. a checkbox to enable / disable user
  2. Be able to set up the email of the user already created

thanks

@raghunayyar

This comment has been minimized.

Show comment
Hide comment
@raghunayyar

raghunayyar Feb 14, 2014

Member

@heavymanto lets do the MVP first please! I am sure the features you suggested will be followed by.

Member

raghunayyar commented Feb 14, 2014

@heavymanto lets do the MVP first please! I am sure the features you suggested will be followed by.

@DeepDiver1975

This comment has been minimized.

Show comment
Hide comment
@DeepDiver1975

DeepDiver1975 Feb 14, 2014

Member

I have a strange scroll behavior:
the group panel and the user list can be scrolled on few pixels up/down - no matter how big the window is

@raghunayyar @jancborchardt this scrolling issue is still there

Member

DeepDiver1975 commented Feb 14, 2014

I have a strange scroll behavior:
the group panel and the user list can be scrolled on few pixels up/down - no matter how big the window is

@raghunayyar @jancborchardt this scrolling issue is still there

@scrutinizer-notifier

This comment has been minimized.

Show comment
Hide comment
@scrutinizer-notifier

scrutinizer-notifier commented Jun 2, 2014

A new inspection was created.

@scrutinizer-notifier

This comment has been minimized.

Show comment
Hide comment
@scrutinizer-notifier

scrutinizer-notifier Jun 2, 2014

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

scrutinizer-notifier commented Jun 2, 2014

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

@owncloud-bot

This comment has been minimized.

Show comment
Hide comment
@owncloud-bot

owncloud-bot Jun 2, 2014

Contributor

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

Contributor

owncloud-bot commented Jun 2, 2014

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

@blizzz blizzz changed the title from [WIP] : Continuing the New User management to Continuing the New User management Jun 2, 2014

@blizzz

This comment has been minimized.

Show comment
Hide comment
@blizzz

blizzz Jun 2, 2014

Contributor

Ladies & Gentlemen, let's get this reviewed and merged. Remaining tasks from #7151 (comment) will be dealt with as bugs.

Contributor

blizzz commented Jun 2, 2014

Ladies & Gentlemen, let's get this reviewed and merged. Remaining tasks from #7151 (comment) will be dealt with as bugs.

@karlitschek

This comment has been minimized.

Show comment
Hide comment
@karlitschek

karlitschek Jun 2, 2014

Member

looks good to me 👍

Member

karlitschek commented Jun 2, 2014

looks good to me 👍

@karlitschek

This comment has been minimized.

Show comment
Hide comment
@karlitschek

karlitschek Jun 2, 2014

Member

Who can help to merge this? @PVince81 @DeepDiver1975 @MTRichards ?

Member

karlitschek commented Jun 2, 2014

Who can help to merge this? @PVince81 @DeepDiver1975 @MTRichards ?

@scrutinizer-notifier

This comment has been minimized.

Show comment
Hide comment
@scrutinizer-notifier

scrutinizer-notifier commented Jun 2, 2014

A new inspection was created.

@schiessle

This comment has been minimized.

Show comment
Hide comment
@schiessle

schiessle Jun 2, 2014

Member

Tested recovery key functionality for encryption, works as expected... For this part 👍 Also otherwise it looks good, I didn't looked a the code.

Member

schiessle commented Jun 2, 2014

Tested recovery key functionality for encryption, works as expected... For this part 👍 Also otherwise it looks good, I didn't looked a the code.

@schiessle

This comment has been minimized.

Show comment
Hide comment
@schiessle

schiessle Jun 2, 2014

Member

Just a minor thing, why does the quote drop-down looks different then the other drop-downs?

usermanagement

Member

schiessle commented Jun 2, 2014

Just a minor thing, why does the quote drop-down looks different then the other drop-downs?

usermanagement

@blizzz

This comment has been minimized.

Show comment
Hide comment
@blizzz

blizzz Jun 2, 2014

Contributor

The other ones are jquery ui multi-select dropdowns, does not differ to 6.

Contributor

blizzz commented Jun 2, 2014

The other ones are jquery ui multi-select dropdowns, does not differ to 6.

@owncloud-bot

This comment has been minimized.

Show comment
Hide comment
@owncloud-bot

owncloud-bot Jun 2, 2014

Contributor

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

Contributor

owncloud-bot commented Jun 2, 2014

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

@DeepDiver1975

This comment has been minimized.

Show comment
Hide comment
@DeepDiver1975

DeepDiver1975 Jun 3, 2014

Member

👍 🚀 👯 :shipit:

Member

DeepDiver1975 commented Jun 3, 2014

👍 🚀 👯 :shipit:

DeepDiver1975 added a commit that referenced this pull request Jun 3, 2014

Merge pull request #7151 from owncloud/user-jquery
Continuing the New User management

@DeepDiver1975 DeepDiver1975 merged commit 79b3558 into master Jun 3, 2014

1 check passed

default Merged build finished.
Details

@DeepDiver1975 DeepDiver1975 deleted the user-jquery branch Jun 3, 2014

@MorrisJobke

This comment has been minimized.

Show comment
Hide comment
@MorrisJobke

MorrisJobke Jun 3, 2014

Member

👏

Member

MorrisJobke commented Jun 3, 2014

👏

@vgezer

This comment has been minimized.

Show comment
Hide comment
@vgezer

vgezer Jun 4, 2014

Contributor

@blizzz Can you use a placeholder so that translators will be understanding what is going on with this string? It also has a trailingspace which users will not notice on transifex.

Contributor

vgezer commented on settings/js/users/deleteHandler.js in b3d68fc Jun 4, 2014

@blizzz Can you use a placeholder so that translators will be understanding what is going on with this string? It also has a trailingspace which users will not notice on transifex.

This comment has been minimized.

Show comment
Hide comment
@blizzz

blizzz Jun 5, 2014

Contributor

Could you please open a new issue for this? It's hard to track in a commit (that was already merged). Do you also have an example for the placeholder?

Contributor

blizzz replied Jun 5, 2014

Could you please open a new issue for this? It's hard to track in a commit (that was already merged). Do you also have an example for the placeholder?

@vgezer

This comment has been minimized.

Show comment
Hide comment
@vgezer

vgezer Jun 4, 2014

Contributor

Here, too

Contributor

vgezer commented on settings/js/users/groups.js in b3d68fc Jun 4, 2014

Here, too

@vgezer

This comment has been minimized.

Show comment
Hide comment
@vgezer

vgezer Jun 4, 2014

Contributor

And here.

Contributor

vgezer commented on settings/js/users/users.js in b3d68fc Jun 4, 2014

And here.

@vgezer

This comment has been minimized.

Show comment
Hide comment
@vgezer

vgezer Jun 7, 2014

Contributor

Can we remove groups now? I remember seeing it 2 days ago, but now I cannot.

Contributor

vgezer commented Jun 7, 2014

Can we remove groups now? I remember seeing it 2 days ago, but now I cannot.

@blizzz

This comment has been minimized.

Show comment
Hide comment
@blizzz

blizzz Jun 10, 2014

Contributor

It was implemented, maybe cleanup or rebase killed it… can you open a new issue for it, please?

Contributor

blizzz commented Jun 10, 2014

It was implemented, maybe cleanup or rebase killed it… can you open a new issue for it, please?

@juhulian

This comment has been minimized.

Show comment
Hide comment
@juhulian

juhulian Mar 30, 2016

It would be nice to have a monitor mode for user storage...

juhulian commented Mar 30, 2016

It would be nice to have a monitor mode for user storage...

@jancborchardt

This comment has been minimized.

Show comment
Hide comment
@jancborchardt

jancborchardt Mar 30, 2016

Member

@juhulian please instead of commenting on a long solved pull request open a new issue. :) In there also explain more in detail what you mean. Thanks

Member

jancborchardt commented Mar 30, 2016

@juhulian please instead of commenting on a long solved pull request open a new issue. :) In there also explain more in detail what you mean. Thanks

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