Skip to content

fix(channels): exclude attachments from deleted messages#2322

Merged
synoet merged 2 commits intomainfrom
synoet/exclude-attachments-from-deleted-messages
Mar 31, 2026
Merged

fix(channels): exclude attachments from deleted messages#2322
synoet merged 2 commits intomainfrom
synoet/exclude-attachments-from-deleted-messages

Conversation

@synoet
Copy link
Copy Markdown
Contributor

@synoet synoet commented Mar 31, 2026

No description provided.

@synoet synoet requested a review from a team as a code owner March 31, 2026 22:12
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 31, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 4315b583-2433-4d3f-9b6a-b9b631fee494

📥 Commits

Reviewing files that changed from the base of the PR and between e041beb and 61923a4.

📒 Files selected for processing (1)
  • rust/cloud-storage/channels/src/outbound/pg_channels_repo/tests.rs

📝 Walkthrough

Summary by CodeRabbit

  • Bug Fixes
    • Attachments belonging to soft-deleted messages are no longer returned by the channel-level attachments endpoint. Messages that are soft-deleted but still have active thread replies remain visible in message listings; only their attachments are excluded from channel attachment results.

Walkthrough

Updated channel attachments endpoint to exclude attachments whose parent messages are soft-deleted. Adjusted SQL in get_channel_attachments, updated fixtures to add an attachment tied to a soft-deleted message, and added tests asserting those attachments are omitted from channel-level results.

Changes

Cohort / File(s) Summary
Query & Fixture
rust/cloud-storage/channels/src/outbound/pg_channels_repo.rs, rust/cloud-storage/channels/fixtures/channels_repo.sql
Added AND m.deleted_at IS NULL to the get_channel_attachments query to exclude attachments whose messages are soft-deleted; updated fixture to add an attachment row for the soft-deleted message (msg2) and document expected exclusion.
Tests
rust/cloud-storage/channels/src/outbound/pg_channels_repo/tests.rs
Added DELETED_MSG_ATTACHMENT constant, updated expectations for MSG2 attachments, and added channel_attachments_exclude_deleted_messages test asserting deleted-message attachments are not returned by get_channel_attachments.

Possibly related PRs

🚥 Pre-merge checks | ✅ 1 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Description check ❓ Inconclusive No pull request description was provided by the author, making it impossible to evaluate relevance to the changeset. Add a description explaining the purpose of this change, such as why attachments from deleted messages should be excluded and any related context.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed Title follows conventional commits format with 'fix:' prefix, is under 72 characters (56 chars), and clearly describes the main change: excluding attachments from deleted messages.

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


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

Copy link
Copy Markdown
Contributor

@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.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@rust/cloud-storage/channels/fixtures/channels_repo.sql`:
- Around line 89-93: The fixture added attaches attachment
'00000000-0000-0000-0000-00000000a004' to MSG2, which conflicts with the test
assertion map.get(&MSG2).is_none(); update the test in outbound/pg_channels_repo
tests to expect MSG2 to have that attachment (replace map.get(&MSG2).is_none()
with an assertion that the entry exists, e.g., map.get(&MSG2).is_some()) and
adjust the expected attachment list for MSG2 to include the new attachment id
'00000000-0000-0000-0000-00000000a004' so the test reflects the updated fixture.
🪄 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: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 3ba336da-7175-42b3-8112-2a3fcd4b0837

📥 Commits

Reviewing files that changed from the base of the PR and between 35ff875 and e041beb.

⛔ Files ignored due to path filters (1)
  • rust/cloud-storage/channels/.sqlx/query-6958dcc3ad28334546d21a169c3724111cd93d5304bf2d9da4dac7a1cc5ad77d.json is excluded by !**/.sqlx/**
📒 Files selected for processing (3)
  • rust/cloud-storage/channels/fixtures/channels_repo.sql
  • rust/cloud-storage/channels/src/outbound/pg_channels_repo.rs
  • rust/cloud-storage/channels/src/outbound/pg_channels_repo/tests.rs

@synoet synoet merged commit ab93115 into main Mar 31, 2026
38 checks passed
@synoet synoet deleted the synoet/exclude-attachments-from-deleted-messages branch March 31, 2026 22:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant