-
Notifications
You must be signed in to change notification settings - Fork 245
Only one super admin should exists #1710
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR ensures that only one super admin exists by cleaning up duplicate super admin records and updating the user creation logic for super admins.
- Added a migration changeSet to delete extra super admin users.
- Modified the user creation method to update the existing super admin if one exists.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
server/api-service/lowcoder-server/src/main/java/org/lowcoder/runner/migrations/DatabaseChangelog.java | Added migration to remove duplicate super admin users. |
server/api-service/lowcoder-domain/src/main/java/org/lowcoder/domain/user/service/UserServiceImpl.java | Updated createNewUserByAuthUser to update an existing super admin instead of creating a new one. |
Comments suppressed due to low confidence (1)
server/api-service/lowcoder-domain/src/main/java/org/lowcoder/domain/user/service/UserServiceImpl.java:187
- [nitpick] Consider renaming the lambda parameter from 'user' to 'existingSuperAdmin' to increase clarity regarding its role in the super admin update process.
return repository.findBySuperAdminIsTrue()
...-service/lowcoder-server/src/main/java/org/lowcoder/runner/migrations/DatabaseChangelog.java
Outdated
Show resolved
Hide resolved
✅ Deploy Preview for lowcoder-test canceled.
|
…runner/migrations/DatabaseChangelog.java Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
A migration and service update to enforce that only one super admin exists by deleting any older super admin users and updating the existing one instead of always creating new.
- Added a migration ChangeSet to remove extra super admin users, keeping only the most recent.
- Modified user creation logic to update the existing super admin when
isSuperAdmin
is true or create if none exists.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
server/api-service/lowcoder-server/src/main/java/org/lowcoder/runner/migrations/DatabaseChangelog.java | New ChangeSet delete-old-super-admin to delete older super admins in the database |
server/api-service/lowcoder-domain/src/main/java/org/lowcoder/domain/user/service/UserServiceImpl.java | Updated createNewUserByAuthUser to find and update an existing super admin instead of unconditionally creating |
Comments suppressed due to low confidence (3)
server/api-service/lowcoder-server/src/main/java/org/lowcoder/runner/migrations/DatabaseChangelog.java:482
- Add unit or integration tests to verify that the
deleteOldSuperAdmin
migration correctly retains only the most recent super admin and removes older ones.
@ChangeSet(order = "031", id = "delete-old-super-admin", author = "Thomas")
server/api-service/lowcoder-domain/src/main/java/org/lowcoder/domain/user/service/UserServiceImpl.java:186
- Add tests for
createNewUserByAuthUser
whenisSuperAdmin
is true to cover both paths: updating an existing super admin and creating a new one if none exists.
if(isSuperAdmin) {
server/api-service/lowcoder-server/src/main/java/org/lowcoder/runner/migrations/DatabaseChangelog.java:482
- [nitpick] The method name
deleteOldSuperAdmin
suggests deleting a single admin; consider renaming todeleteOldSuperAdmins
orcleanupExtraSuperAdmins
to reflect that it may remove multiple users.
@ChangeSet(order = "031", id = "delete-old-super-admin", author = "Thomas")
@@ -479,6 +479,27 @@ public void renameApplicationRecordCollection(MongockTemplate mongoTemplate, Mon | |||
|
|||
} | |||
|
|||
@ChangeSet(order = "031", id = "delete-old-super-admin", author = "Thomas") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider adding a unique index on the superAdmin
field at the database level to enforce uniqueness and prevent race conditions from creating multiple super admin users.
Copilot uses AI. Check for mistakes.
Proposed changes
Make only one super admin exists
Types of changes
What types of changes does your code introduce to Lowcoder?
Put an
x
in the boxes that apply.Checklist
You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help!
This is simply a reminder of what we are going to look for before merging your code.
Put an
x
in the boxes that apply.Further comments
If this is a relatively large or complex change, kick off the discussion by explaining why you chose the solution you did and what alternatives you considered, etc.