server: remove user-project mappings when delete a project - #14150
weizhouapache wants to merge 1 commit into
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## 4.20 #14150 +/- ##
============================================
- Coverage 16.37% 16.37% -0.01%
+ Complexity 13614 13613 -1
============================================
Files 5669 5669
Lines 501527 501528 +1
Branches 60922 60923 +1
============================================
- Hits 82148 82142 -6
- Misses 410171 410179 +8
+ Partials 9208 9207 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
🟡 Changes recommended
Add regression tests covering removal of user-specific mappings during project cleanup.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Fixes project cleanup so user-specific project mappings are removed when deleting a project, allowing associated accounts to be deleted.
Changes:
- Allows nullable
userIdlookups in the project-account DAO. - Uses account-wide lookup during project cleanup.
- Updates the DAO contract.
File summaries
| File | Summary |
|---|---|
server/src/main/java/com/cloud/projects/ProjectManagerImpl.java |
Removes mappings regardless of user association during cleanup. |
engine/schema/src/main/java/com/cloud/projects/dao/ProjectAccountDaoImpl.java |
Implements optional user filtering. |
engine/schema/src/main/java/com/cloud/projects/dao/ProjectAccountDao.java |
Updates the DAO method signature. |
Review details
Suppressed comments (1)
engine/schema/src/main/java/com/cloud/projects/dao/ProjectAccountDaoImpl.java:116
- Please add a regression test for the new nullable path. The current tests exercise user-specific lookups but do not verify that a user-specific
project_accountrow is found and removed whenuserIdis null; without that coverage, the fix for #12601 can regress to the old account-level-only lookup unnoticed.
if (userId != null) {
sc.setParameters("userId", userId);
}
- Files reviewed: 3/3 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| @Override | ||
| public boolean unassignAccountFromProject(long projectId, long accountId) { | ||
| ProjectAccountVO projectAccount = _projectAccountDao.findByProjectIdAccountId(projectId, accountId); | ||
| ProjectAccountVO projectAccount = _projectAccountDao.findByProjectIdUserId(projectId, accountId, null); |
Description
This PR fixes #12601
When an account is added to a project with a project role (e.g. via addUserToProject), the resulting project_account row has a non-null user_id. Project cleanup (deleteProject ... cleanup=true) unassigns every account from the project via ProjectAccountDaoImpl#findByProjectIdUserId(projectId, accountId, null), expecting a null userId to mean "match this account regardless of which user_id the row has".
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
Verified on a live 2-node management server deployment:
How did you try to break this feature and the system with this change?