Skip to content

EPMRPP-116737 || fix project deletion logic in OrganizationProjectHandlerImpl#2713

Open
grabsefx wants to merge 2 commits into
developfrom
EPMRPP-116737-fix
Open

EPMRPP-116737 || fix project deletion logic in OrganizationProjectHandlerImpl#2713
grabsefx wants to merge 2 commits into
developfrom
EPMRPP-116737-fix

Conversation

@grabsefx

@grabsefx grabsefx commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

Summary by CodeRabbit

  • Bug Fixes

    • Project deletion now removes dependent resources in a safer sequence, preventing leftover data and improving reliability.
  • Chores

    • Improved internal object equality handling to ensure consistent behavior across system operations.

@grabsefx grabsefx requested a review from pbortnik as a code owner June 12, 2026 09:59
@coderabbitai

coderabbitai Bot commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: b213679c-4194-4f5a-b0e2-91aea266c44f

📥 Commits

Reviewing files that changed from the base of the PR and between 17be475 and d53dfea.

📒 Files selected for processing (1)
  • src/main/java/com/epam/reportportal/base/infrastructure/persistence/entity/project/email/SenderCaseOptions.java

Walkthrough

Reorders deleteProjectWithDependants to perform dependent-data cleanup (issue types, log index, analyzer suggestions, logs, attachments) before removing project content and deleting the Project entity. Separately, SenderCaseOptions now implements equals and hashCode based on its options map.

Changes

Project deletion cleanup sequence

Layer / File(s) Summary
Project deletion cleanup sequence
src/main/java/com/epam/reportportal/base/core/project/impl/OrganizationProjectHandlerImpl.java
deleteProjectWithDependants now deletes non-default issue types, then removes log index and analyzer suggestions, deletes logs and attachments, and only afterwards removes project content and deletes the Project entity.

SenderCaseOptions equality

Layer / File(s) Summary
SenderCaseOptions equals/hashCode
src/main/java/com/epam/reportportal/base/infrastructure/persistence/entity/project/email/SenderCaseOptions.java
Adds java.util.Objects import and implements equals(Object) and hashCode() that use the options map for value-based equality and hashing.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • reportportal/service-api#2710: Updates project deletion cleanup ordering in a different handler implementation using the same deletion sequence pattern.

Suggested reviewers

  • pbortnik
  • Evelina02

Poem

🐰 I hop through code with tidy paws,
Reordering cleanups for safer laws.
First remove the dependants, then all that's tied,
And Sender options now compare by side.
A little rabbit nods with pride.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: fixing project deletion logic in OrganizationProjectHandlerImpl, which is the primary focus of the changeset.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch EPMRPP-116737-fix

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@src/main/java/com/epam/reportportal/base/core/project/impl/OrganizationProjectHandlerImpl.java`:
- Around line 241-246: The cleanup of external systems (logIndexer.deleteIndex,
analyzerServiceClient.removeSuggest, logRepository.deleteByProjectId,
attachmentBinaryDataService.deleteAllByProjectId) is executed before the
irreversible DB removals (projectContentRemover.remove and
projectRepository.delete), which can leave the DB state inconsistent if a later
DB delete fails; move the external cleanup calls so they run after
projectContentRemover.remove(project) and projectRepository.delete(project) (and
consider keeping them in their existing order), and optionally guard these
external calls with try/catch/logging to handle failures without affecting the
completed DB delete.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 6b268df0-c328-494d-b2d0-8b1869490afe

📥 Commits

Reviewing files that changed from the base of the PR and between 336be2c and 17be475.

📒 Files selected for processing (1)
  • src/main/java/com/epam/reportportal/base/core/project/impl/OrganizationProjectHandlerImpl.java

Comment on lines 241 to +246
logIndexer.deleteIndex(project.getId());
analyzerServiceClient.removeSuggest(project.getId());
logRepository.deleteByProjectId(project.getId());
attachmentBinaryDataService.deleteAllByProjectId(project.getId());
projectContentRemover.remove(project);
projectRepository.delete(project);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

External cleanup runs before the irreversible project delete path.

On Lines 241-246, index/suggestion cleanup happens before projectContentRemover.remove(project) and projectRepository.delete(project). If a later DB delete step fails, the project can remain while external search/analyzer data is already removed.

Suggested ordering change
-    logIndexer.deleteIndex(project.getId());
-    analyzerServiceClient.removeSuggest(project.getId());
     logRepository.deleteByProjectId(project.getId());
     attachmentBinaryDataService.deleteAllByProjectId(project.getId());
     projectContentRemover.remove(project);
     projectRepository.delete(project);
+    logIndexer.deleteIndex(project.getId());
+    analyzerServiceClient.removeSuggest(project.getId());
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@src/main/java/com/epam/reportportal/base/core/project/impl/OrganizationProjectHandlerImpl.java`
around lines 241 - 246, The cleanup of external systems (logIndexer.deleteIndex,
analyzerServiceClient.removeSuggest, logRepository.deleteByProjectId,
attachmentBinaryDataService.deleteAllByProjectId) is executed before the
irreversible DB removals (projectContentRemover.remove and
projectRepository.delete), which can leave the DB state inconsistent if a later
DB delete fails; move the external cleanup calls so they run after
projectContentRemover.remove(project) and projectRepository.delete(project) (and
consider keeping them in their existing order), and optionally guard these
external calls with try/catch/logging to handle failures without affecting the
completed DB delete.

@sonarqubecloud

Copy link
Copy Markdown

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.

1 participant