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

Permissions UI #538

Merged
merged 53 commits into from Aug 2, 2014
Merged

Permissions UI #538

merged 53 commits into from Aug 2, 2014

Conversation

mitar
Copy link
Member

@mitar mitar commented Jul 13, 2014

Among other things, fixes #482 and #450.

@mitar mitar added this to the 0.3 - Waffle milestone Jul 13, 2014
if @membersCount is 1 then "1 member" else "#{ @membersCount } members"

Template.groupCatalogItem.public = ->
console.log @
Copy link
Member Author

Choose a reason for hiding this comment

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

Remove.

@mitar
Copy link
Member Author

mitar commented Jul 13, 2014

Hm. I thought you also moved permissions display out of annotation menu? So that annotations would have icon of visibility next to the timestamp displayed. I would have an icon there and if you click it you get permission dialog. And then nothing about permissions is in menu.

Template.rolesControl.canModifyAccess = Template.accessControl.canModifyAccess

Template.rolesControlList.rolesList = ->

Copy link
Member Author

Choose a reason for hiding this comment

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

Unnecessary empty line.

@MatejJan
Copy link
Member

Defaults in annotations? Will wait for you. :) Plenty of other work elsewhere. 👍

MatejJan and others added 4 commits July 23, 2014 13:40
The issue is that we are pushing information out. So if one makes a
public entity first, someone else can learn about it before entity is
switched to private. By having it private, this cannot happen. And also
by having private people will learn where they can configure privacy
settings when they will want to share things.
@mitar
Copy link
Member Author

mitar commented Jul 29, 2014

OK, now about defaults. Point is, that we currently have already an API for setting defaults and I have a feeling that you put your code for defaults outside this API. So let me check now how to integrate this.

Check for null explicitly. Return boolean instead of count. Don't throw
an exception if user does not have access to the document.
@mitar
Copy link
Member Author

mitar commented Jul 29, 2014

For groups, also all group members can see the group. Should we expose that through on permission dialog?

@mitar
Copy link
Member Author

mitar commented Jul 29, 2014

Or maybe it would be enough just to write in the case of groups that users listed below and members can see the group.

For permissions, when access is private, maintainers and admins should
also be added to read access list so that if access is changed to
public and then their maintainer or admin permission is revoked, they
still retain read access if document is after all that switched back to
private access. We have this logic in applyDefaultAccess and setRole
and they should match.
@mitar
Copy link
Member Author

mitar commented Jul 29, 2014

OK. I synced defaults. Now it looks LGTM to me, only for that group membership thing we should do something to not lie.

Conflicts:
	client/collection.coffee
	client/group.coffee
	client/lib/access.coffee
	client/lib/catalog.coffee
	client/lib/controls.coffee
	client/publication.coffee

Also renamed all e variables to event (see #375)
@mitar
Copy link
Member Author

mitar commented Aug 1, 2014

BTW, this is almost ready for merging?

@MatejJan
Copy link
Member

MatejJan commented Aug 2, 2014

only for that group membership thing we should do something to not lie.

Done!

Merge when travis passes? (edit: this should have been an assertive statement. :))

@mitar
Copy link
Member Author

mitar commented Aug 2, 2014

Yea!

MatejJan added a commit that referenced this pull request Aug 2, 2014
@MatejJan MatejJan merged commit 3cfb77f into development Aug 2, 2014
@MatejJan MatejJan deleted the permissions-ui branch August 18, 2014 16:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Administrator and maintainer rights
2 participants