Skip to content

Fix allow admins deleting attachments with links#15994

Merged
alecslupu merged 3 commits intodecidim:developfrom
PopulateTools:fix/allow_deleting_link_attachments
Feb 6, 2026
Merged

Fix allow admins deleting attachments with links#15994
alecslupu merged 3 commits intodecidim:developfrom
PopulateTools:fix/allow_deleting_link_attachments

Conversation

@entantoencuanto
Copy link
Copy Markdown
Contributor

@entantoencuanto entantoencuanto commented Feb 4, 2026 •
edited by coderabbitai bot
Loading

🎩 What? Why?

This PR

  • Prevents calling the set_link_content_type_and_size callback of the Decidim::Attachment model if the instance is not editable because is in frozen state or destroyed.
  • This change allows admins to delete attachments with links from the admin panel
  • Adds a test to check the fix.

📌 Related Issues

Summary by CodeRabbit

  • Bug Fixes

    • Improved stability when deleting attachments that contain links and ensured clear success feedback.
    • Avoided unnecessary metadata processing for non-editable link attachments to reduce validation issues.

  • Tests

    • Added end-to-end tests validating deletion of attachments with links and confirming their removal.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Feb 4, 2026 •
edited
Loading

📝 Walkthrough

Walkthrough

Change guards link metadata initialization during validation for attachments so it doesn't run on destroyed/frozen records; adds an editable_link? predicate and tests that verify admins can delete attachments with links.

Changes

Cohort / File(s) Summary
Core Model
decidim-core/app/models/decidim/attachment.rb		 Modify before_validation to run set_link_content_type_and_size only when editable_link? is true; add public editable_link? which checks link? && !destroyed? && !frozen?.
Admin Tests
decidim-admin/lib/decidim/admin/test/manage_attachments_examples.rb		 Add attachment_with_link fixture/trait and two specs that delete an attachment with a link via the UI dropdown, confirm deletion, expect a success callout, and verify the attachment is removed from the list.

Sequence Diagram(s)

(omitted)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested reviewers

  • alecslupu
  • froger

Poem

🐰 I found a link that froze the show,
I learned when to skip and when to go,
editable_link? clears the way,
Now deletions hop without dismay,
Hooray for smooth admin flow! 🥕

🚥 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 'Fix allow admins deleting attachments with links' accurately reflects the main objective of the PR: enabling admins to delete attachments that have links.
Linked Issues check ✅ Passed The PR addresses the core requirement from issue #15993 by preventing the callback from running on frozen/destroyed instances, allowing admins to delete link attachments without FrozenError while preserving link handling on create/update.
Out of Scope Changes check ✅ Passed All changes are directly scoped to fixing the deletion issue: modifying the callback condition in Attachment model and adding test coverage for the deletion scenario.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

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

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

@github-actions github-actions bot added module: core module: admin type: fix PRs that implement a fix for a bug labels Feb 4, 2026
github-actions[bot]
github-actions bot previously approved these changes Feb 4, 2026
View reviewed changes
@entantoencuanto entantoencuanto force-pushed the fix/allow_deleting_link_attachments branch from b93e79d to 01cd9bc Compare February 4, 2026 21:00
github-actions[bot]
github-actions bot previously approved these changes Feb 4, 2026
View reviewed changes
This was referenced Feb 5, 2026
Closed
Merged
@alecslupu alecslupu self-assigned this Feb 6, 2026
@alecslupu alecslupu added release: v0.30 Issues or PRs that need to be tackled for v0.30 release: v0.31 Issues or PRs that need to be tackled for v0.31 and removed release: v0.30 Issues or PRs that need to be tackled for v0.30 labels Feb 6, 2026
alecslupu
alecslupu approved these changes Feb 6, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@alecslupu alecslupu left a comment

Choose a reason for hiding this comment

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

👍

@alecslupu alecslupu merged commit 332f474 into decidim:develop Feb 6, 2026
92 of 96 checks passed
This was referenced Feb 21, 2026
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@github-actions github-actions[bot] github-actions[bot] approved these changes

+1 more reviewer

@alecslupu alecslupu alecslupu approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@alecslupu alecslupu

Labels

module: admin module: core release: v0.30 Issues or PRs that need to be tackled for v0.30 release: v0.31 Issues or PRs that need to be tackled for v0.31 type: fix PRs that implement a fix for a bug

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Admins are unable to delete attachments with links

2 participants

@entantoencuanto @alecslupu