Make the document a required field in spaces import by andreslucena · Pull Request #15980 · decidim/decidim

andreslucena · 2026-02-03T08:13:14Z

🎩 What? Why?

While working with #15895, I'm trying to make the import assembly feature work but I'm finding too many bugs in the process.

This PR is the first of a series to make this feature work with a bit lower number of bugs.

The first two that I found are related to the document upload field itself.

on assemblies, if an admin goes to this form and doesn't actually uploads the document, then it'll have a 500 error. The fix is to add the validation in the form object
on proceses, if an admin goes to this form and doesn't actually uploads the document, then it doesn't show any error. The fix is to remove the required HTML5 attribute from this form, as it doesn't play nice with the upload that we have. this is actually related to another change that I'm doing, you can ignore this for now

Testing

For assemblies:

Sign in as admin
Go to http://localhost:3000/admin/assemblies/imports
Fill the title and the slug - do NOT upload the document
Click in Import
(Before the patch) See server-side error
(After the patch) See error in the form

Stacktrace

TypeError (no implicit conversion of nil into String):

json (2.18.0) lib/json/common.rb:353:in 'JSON::Ext::Parser.parse'
json (2.18.0) lib/json/common.rb:353:in 'JSON.parse'
/workspaces/decidim/decidim-assemblies/app/commands/decidim/assemblies/admin/import_assembly.rb:56:in 'Decidim::Assemblies::Admin::ImportAssembly#document_parsed'
/workspaces/decidim/decidim-assemblies/app/commands/decidim/assemblies/admin/import_assembly.rb:52:in 'Decidim::Assemblies::Admin::ImportAssembly#assemblies'
/workspaces/decidim/decidim-assemblies/app/commands/decidim/assemblies/admin/import_assembly.rb:41:in 'Decidim::Assemblies::Admin::ImportAssembly#import_assembly'
/workspaces/decidim/decidim-assemblies/app/commands/decidim/assemblies/admin/import_assembly.rb:28:in 'block in Decidim::Assemblies::Admin::ImportAssembly#call'
activerecord (7.2.2.2) lib/active_record/connection_adapters/abstract/transaction.rb:616:in 'block in ActiveRecord::ConnectionAdapters::TransactionManager#within_new_transaction'
activesupport (7.2.2.2) lib/active_support/concurrency/null_lock.rb:9:in 'ActiveSupport::Concurrency::NullLock#synchronize'
activerecord (7.2.2.2) lib/active_record/connection_adapters/abstract/transaction.rb:613:in 'ActiveRecord::ConnectionAdapters::TransactionManager#within_new_transaction'
activerecord (7.2.2.2) lib/active_record/connection_adapters/abstract/database_statements.rb:361:in 'ActiveRecord::ConnectionAdapters::DatabaseStatements#transaction'
activerecord (7.2.2.2) lib/active_record/transactions.rb:234:in 'block in ActiveRecord::Transactions::ClassMethods#transaction'
activerecord (7.2.2.2) lib/active_record/connection_adapters/abstract/connection_pool.rb:421:in 'ActiveRecord::ConnectionAdapters::ConnectionPool#with_connection'
activerecord (7.2.2.2) lib/active_record/connection_handling.rb:296:in 'ActiveRecord::ConnectionHandling#with_connection'
activerecord (7.2.2.2) lib/active_record/transactions.rb:233:in 'ActiveRecord::Transactions::ClassMethods#transaction'
/workspaces/decidim/decidim-core/lib/decidim/command.rb:30:in 'Decidim::Command#transaction'
/workspaces/decidim/decidim-assemblies/app/commands/decidim/assemblies/admin/import_assembly.rb:27:in 'Decidim::Assemblies::Admin::ImportAssembly#call'
/workspaces/decidim/decidim-core/lib/decidim/command.rb:19:in 'Decidim::Command.call'
/workspaces/decidim/decidim-assemblies/app/controllers/decidim/assemblies/admin/assembly_imports_controller.rb:18:in 'Decidim::Assemblies::Admin::AssemblyImportsController#create'
actionpack (7.2.2.2) lib/action_controller/metal/basic_implicit_render.rb:8:in 'ActionController::BasicImplicitRender#send_action'
actionpack (7.2.2.2) lib/abstract_controller/base.rb:226:in 'AbstractController::Base#process_action'
actionpack (7.2.2.2) lib/action_controller/metal/rendering.rb:193:in 'ActionController::Rendering#process_action'
actionpack (7.2.2.2) lib/abstract_controller/callbacks.rb:261:in 'block in AbstractController::Callbacks#process_action'
activesupport (7.2.2.2) lib/active_support/callbacks.rb:121:in 'block in ActiveSupport::Callbacks#run_callbacks'
activesupport (7.2.2.2) lib/active_support/core_ext/time/zones.rb:65:in 'Time.use_zone'
/workspaces/decidim/decidim-core/app/controllers/concerns/decidim/use_organization_time_zone.rb:21:in 'Decidim::Admin::ApplicationController#use_organization_time_zone'

📷 Screenshots

Before

After

♥️ Thank you!

Summary by CodeRabbit

Bug Fixes
- Import flows now enforce that a valid document must be provided server-side; submissions without a proper document are rejected with clear errors.
- The import form UI no longer marks the document field as client-side required (server validation still applies).
Tests
- Added comprehensive tests covering document, slug, title and import-option validations.

coderabbitai · 2026-02-03T08:13:40Z

📝 Walkthrough

Walkthrough

Adds document presence validation to two import form classes, introduces matching RSpec test suites, and removes the required: true attribute from the participatory process import form view input.

Changes

Cohort / File(s)	Summary
Assembly Import Form `decidim-assemblies/app/forms/decidim/assemblies/admin/assembly_import_form.rb`	Adds `presence: true` validation for the `document` attribute.
Assembly Import Form Tests `decidim-assemblies/spec/forms/decidim/assemblies/admin/assembly_import_form_spec.rb`	New RSpec suite covering valid submission and failure cases (missing document, wrong content type, missing/invalid/non-unique slug, missing default-language title).
Participatory Process Import Form `decidim-participatory_processes/app/forms/decidim/participatory_processes/admin/participatory_process_import_form.rb`	Adds `presence: true` validation for the `document` attribute.
Participatory Process Import Form Tests `decidim-participatory_processes/spec/forms/decidim/participatory_processes/admin/participatory_process_import_form_spec.rb`	New RSpec suite mirroring assembly tests for document presence, content type, slug validation, uniqueness, and multilingual title requirements.
Participatory Process Import View `decidim-participatory_processes/app/views/decidim/participatory_processes/admin/participatory_process_imports/_form.html.erb`	Removed `required: true` from the document upload field in the form markup.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

🐰 I hopped in with forms anew,
Paper files now must come through,
Tests that check each careful part—
A tiny change, a big-hearted start. 🥕

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and accurately describes the main change: adding document as a required field in spaces (assemblies and participatory processes) import forms. It is concise and specific enough for quick understanding.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ 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/import-space-required-document

Important

Action Needed: IP Allowlist Update

If your organization protects your Git platform with IP whitelisting, please add the new CodeRabbit IP address to your allowlist:

✨ 136.113.208.247/32 (new)
34.170.211.100/32
35.222.179.152/32

Failure to add the new IP will result in interrupted reviews.

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 PR prevents a 500 error during participatory space imports when the admin submits the import form without uploading a document, by making the uploaded JSON document required at the form-validation level.

Changes:

Add presence: true validation for the document attribute in participatory process and assembly import forms.
Add new form specs covering valid input, missing document, invalid document content type, and slug/title validation cases for both imports.

Reviewed changes

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

File	Description
decidim-participatory_processes/app/forms/decidim/participatory_processes/admin/participatory_process_import_form.rb	Requires an import document to be present, preventing imports from proceeding with a missing file.
decidim-participatory_processes/spec/forms/decidim/participatory_processes/admin/participatory_process_import_form_spec.rb	Adds form-level regression coverage for missing/invalid documents and slug/title validation.
decidim-assemblies/app/forms/decidim/assemblies/admin/assembly_import_form.rb	Requires an import document to be present, preventing imports from proceeding with a missing file.
decidim-assemblies/spec/forms/decidim/assemblies/admin/assembly_import_form_spec.rb	Adds form-level regression coverage for missing/invalid documents and slug/title validation.

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

decidim-assemblies/app/forms/decidim/assemblies/admin/assembly_import_form.rb

alecslupu

👍 - Merging with QLTY failing pipeline.

Make the document a required field in spaces import

a5fcb17

Copilot AI review requested due to automatic review settings February 3, 2026 08:13

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

Copilot started reviewing on behalf of andreslucena February 3, 2026 08:13 View session

github-actions bot added module: assemblies module: participatory processes labels 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-assemblies/app/forms/decidim/assemblies/admin/assembly_import_form.rb Show resolved Hide resolved

Remove required attribute from upload field's processes form

1d37e90

andreslucena dismissed github-actions[bot]’s stale review via 1d37e90 February 3, 2026 08:29

github-actions bot approved these changes Feb 3, 2026

View reviewed changes

andreslucena mentioned this pull request Feb 3, 2026

Importing Assemblies #10357

Closed

andreslucena assigned alecslupu Feb 3, 2026

alecslupu approved these changes Feb 3, 2026

View reviewed changes

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 3, 2026

andreslucena mentioned this pull request Feb 3, 2026

Show errors outside of the upload modal #15981

Merged

alecslupu merged commit b0778c9 into develop Feb 3, 2026
65 of 70 checks passed

alecslupu deleted the fix/import-space-required-document branch February 3, 2026 12:02

This was referenced Feb 3, 2026

Backport 'Make the document a required field in spaces import' to v0.31 #15982

Merged

Backport 'Make the document a required field in spaces import' to v0.30 #15983

Merged

coderabbitai bot mentioned this pull request Feb 3, 2026

Fix markup and UX between the spaces imports #15984

Merged

alecslupu pushed a commit that referenced this pull request Feb 3, 2026

Make the document a required field in spaces import (#15980)

e7483df

coderabbitai bot mentioned this pull request Feb 3, 2026

Finishing touches with regard to header improvements #15683

Merged

6 tasks

coderabbitai bot mentioned this pull request Feb 19, 2026

Add validation for the import space with empty JSON #16148

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Make the document a required field in spaces import#15980

Make the document a required field in spaces import#15980
alecslupu merged 2 commits intodevelopfrom
fix/import-space-required-document

andreslucena commented Feb 3, 2026 •

edited

Loading

Uh oh!

coderabbitai bot commented Feb 3, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Poem

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

alecslupu left a comment •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

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

Uh oh!

🎩 What? Why?

Testing

Stacktrace

📷 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

Poem

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

alecslupu left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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

Loading

coderabbitai bot commented Feb 3, 2026 •

edited

Loading

alecslupu left a comment •

edited

Loading