Show errors outside of the upload modal by andreslucena · Pull Request #15981 · decidim/decidim

andreslucena · 2026-02-03T11:56:11Z

🎩 What? Why?

Related to the changes that I'm proposing at #15980, if there's an error in the form related to the modal, this error isn't shown.

📌 Related Issues

Testing

Go to the new attachments form
Do not fill any field
Click in the "Create attachment" button
See the error

📷 Screenshots

Before

After

♥️ Thank you!

Summary by CodeRabbit

Bug Fixes
- File upload validation now uses HTML5 validation, improving reliability and browser-native error handling.
- Validation errors for required uploads are shown alongside other form errors (outside the modal) for consistent display.
- Help text and error messages for upload fields are rendered consistently after the upload control.

coderabbitai · 2026-02-03T11:56:33Z

📝 Walkthrough

Walkthrough

Prevents validation error messages from being rendered inside file upload modals by adding a hidden validation input to the modal and moving the error-element composition to the form builder, linking errors to the main form via a data attribute.

Changes

Cohort / File(s)	Summary
Modal implementation `decidim-core/app/cells/decidim/upload_modal_cell.rb`	Adds a hidden validation input with an id from `validation_field_id` and removes inline rendering of error/help elements from the modal.
Form builder `decidim-core/lib/decidim/form_builder.rb`	`upload` now composes the modal plus error/help HTML; `abide_error_element` signature updated to `(..., options = {})` and supports `data-form-error-for` to target the validation field.
Modal specs `decidim-core/spec/cells/decidim/upload_modal_cell_spec.rb`	Adds tests asserting the hidden validation input is present/hidden and that `.form-error` and `.help-text` are not rendered inside the modal when required.
Form builder specs `decidim-core/spec/lib/form_builder_spec.rb`	Adds tests for upload field: rendering of form errors, help text placement, and presence of HTML5 validation error element with `data-form-error-for` linking to the upload field.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested reviewers

alecslupu
github-actions

Poem

I hid a tiny checkbox beneath the moonlit form,
so modals stay tidy and errors keep warm.
The field points outside with a gentle cheer,
“Look here,” it whispers, “the message is near.”
Hop, hop — correctness saved by a rabbit's charm 🐇✨

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 37.50% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Linked Issues check	❓ Inconclusive	The code changes implement the core requirement from `#15892` and `#11678` by moving validation errors outside the modal using a validation field approach. However, the PR does not address `#10357` regarding Assembly import attachments/categories handling.	Verify whether `#10357` (Assembly import handling) is intended to be fixed in this PR or should be addressed separately, as the current changes do not reference Assembly import logic.

✅ 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 'Show errors outside of the upload modal' clearly and concisely describes the primary change: moving validation errors outside the modal to the main form area.
Out of Scope Changes check	✅ Passed	All changes are scoped to the upload modal and form builder for handling validation errors outside the modal. No unrelated modifications detected.

_{✏️ 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
Commit unit tests in branch fix/upload-errors

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

Copilot

Pull request overview

This change ensures that validation errors related to upload fields (e.g., “cannot be blank”) are rendered outside the upload modal in the main form area, so admins can see and act on them without needing the modal open. It also aligns the upload field’s HTML5 validation wiring (hidden checkbox + error span) with Foundation Abide’s expectations and adds tests around these behaviors.

Changes:

Updated Decidim::FormBuilder#upload to append error_and_help_text and an abide_error_element linked to a hidden validation field, so errors render outside the modal and hook correctly into Foundation Abide.
Refactored Decidim::UploadModalCell#input_validation_field to only emit the hidden checkbox (no error/help markup), and added an explicit validation_field_id used by the form builder.
Added RSpec coverage for upload builder error/help text rendering and for the upload modal cell’s hidden validation checkbox, ID, and absence of errors/help-text inside the modal.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File	Description
decidim-core/lib/decidim/form_builder.rb	Extends `#upload` to render error/help text and a Foundation Abide error span outside the modal; enhances `abide_error_element` to accept a `:for` option and wire `data-form-error-for` to the hidden validation field.
decidim-core/app/cells/decidim/upload_modal_cell.rb	Simplifies `input_validation_field` to a hidden checkbox with a stable `id` (`attribute_validation`) and documents that validation errors now display in the main form area instead of inside the modal.
decidim-core/spec/lib/form_builder_spec.rb	Adds tests ensuring upload error messages and help text are rendered (and localized) after the upload cell, and that required upload fields get an HTML5 error element bound to the validation field’s ID.
decidim-core/spec/cells/decidim/upload_modal_cell_spec.rb	Adds tests verifying the required-field hidden checkbox’s ID for Abide, the required indicator, and that no `.form-error` or `.help-text` appear inside the upload modal itself.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

decidim-core/lib/decidim/form_builder.rb

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

andreslucena · 2026-02-05T06:49:58Z

This is ready for your review @alecslupu

alecslupu

👍

andreslucena added 2 commits February 3, 2026 10:58

Show errors outside of the upload modal

6df39c0

Show the required attribute error when it is in the upload button

92187ca

andreslucena added the type: fix PRs that implement a fix for a bug label Feb 3, 2026

Copilot AI review requested due to automatic review settings February 3, 2026 11:56

Copilot started reviewing on behalf of andreslucena February 3, 2026 11:56 View session

github-actions bot added the module: core label Feb 3, 2026

github-actions bot previously approved these changes Feb 3, 2026

View reviewed changes

Copilot AI reviewed Feb 3, 2026

View reviewed changes

decidim-core/lib/decidim/form_builder.rb Outdated Show resolved Hide resolved

Apply suggestions from code review

0092d4a

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

andreslucena dismissed github-actions[bot]’s stale review via 0092d4a February 4, 2026 14:54

github-actions bot approved these changes Feb 4, 2026

View reviewed changes

andreslucena assigned alecslupu Feb 5, 2026

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 labels Feb 6, 2026

alecslupu approved these changes Feb 6, 2026

View reviewed changes

alecslupu merged commit 4e78dfd into develop Feb 6, 2026
93 of 97 checks passed

alecslupu deleted the fix/upload-errors branch February 6, 2026 01:00

This was referenced Feb 6, 2026

Backport 'Show errors outside of the upload modal' to v0.31 #16009

Merged

Backport 'Show errors outside of the upload modal' to v0.30 #16010

Merged

coderabbitai bot mentioned this pull request Feb 6, 2026

Attachment not persisting on error. #15938

Merged

This was referenced Feb 19, 2026

Fix edit proposal with attachment #16094

Merged

Fix error state is not visible when translation is missing #16157

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Show errors outside of the upload modal#15981

Show errors outside of the upload modal#15981
alecslupu merged 3 commits intodevelopfrom
fix/upload-errors

andreslucena commented Feb 3, 2026 •

edited by coderabbitai bot

Loading

Uh oh!

coderabbitai bot commented Feb 3, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Suggested reviewers

Poem

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

andreslucena commented Feb 5, 2026

Uh oh!

alecslupu left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

andreslucena commented Feb 3, 2026 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🎩 What? Why?

📌 Related Issues

Testing

📷 Screenshots

Before

After

Summary by CodeRabbit

Uh oh!

coderabbitai bot commented Feb 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Suggested reviewers

Poem

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

andreslucena commented Feb 5, 2026

Uh oh!

alecslupu left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

andreslucena commented Feb 3, 2026 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Feb 3, 2026 •

edited

Loading