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

MG-2071 - Rename viewer relation #2237

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

nyagamunene
Copy link
Contributor

What type of PR is this?

This a feature because it rename viewer relation to contributor since he can not only view but also edit.

What does this do?

It renames the ViewerRelation to ContributorRelation and ViewPermission to ContributePermission.

Which issue(s) does this PR fix/relate to?

Have you included tests for your changes?

Yes

Did you document any new/modified feature?

Yes

Notes

@nyagamunene nyagamunene self-assigned this May 15, 2024
@nyagamunene nyagamunene marked this pull request as ready for review May 15, 2024 17:23
@nyagamunene nyagamunene changed the title MG-2071-Rename viewer relation. MG-2071 - Rename viewer relation May 21, 2024
@nyagamunene nyagamunene force-pushed the MG-2071-RenameViewerRelation branch 2 times, most recently from 5b6c20c to 6cf308a Compare May 24, 2024 09:31
Copy link
Member

@rodneyosodo rodneyosodo left a comment

Choose a reason for hiding this comment

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

Not all places have been replaced double check

docker/spicedb/schema.zed Outdated Show resolved Hide resolved
Copy link
Contributor

@arvindh123 arvindh123 left a comment

Choose a reason for hiding this comment

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

@nyagamunene
Rename only Viewer relation to contributor and don't change the view permission to contribution , Please keep it as view

permission membership = view + member

// These permissions are made for listing purposes. They enable listing users who have only particular permission excluding higher-level permissions users.
permission admin_only = admin
permission edit_only = edit - admin
permission viewer_only = view - edit
permission contributor_only = view - edit
permission membership_only = membership - view
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
permission membership_only = membership - view
permission view_only = view - edit

@@ -16,7 +16,7 @@ definition thing {
// These permission are made for only list purpose. It helps to list users have only particular permission excluding other higher and lower permission.
permission admin_only = admin
permission edit_only = edit - admin
permission viewer_only = view - edit
permission contributor_only = view - edit

Copy link
Contributor

Choose a reason for hiding this comment

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

Since view and edit are permission and they are not relations, Lets change this to view_only

Suggested change
permission view_only = view - edit

auth/service.go Outdated
@@ -632,7 +632,7 @@ func (svc service) RetrieveDomain(ctx context.Context, token, id string) (Domain
SubjectKind: UsersKind,
Object: id,
ObjectType: DomainType,
Permission: MembershipPermission,
Permission: ViewPermission,
Copy link
Contributor

Choose a reason for hiding this comment

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

@nyagamunene Could you please explain ther reason for change to ViewPermission

Copy link
Contributor Author

Choose a reason for hiding this comment

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

From the previous understanding of how the member work I thought he should not be able to login to a domain since he will see the other members' groups, things and channels

@@ -292,7 +292,7 @@ type SDK interface {
// pm := sdk.PageMetadata{
// Offset: 0,
// Limit: 10,
// Permission: "edit", // available Options: "administrator", "delete", edit", "view", "share", "owner", "admin", "editor", "viewer"
// Permission: "edit", // available Options: "delete", edit", "view", "share", "admin"
// }
Copy link
Contributor

Choose a reason for hiding this comment

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

We also support to list using relations, Please change like below and test the listing API

Suggested change
// }
// Permission: "edit", // available Options: "administrator", "delete", edit", "view", "share", "owner", "admin", "editor", "contributor"

@@ -280,7 +280,7 @@ type SDK interface {
// pm := sdk.PageMetadata{
// Offset: 0,
// Limit: 10,
// Permission: "edit", // available Options: "administrator", "delete", edit", "view", "share", "owner", "admin", "editor", "viewer"
// Permission: "edit", // available Options: "delete", edit", "view", "share", "admin"
// }
Copy link
Contributor

Choose a reason for hiding this comment

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

We also support to list using relations, Please change like below and test the listing API

Suggested change
// }
// Permission: "edit", // available Options: "administrator", "delete", edit", "view", "share", "owner", "admin", "editor", "contributor"

Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
@nyagamunene nyagamunene force-pushed the MG-2071-RenameViewerRelation branch from 8579156 to 312035f Compare June 5, 2024 11:28
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
@nyagamunene nyagamunene force-pushed the MG-2071-RenameViewerRelation branch from 91aef93 to 7257872 Compare June 7, 2024 10:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants