MM-63792: allow duplicating boards with empty file/attachment blocks

[nang2049]

Summary

Duplicating a board failed with 400 Block ID cannot be empty when any image or attachment block had an empty fileId or attachmentId. The message came from ValidateFileId("") inside Block.IsValid() and CopyCardFilesaborting the whole duplicate. Fix by treat an empty file reference as "no file attached"

Ticket Link

https://mattermost.atlassian.net/browse/MM-63792

Manual QA

1. Create a board, add a card with an image or attachment element ans upload a file.
2. Remove the file from the element then refresh.
3. Board menu → Duplicate board
   • Before: red error duplicate fails.
   • After: <name> copy appears in the sidebar and opens.

Change Impact: 🟡 Medium

Regression Risk: Changes affect shared block validation and board duplication flows, but behavior is narrowly scoped to empty file references and covered by automated tests.
QA Recommendation: Manual QA is recommended for duplicating boards with removed image or attachment files.

Generated by CodeRabbitAI

[coderabbitai]

📝 Walkthrough

Walkthrough

Empty fileId and attachmentId values are now treated as unattached blocks. Validation skips empty identifiers, while card file copying falls back to attachmentId and skips blocks lacking both IDs.

Changes

File ID handling

Layer / File(s)	Summary
Allow empty attachment identifiers server/model/block.go, server/model/block_test.go	Block validation skips file ID checks for empty identifiers and verifies acceptance for image and attachment blocks.
Fallback during card file copying server/app/files.go, server/app/files_test.go	Card file copying uses attachmentId when fileId is empty and skips blocks without either identifier; the corresponding test now expects success.

Estimated code review effort: 2 (Simple) | ~10 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: allowing board duplication when image or attachment blocks have empty file references.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.

✨ 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 MM-63792

---

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

[coderabbitai]

🧹 Nitpick comments (1)

server/app/files_test.go (1)

675-681: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Cover the attachment-only fallback path.

This test only verifies an empty fileId with no attachmentId, so it exercises the skip path. Add a case with an empty fileId and valid attachmentId that asserts the attachment ID is copied and patched successfully.

🤖 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 `@server/app/files_test.go` around lines 675 - 681, Extend the test around
CopyAndUpdateCardFiles to cover a block with an empty fileId and a valid
attachmentId, verifying the attachment ID is copied and the corresponding file
backend patch succeeds. Keep the existing empty-file-and-attachment skip case,
and assert the expected backend interaction and resulting block state for the
attachment-only fallback path.

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

Nitpick comments:
In `@server/app/files_test.go`:
- Around line 675-681: Extend the test around CopyAndUpdateCardFiles to cover a
block with an empty fileId and a valid attachmentId, verifying the attachment ID
is copied and the corresponding file backend patch succeeds. Keep the existing
empty-file-and-attachment skip case, and assert the expected backend interaction
and resulting block state for the attachment-only fallback path.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 599ef2c2-38f7-4684-a1d9-6b6565269fb2

📥 Commits

Reviewing files that changed from the base of the PR and between 261bc51 and dea686a.

📒 Files selected for processing (4)

• server/app/files.go
• server/app/files_test.go
• server/model/block.go
• server/model/block_test.go

[avasconcelos114]

LGTM!

[ogi-m]

I couldn't repro the original issue so I investigated with Claude and apparently it's only reproducible with legacy data:

The fileId validation in baseValidations() was introduced on 22 Jan 2025. Since then, no new block can be saved with fileId = "" through any normal API path. The bug exclusively hits boards created before that validation existed, where a user started (or abandoned) an image/attachment upload and the empty string got persisted.