feat: add disable org on creation flag and disallow removing of last admin from the org/group #317
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Member
Chief-Rishab
commented
Sep 1, 2023
•
Chief-Rishab
commented
- Disable org on creation flag added.
- Org APIs now accepts both org name and uuid. Have added checks to prevent actions on disabled orgs
- Additionally a check to prevent the last org admin and project owner to be removed. Another user must be given these permissions before removing that user
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
pyadav
force-pushed
the
main
branch
5 times, most recently
from
6d3a906
to
2d224c4
Compare
Chief-Rishab
force-pushed
the
feat/disable-org-on-creation
branch
from
5fbf9e9
to
be7cbb1
Compare
kushsharma
requested changes
Sep 2, 2023
Chief-Rishab
force-pushed
the
feat/disable-org-on-creation
branch
from
56944a2
to
1822bab
Compare
Chief-Rishab
changed the title
Chief-Rishab
force-pushed
the
feat/disable-org-on-creation
branch
from
1822bab
to
43d87a2
Compare
Chief-Rishab
force-pushed
the
feat/disable-org-on-creation
branch
from
43d87a2
to
7de3577
Compare
kushsharma
reviewed
Sep 4, 2023
kushsharma
reviewed
Sep 4, 2023
kushsharma
reviewed
Sep 4, 2023
kushsharma
reviewed
Sep 4, 2023
Chief-Rishab
force-pushed
the
feat/disable-org-on-creation
branch
from
b62d94a
to
92c5e60
Compare
kushsharma
reviewed
Sep 7, 2023
internal/api/v1beta1/group.go
Outdated
| logger.Error(err.Error()) | ||
| return nil, grpcInternalServerError | ||
| } | ||
| if len(owners) == 1 { |
There was a problem hiding this comment.
Hey where do we check the user we are trying to remove is part of owners list? This would fail the call if I am just removing a random user as well right if we have 1 owner?
kushsharma
reviewed
Sep 7, 2023
internal/api/v1beta1/group.go
Outdated
| } | ||
| } | ||
|
|
||
| owners, err := h.userService.ListByGroup(ctx, request.GetId(), schema.OwnerRelationName) |
There was a problem hiding this comment.
Have you tested this with schema.OwnerRelationName. I think we should check schema.DeletePermission instead to ensure he is the owner.
kushsharma
reviewed
Sep 7, 2023
internal/api/v1beta1/org.go
Outdated
| logger.Error(err.Error()) | ||
| return nil, grpcInternalServerError | ||
| } | ||
| if len(admins) == 1 { |
There was a problem hiding this comment.
Same as groups, where do we check if the user being removed is actually admin?
Chief-Rishab
force-pushed
the
feat/disable-org-on-creation
branch
from
a74896d
to
ff6d06b
Compare
Chief-Rishab
force-pushed
the
feat/disable-org-on-creation
branch
from
ff6d06b
to
c64fe47
Compare
Chief-Rishab
force-pushed
the
feat/disable-org-on-creation
branch
from
c64fe47
to
9d80bf0
Compare
Pull Request Test Coverage Report for Build 6125621992
💛 - Coveralls |
kushsharma
approved these changes
Sep 9, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.