Revert "fix: Editing encrypted message attachment description"

[amaricbrown04-maker]

Reverts #37270

Summary by CodeRabbit

• Refactor
  • Streamlined attachment description parsing by removing encryption-specific branching logic; description handling now uses consistent behavior regardless of encryption status
  • Updated encryption tests accordingly

[changeset-bot]

⚠️ No Changeset found

Latest commit: 841d5fd

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[dionisio-bot]

Looks like this PR is not ready to merge, because of the following issues:

• This PR is missing the 'stat: QA assured' label
• This PR is missing the required milestone or project

Please fix the issues and try again

If you have any trouble, please check the PR guidelines

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[coderabbitai]

Walkthrough

This PR removes the EncryptedMessageAttachment type guard and simplifies the message attachment parsing logic by eliminating encryption-specific branching. The related test case is updated to remove description editing verification steps, and the changeset file is deleted after consolidation.

Changes

Cohort / File(s)	Change Summary
Type definitions packages/core-typings/src/IMessage/MessageAttachment/MessageAttachmentBase.ts	Removed exported EncryptedMessageAttachment type and isEncryptedMessageAttachment() type guard function
Parse logic apps/meteor/client/lib/parseMessageTextToAstMarkdown.ts	Removed import of isEncryptedMessageAttachment; simplified parseMessageAttachment to set descriptionMd based solely on translated flag instead of checking encryption status
E2E test apps/meteor/tests/e2e/e2e-encryption/e2ee-file-encryption.spec.ts	Renamed test title and removed test steps verifying encrypted attachment description editing functionality
Changeset .changeset/healthy-colts-notice.md	Deleted changeset file declaring patch version bumps

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

• Specific areas requiring attention:
  • Verify that removing the isEncryptedMessageAttachment guard doesn't break other code paths that may depend on this type guard for runtime type checking
  • Confirm the parsing logic simplification (removing encryption-specific branching) doesn't regress encrypted attachment rendering or description handling
  • Ensure the removed E2E test steps were intentionally deprecated and that description editing is no longer a supported workflow for encrypted attachments

Possibly related PRs

• #37270: Both PRs modify EncryptedMessageAttachment type handling, parseMessageAttachment logic, and E2E tests for encrypted attachment description editing

Suggested labels

stat: ready to merge, stat: QA assured

Suggested reviewers

• cardoso
• aleksandernsilva
• gabriellsh

Poem

🐰 Type guards dismissed, encryption cast aside,
Parsing flows simplified with fewer to decide,
Tests now lean and clean, no more edit dance,
Attachments freed from crypto's tight stance! ✨

Pre-merge checks and finishing touches

✅ Passed checks (3 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: reverting a previous fix for editing encrypted message attachment descriptions, which is exactly what the changeset accomplishes.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

• Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 20490af and 841d5fd.

📒 Files selected for processing (4)

• .changeset/healthy-colts-notice.md (0 hunks)
• apps/meteor/client/lib/parseMessageTextToAstMarkdown.ts (1 hunks)
• apps/meteor/tests/e2e/e2e-encryption/e2ee-file-encryption.spec.ts (1 hunks)
• packages/core-typings/src/IMessage/MessageAttachment/MessageAttachmentBase.ts (0 hunks)

💤 Files with no reviewable changes (2)

• packages/core-typings/src/IMessage/MessageAttachment/MessageAttachmentBase.ts
• .changeset/healthy-colts-notice.md

🧰 Additional context used

📓 Path-based instructions (3)

apps/meteor/tests/e2e/**/*.spec.ts

📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)

apps/meteor/tests/e2e/**/*.spec.ts: All Playwright test files must be located under apps/meteor/tests/e2e/ and use the .spec.ts extension (e.g., login.spec.ts)
Use descriptive test names that clearly communicate expected behavior
Use test.beforeAll() and test.afterAll() for setup and teardown
Use test.step() to organize complex test scenarios
Group related tests in the same file
Utilize Playwright fixtures (test, page, expect) consistently
Prefer web-first assertions (e.g., toBeVisible, toHaveText)
Use expect matchers (toEqual, toContain, toBeTruthy, toHaveLength, etc.) instead of assert statements
Maintain test isolation between test cases
Ensure a clean state for each test execution
Ensure tests run reliably in parallel without shared state conflicts

Files:

• apps/meteor/tests/e2e/e2e-encryption/e2ee-file-encryption.spec.ts

apps/meteor/tests/e2e/**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)

apps/meteor/tests/e2e/**/*.{ts,tsx,js,jsx}: Write concise, technical TypeScript/JavaScript with accurate typing
Follow DRY by extracting reusable logic into helper functions or page objects
Avoid code comments in the implementation

Files:

• apps/meteor/tests/e2e/e2e-encryption/e2ee-file-encryption.spec.ts

apps/meteor/tests/e2e/**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)

apps/meteor/tests/e2e/**/*.{ts,tsx}: Avoid using page.locator(); prefer semantic locators like page.getByRole, page.getByLabel, page.getByText, and page.getByTitle
Store commonly used locators in variables/constants for reuse
Use page.waitFor() with specific conditions and avoid hardcoded timeouts
Implement proper wait strategies for dynamic content
Follow the Page Object Model pattern consistently

Files:

• apps/meteor/tests/e2e/e2e-encryption/e2ee-file-encryption.spec.ts

🧠 Learnings (10)

📚 Learning: 2025-09-16T22:08:51.490Z

Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use descriptive test names that clearly communicate expected behavior

Applied to files:

• apps/meteor/tests/e2e/e2e-encryption/e2ee-file-encryption.spec.ts

📚 Learning: 2025-09-16T22:08:51.490Z

Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Group related tests in the same file

Applied to files:

• apps/meteor/tests/e2e/e2e-encryption/e2ee-file-encryption.spec.ts

📚 Learning: 2025-09-16T22:08:51.490Z

Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Ensure tests run reliably in parallel without shared state conflicts

Applied to files:

• apps/meteor/tests/e2e/e2e-encryption/e2ee-file-encryption.spec.ts

📚 Learning: 2025-09-16T22:08:51.490Z

Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.{ts,tsx,js,jsx} : Write concise, technical TypeScript/JavaScript with accurate typing

Applied to files:

• apps/meteor/tests/e2e/e2e-encryption/e2ee-file-encryption.spec.ts

📚 Learning: 2025-09-16T22:08:51.490Z

Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : All Playwright test files must be located under apps/meteor/tests/e2e/ and use the .spec.ts extension (e.g., login.spec.ts)

Applied to files:

• apps/meteor/tests/e2e/e2e-encryption/e2ee-file-encryption.spec.ts

📚 Learning: 2025-09-16T22:08:51.490Z

Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Maintain test isolation between test cases

Applied to files:

• apps/meteor/tests/e2e/e2e-encryption/e2ee-file-encryption.spec.ts

📚 Learning: 2025-09-16T22:08:51.490Z

Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Ensure a clean state for each test execution

Applied to files:

• apps/meteor/tests/e2e/e2e-encryption/e2ee-file-encryption.spec.ts

📚 Learning: 2025-09-16T22:08:51.490Z

Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.{ts,tsx,js,jsx} : Avoid code comments in the implementation

Applied to files:

• apps/meteor/tests/e2e/e2e-encryption/e2ee-file-encryption.spec.ts

📚 Learning: 2025-09-16T22:08:51.490Z

Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use test.step() to organize complex test scenarios

Applied to files:

• apps/meteor/tests/e2e/e2e-encryption/e2ee-file-encryption.spec.ts

📚 Learning: 2025-09-16T22:08:51.490Z

Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Utilize Playwright fixtures (test, page, expect) consistently

Applied to files:

• apps/meteor/tests/e2e/e2e-encryption/e2ee-file-encryption.spec.ts

🔇 Additional comments (2)

apps/meteor/tests/e2e/e2e-encryption/e2ee-file-encryption.spec.ts (1)

41-62: LGTM! Test structure and coverage look good.

The updated test name accurately reflects the remaining functionality, and the test steps properly verify file encryption with descriptions. The test follows Playwright best practices with clear step organization and web-first assertions.

apps/meteor/client/lib/parseMessageTextToAstMarkdown.ts (1)

79-83: Verification complete: All encrypted attachment references successfully removed.

The codebase-wide search confirms that both isEncryptedMessageAttachment and EncryptedMessageAttachment have been entirely removed—no orphaned imports, no lingering usage patterns. The file under review is clean, with imports containing only relevant attachment types from core-typings. The code at lines 79–83 correctly implements the simplified logic for file attachment description handling.

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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