Skip to content

Fix james upgrade#2281

Closed
Arsnael wants to merge 2 commits intolinagora:masterfrom
Arsnael:fix-master-upgrade
Closed

Fix james upgrade#2281
Arsnael wants to merge 2 commits intolinagora:masterfrom
Arsnael:fix-master-upgrade

Conversation

@Arsnael
Copy link
Member

@Arsnael Arsnael commented Mar 23, 2026

Summary by CodeRabbit

  • Bug Fixes

    • Fixed preservation of mailbox access control metadata during message deletion operations across multiple systems, ensuring consistent behavior when handling deleted messages.
  • Chores

    • Updated internal project dependencies to latest versions for improved stability and compatibility.

@coderabbitai
Copy link

coderabbitai bot commented Mar 23, 2026

Walkthrough

This PR updates the james-project submodule reference and ensures mailbox ACL information is consistently preserved across deletion event handling. Changes include: (1) adding MailboxACL.EMPTY parameter to MessageContentDeletionEvent construction in keyword and RAG deletion listener tests, (2) expanding TeamMailboxDeletedMessageVaultDeletionListener constructor to accept MessageIdManager and SessionProvider and forward them to the superclass, and (3) preserving mailbox ACL when remapping deletion events.

Possibly related PRs

Suggested reviewers

  • quantranhong1999
  • chibenwa
🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

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.
Title check ❓ Inconclusive The title 'Fix james upgrade' is vague and does not clearly convey the specific changes made in the pull request. Consider a more descriptive title that specifies what aspect of the James upgrade is being fixed, such as 'Update James submodule and fix MessageContentDeletionEvent ACL handling' or similar.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
tmail-backend/mailbox/plugin/tmail-deleted-message-vault/common/src/main/java/com/linagora/tmail/vault/TeamMailboxDeletedMessageVaultDeletionListener.java (1)

87-101: Consider adding a non-empty ACL remapping regression test.

This path now preserves ACL, but current fixture updates mostly use MailboxACL.EMPTY; a focused test with non-empty ACL would better protect this behavior.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@tmail-backend/mailbox/plugin/tmail-deleted-message-vault/common/src/main/java/com/linagora/tmail/vault/TeamMailboxDeletedMessageVaultDeletionListener.java`
around lines 87 - 101, The test suite lacks a regression test ensuring ACLs are
preserved by TeamMailboxDeletedMessageVaultDeletionListener when building
MessageContentDeletionEvent (via mailboxACL()), so add a focused unit test that
supplies a non-empty MailboxACL (not MailboxACL.EMPTY) in the fixture, invokes
the listener/mapping that returns a MessageContentDeletionEvent, and asserts the
resulting MessageContentDeletionEvent.mailboxACL equals the original non-empty
MailboxACL; update or add test helper(s) to construct a mailbox event with a
populated ACL and ensure the mapping code path in
TeamMailboxDeletedMessageVaultDeletionListener is exercised.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In
`@tmail-backend/mailbox/plugin/tmail-deleted-message-vault/common/src/main/java/com/linagora/tmail/vault/TeamMailboxDeletedMessageVaultDeletionListener.java`:
- Around line 87-101: The test suite lacks a regression test ensuring ACLs are
preserved by TeamMailboxDeletedMessageVaultDeletionListener when building
MessageContentDeletionEvent (via mailboxACL()), so add a focused unit test that
supplies a non-empty MailboxACL (not MailboxACL.EMPTY) in the fixture, invokes
the listener/mapping that returns a MessageContentDeletionEvent, and asserts the
resulting MessageContentDeletionEvent.mailboxACL equals the original non-empty
MailboxACL; update or add test helper(s) to construct a mailbox event with a
populated ACL and ensure the mapping code path in
TeamMailboxDeletedMessageVaultDeletionListener is exercised.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: e55e31a5-57b7-4501-831c-49868307a2f2

📥 Commits

Reviewing files that changed from the base of the PR and between 954345c and 87a2d79.

📒 Files selected for processing (4)
  • james-project
  • tmail-backend/jmap/extensions/src/test/java/com/linagora/tmail/james/jmap/event/KeywordEmailQueryViewListenerTest.java
  • tmail-backend/mailbox/plugin/tmail-deleted-message-vault/common/src/main/java/com/linagora/tmail/vault/TeamMailboxDeletedMessageVaultDeletionListener.java
  • tmail-backend/tmail-third-party/ai-bot/src/test/java/com/linagora/tmail/mailet/rag/RagDeletionListenerTest.java

@Arsnael
Copy link
Member Author

Arsnael commented Mar 23, 2026

cf #2282 (review)

@Arsnael Arsnael closed this Mar 23, 2026
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