Upgrade to version 0.31#132
Open
ElviaBth wants to merge 21 commits into
Open
Conversation
davidbeig
requested changes
Feb 25, 2026
There was a problem hiding this comment.
It's a really god job @ElviaBth !!
Tests are good, and the upgrade seems to work well. There are some changes and comments I've made in the review.
Take care of them and then we can continue!
The data-component="accordion" is deprecated also. Check for this ones also.
davidbeig
reviewed
Feb 25, 2026
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
There was a problem hiding this comment.
Pull request overview
Upgrades the participatory documents module to be compatible with Decidim 0.31, including the Decidim-side terminology shift from “valuator” to “evaluator” and the Webpacker → Shakapacker frontend integration changes.
Changes:
- Bump gem/module + JS dependencies and CI runtimes to Decidim 0.31 / Ruby 3.3 / Node 22.
- Replace “valuation/valuator” assignment flows with “evaluation/evaluator” (models, permissions, controllers, views, locales, specs).
- Update PDF viewer / suggestion form UI + JS integration to match Decidim 0.31 frontend patterns.
Reviewed changes
Copilot reviewed 78 out of 80 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| spec/system/user_interacts_with_pdf_viewer_spec.rb | Adjusts system interactions/selectors for updated suggestion forms. |
| spec/system/index_participatory_documents_spec.rb | Updates PDF viewer assertion target to new container. |
| spec/system/admin/admin_manages_suggestions_spec.rb | Aligns admin UI expectations (export, filters, attachments) with 0.31. |
| spec/system/admin/admin_manages_suggestion_valuators_spec.rb | Removes old “valuator” system spec. |
| spec/system/admin/admin_manages_suggestion_evaluators_spec.rb | Adds new “evaluator” system spec coverage. |
| spec/system/admin/admin_manages_participatory_documents_spec.rb | Uses translated virus error message. |
| spec/serializers/suggestion_serialiser_spec.rb | Updates serializer expectations: author_name + evaluators. |
| spec/serializers/my_suggestion_serialiser_spec.rb | Updates serializer expectations: author_name + evaluators key absence. |
| spec/permissions/decidim/participatory_documents/admin/permissions_spec.rb | Renames permission examples/actions to evaluator equivalents. |
| spec/models/decidim/participatory_documents/valuation_assignment_spec.rb | Removes old valuation assignment model spec. |
| spec/models/decidim/participatory_documents/evaluator_components_spec.rb | Renames role config spec from Valuator to Evaluator. |
| spec/models/decidim/participatory_documents/evaluation_assignment_spec.rb | Adds evaluation assignment model spec. |
| spec/models/decidim/participatory_documents/document_spec.rb | Fixes model reload pathing under new load behavior. |
| spec/lib/module_config_spec.rb | Makes shell command execution stricter (capture stderr + fail on nonzero). |
| spec/lib/component_spec.rb | Renames wording from valuators to evaluators. |
| spec/jobs/export_my_suggestions_job_spec.rb | Updates export mailer expectations/signature for Decidim 0.31. |
| spec/controllers/decidim/participatory_documents/section_suggestions_controller_spec.rb | Removes explicit engine routing setup. |
| spec/controllers/decidim/participatory_documents/document_suggestions_controller_spec.rb | Removes explicit engine routing setup. |
| spec/controllers/decidim/participatory_documents/admin/section_suggestions_controller_spec.rb | Removes explicit admin engine routing setup. |
| spec/controllers/decidim/participatory_documents/admin/evaluation_assignments_controller_spec.rb | Ports controller spec from valuation→evaluation assignments. |
| spec/controllers/decidim/participatory_documents/admin/annotations_controller_spec.rb | Removes explicit admin engine routing setup. |
| spec/commands/decidim/participatory_documents/admin/assign_suggestion_to_evaluator_spec.rb | Updates command spec to evaluator-based assignment. |
| package.json | Bumps Decidim JS packages to 0.31; adds pdfjs-dist; bumps module version. |
| lib/tasks/decidim_participatory_documents.rake | Switches upgrade hook from webpacker→shakapacker. |
| lib/decidim/participatory_documents/version.rb | Bumps module version + Decidim compatibility range to 0.31.x. |
| lib/decidim/participatory_documents/test/factories.rb | Updates factories for ActiveStorage attach + evaluator assignment factory rename. |
| lib/decidim/participatory_documents/engine.rb | Updates role override include to Evaluator. |
| lib/decidim/participatory_documents/component.rb | Updates stats registry priority example for new enum. |
| lib/decidim/participatory_documents/admin_engine.rb | Renames routes to evaluation assignments; removes duplicate icon registration. |
| decidim-participatory_documents.gemspec | Requires Ruby >= 3.3 and adjusts dev dep lint rule. |
| db/migrate/20221115233024_create_decidim_participatory_documents_valuation_assignments.rb | Removes old valuation assignments migration. |
| db/migrate/20221115233024_create_decidim_participatory_documents_evaluation_assignments.rb | Adds evaluation assignments migration. |
| config/locales/en.yml | Renames valuator strings to evaluator + adds evaluation assignment texts + virus message. |
| config/assets.rb | Switches asset registration from Decidim::Webpacker to Decidim::Shakapacker. |
| app/views/decidim/participatory_documents/section_suggestions/_form.html.erb | Uses EngineRouter main proxy for correct mounted paths. |
| app/views/decidim/participatory_documents/document_suggestions/_form.html.erb | Uses EngineRouter main proxy for correct mounted paths. |
| app/views/decidim/participatory_documents/admin/suggestions/show.html.erb | Switches valuators partial to evaluators partial. |
| app/views/decidim/participatory_documents/admin/suggestions/index.html.erb | Updates header/actions layout + evaluator bulk action partials + column label. |
| app/views/decidim/participatory_documents/admin/suggestions/bulk_actions/_unassign_from_valuator.html.erb | Removes valuator unassign bulk action partial. |
| app/views/decidim/participatory_documents/admin/suggestions/bulk_actions/_unassign_from_evaluator.html.erb | Adds evaluator unassign bulk action partial. |
| app/views/decidim/participatory_documents/admin/suggestions/bulk_actions/_evaluators_picker.html.erb | Adds evaluator multi-select picker partial. |
| app/views/decidim/participatory_documents/admin/suggestions/bulk_actions/_dropdown.html.erb | Reworks dropdown markup and actions to evaluator naming. |
| app/views/decidim/participatory_documents/admin/suggestions/bulk_actions/_assign_to_valuator.html.erb | Removes valuator assign bulk action partial. |
| app/views/decidim/participatory_documents/admin/suggestions/bulk_actions/_assign_to_evaluator.html.erb | Adds evaluator assign bulk action partial. |
| app/views/decidim/participatory_documents/admin/suggestions/_suggestion.html.erb | Uses author_name + evaluation assignment display. |
| app/views/decidim/participatory_documents/admin/suggestions/_evaluators.html.erb | Replaces valuators panel with evaluators panel + routes/permissions updated. |
| app/views/decidim/participatory_documents/admin/suggestions/_bulk-actions.html.erb | Removes embedded assign/unassign partial renders (moved elsewhere). |
| app/views/decidim/participatory_documents/admin/suggestions/_add_evaluators.html.erb | Ports single-suggestion assignment UI to evaluator naming/routes. |
| app/views/decidim/participatory_documents/admin/suggestion_notes/_suggestion_notes.html.erb | Updates accordion hook to stimulus-style controller attribute. |
| app/serializers/decidim/participatory_documents/suggestion_serializer.rb | Switches author field and valuators→evaluators serialization. |
| app/serializers/decidim/participatory_documents/my_suggestion_serializer.rb | Switches author field to author_name. |
| app/permissions/decidim/participatory_documents/admin/permissions.rb | Ports valuator permission logic to evaluator logic + new action names. |
| app/packs/src/decidim/participatory_documents/upload_modal.js | Updates form selectors for new modal/form structure. |
| app/packs/src/decidim/participatory_documents/pdf/suggestion_form.js | Re-initializes upload fields after HTML replace + adds XHR headers + new form selectors. |
| app/packs/src/decidim/participatory_documents/admin/manage_bulk_suggestions.js | Adds TomSelect initialization and adapts bulk action handling for new UI. |
| app/models/decidim/participatory_documents/suggestion.rb | Replaces valuation assignments with evaluation assignments + new filterable/ransack hooks. |
| app/models/decidim/participatory_documents/evaluation_assignment.rb | Renames model and association to evaluator_role + presenter reference. |
| app/models/concerns/decidim/participatory_documents/participatory_space_user_role_override.rb | Updates dependent association cleanup from valuation→evaluation. |
| app/models/concerns/decidim/participatory_documents/evaluator_override.rb | Renames override module ValuatorOverride→EvaluatorOverride. |
| app/jobs/decidim/participatory_documents/export_my_suggestions_job.rb | Updates export mailer invocation to new Decidim export flow. |
| app/helpers/decidim/participatory_documents/admin/suggestion_helper.rb | Replaces valuator select helper with evaluator picker partial rendering. |
| app/helpers/decidim/participatory_documents/admin/button_helper.rb | Adjusts icon CSS class for updated UI. |
| app/forms/decidim/participatory_documents/admin/valuation_assignment_form.rb | Removes old valuator assignment form. |
| app/forms/decidim/participatory_documents/admin/evaluation_assignment_form.rb | Adds evaluator assignment form with multi-role selection. |
| app/controllers/decidim/participatory_documents/section_suggestions_controller.rb | Updates redirect paths to EngineRouter main proxy. |
| app/controllers/decidim/participatory_documents/document_suggestions_controller.rb | Updates redirect paths to EngineRouter main proxy. |
| app/controllers/decidim/participatory_documents/admin/valuation_assignments_controller.rb | Removes old valuation assignments controller. |
| app/controllers/decidim/participatory_documents/admin/suggestions_controller.rb | Ports filters, role detection, and assignment logic to evaluator equivalents. |
| app/controllers/decidim/participatory_documents/admin/evaluation_assignments_controller.rb | Adds controller for creating/destroying evaluator assignments (bulk + single). |
| app/commands/decidim/participatory_documents/admin/unassign_suggestions_from_evaluator.rb | Ports unassignment command to evaluator assignments + multi-role support. |
| app/commands/decidim/participatory_documents/admin/create_suggestion_note.rb | Renames comment indicating evaluator notifications. |
| app/commands/decidim/participatory_documents/admin/assign_suggestions_to_evaluator.rb | Ports assignment command to evaluator assignments + multi-role support. |
| README.md | Updates version compatibility table for Decidim 0.31. |
| Gemfile.lock | Updates dependencies to Decidim 0.31 / Rails 7.2 / Ruby 3.3 compatible set. |
| Gemfile | Adds decidim-initiatives for 0.31 test/dev stack. |
| .ruby-version | Bumps local Ruby version to 3.3.10. |
| .rubocop.yml | Switches to decidim-dev rubocop config and adjusts exclusions. |
| .github/workflows/test.yml | Updates CI Ruby/Node versions and apt packages list. |
| .github/workflows/lint.yml | Updates CI Ruby/Node versions for linting. |
Comments suppressed due to low confidence (1)
app/models/decidim/participatory_documents/evaluation_assignment.rb:14
log_presenter_class_forreferencesDecidim::ParticipatoryDocuments::AdminLog::EvaluationAssignmentPresenter, but there is no such presenter inapp/presenters/decidim/participatory_documents/admin_log/. This will raise aNameErrorwhen logs are rendered for evaluation assignment actions. Either add the presenter class (mirroring the other AdminLog presenters) or point to an existing presenter class that is actually defined.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
davidbeig
requested changes
Mar 18, 2026
davidbeig
left a comment
davidbeig
left a comment
There was a problem hiding this comment.
Thanks for adding the changes and reviewing the code rabbit suggestions!
I have left some comments now. After that we can finish this upgrade
Thanks <3
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
📝 WalkthroughWalkthroughTerminology changed from “valuator” to “evaluator” across code, views, tests and locales; Ruby/Node/Decidim versions bumped (Ruby 3.3.10, Node 22.14.0, Decidim 0.31); DB table/columns renamed for assignment model; new EvaluationAssignment model, form, controller and presenters added; various routing, asset and JS updates applied. Changes
Sequence Diagram(s)sequenceDiagram
participant Admin as Admin UI (browser)
participant Controller as EvaluationAssignmentsController
participant Form as EvaluationAssignmentForm
participant Command as Admin::AssignSuggestionsToEvaluator
participant Model as EvaluationAssignment (DB)
Admin->>Controller: POST /document/:id/evaluation_assignments (suggestion_ids, evaluator_role_ids)
Controller->>Form: build(params) / validate
Controller->>Controller: authorize per suggestion
Controller->>Command: call(form)
Command->>Model: find_or_create for each (suggestion, evaluator_role)
Model-->>Command: return assignment(s)
Command-->>Controller: result (:ok/:invalid)
Controller-->>Admin: redirect with flash notice/alert
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
app/packs/src/decidim/participatory_documents/admin/manage_bulk_suggestions.js (1)
120-120:⚠️ Potential issue | 🟡 MinorRemove debug
console.logstatement.This appears to be debug logging that should be removed before merging.
🧹 Proposed fix
- console.log(`.js-bulk-action-form .js-suggestion-id-${suggestionId}`);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/packs/src/decidim/participatory_documents/admin/manage_bulk_suggestions.js` at line 120, Remove the leftover debug console.log by deleting the line that prints `.js-bulk-action-form .js-suggestion-id-${suggestionId}` in manage_bulk_suggestions.js (the stray console.log call that references suggestionId); ensure no other console.debug/console.log statements remain in the same function or module so only intentional logging (if any) is left.spec/commands/decidim/participatory_documents/admin/assign_suggestion_to_evaluator_spec.rb (1)
53-59:⚠️ Potential issue | 🟡 MinorIncomplete terminology rename and unused variable.
This context still uses "valuator" terminology and defines
valuator_rolewhich shadows nothing—the form mock on line 28 referencesevaluator_role(defined on line 21), notvaluator_role. This test context doesn't actually test the "missing evaluator" scenario as intended.🔧 Proposed fix
- context "when valuator is missing" do - let(:valuator_role) { nil } + context "when evaluator is missing" do + let(:evaluator_role) { nil } it "is not valid" do expect { subject.call }.to broadcast(:invalid) end end🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@spec/commands/decidim/participatory_documents/admin/assign_suggestion_to_evaluator_spec.rb` around lines 53 - 59, The test context incorrectly uses "valuator" and defines let(:valuator_role) which doesn't override the form mock's evaluator_role; change the context description and the helper to use the correct terminology and variable name (replace let(:valuator_role) { nil } with let(:evaluator_role) { nil }) so the form mock referenced on line 28 sees a nil evaluator_role, and keep the example expectation (expect { subject.call }.to broadcast(:invalid)) as-is to validate the "missing evaluator" scenario.spec/system/admin/admin_manages_suggestions_spec.rb (1)
227-227:⚠️ Potential issue | 🟡 MinorRemove debug
putsstatement.Line 227 contains a debug statement that should be removed before merging.
Proposed fix
it "sorts ascendent" do - puts "first_section suggestions: #{first_section_suggestions}" expect(page).to have_content(translated_attribute(first_section.title))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@spec/system/admin/admin_manages_suggestions_spec.rb` at line 227, Remove the debug puts call that prints first_section_suggestions; specifically delete the line containing puts "first_section suggestions: #{first_section_suggestions}" in the spec (no replacement needed), then run the test suite to confirm nothing else relies on this output.
♻️ Duplicate comments (2)
app/serializers/decidim/participatory_documents/suggestion_serializer.rb (1)
27-27:⚠️ Potential issue | 🟠 MajorGuard against nil evaluators to prevent export failures.
The chained
map(&:evaluator).map(&:name)will raiseNoMethodErrorif anyevaluation_assignmenthas a nil evaluator (e.g., if the user was deleted). This can break CSV/JSON exports for data with orphaned assignments.🛡️ Proposed fix with nil safety
- evaluators: suggestion.evaluation_assignments.map(&:evaluator).map(&:name).join(", "), + evaluators: suggestion.evaluation_assignments.filter_map { |a| a.evaluator&.name }.join(", "),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/serializers/decidim/participatory_documents/suggestion_serializer.rb` at line 27, The current evaluators serialization uses suggestion.evaluation_assignments.map(&:evaluator).map(&:name).join(", ") which will raise if any evaluation_assignment has a nil evaluator; update the evaluators construction in SuggestionSerializer to safely handle nils by mapping evaluation_assignments to evaluator names with nil-safe access (e.g., use evaluator&.name or compact the evaluator objects before mapping) so the resulting string joins only existing names; target the evaluators key and the suggestion.evaluation_assignments / evaluator references when applying this change.app/views/decidim/participatory_documents/admin/suggestions/bulk_actions/_dropdown.html.erb (1)
10-11:⚠️ Potential issue | 🟡 MinorRemove the duplicated dropdown arrow icon.
Lines 10 and 11 both render
<%= icon "arrow-down-s-line" %>, which will display two chevron icons in the button.Proposed fix
<%= t("decidim.participatory_documents.admin.suggestions.index.actions.title") %> <%= icon "arrow-down-s-line" %> - <%= icon "arrow-down-s-line" %> </button>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/views/decidim/participatory_documents/admin/suggestions/bulk_actions/_dropdown.html.erb` around lines 10 - 11, The partial is rendering the same chevron twice by calling the icon helper twice; remove the duplicate call to icon "arrow-down-s-line" in the _dropdown.html.erb partial so only a single <%= icon "arrow-down-s-line" %> remains (locate the two consecutive uses of the icon helper and delete one).
🧹 Nitpick comments (11)
app/views/decidim/participatory_documents/section_suggestions/_form.html.erb (1)
1-2: Minor inconsistency in class name qualification.The view uses fully qualified
Decidim::EngineRouterwhile the controllers use unqualifiedEngineRouter. Both work correctly, but for consistency across the codebase, consider using the same style.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/views/decidim/participatory_documents/section_suggestions/_form.html.erb` around lines 1 - 2, The view is using a fully qualified constant Decidim::EngineRouter while controllers use the unqualified EngineRouter; update the call in the decidim_form_for invocation (the call to Decidim::EngineRouter.main_proxy(...).document_section_suggestions_path) to use EngineRouter.main_proxy(...) instead so the constant qualification matches the rest of the codebase and behavior remains identical.config/assets.rb (1)
19-19: Consider removing or documenting the commented stylesheet registration.Line 19 is commented-out executable config; either remove it or add a short reason for when it should be enabled.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@config/assets.rb` at line 19, The commented-out stylesheet registration Decidim::Shakapacker.register_stylesheet_import("stylesheets/decidim/participatory_documents/participatory_documents_admin", group: :admin) should either be removed or documented: either delete the commented line if it is no longer needed, or replace it with a short inline comment explaining why it is disabled and under what conditions to re-enable it (e.g., required for admin CSS in participatory_documents), so future maintainers know its purpose.app/helpers/decidim/participatory_documents/admin/button_helper.rb (1)
12-12: Remove duplicated CSS class token.Line 12 has
icon iconin the class list. Please keep a singleiconto reduce noise.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/helpers/decidim/participatory_documents/admin/button_helper.rb` at line 12, The class string passed to the icon helper contains a duplicated token ("icon icon") in the icon(...) call inside button_helper.rb; edit that call (the icon(...) invocation that currently uses "icon--document icon icon file-text-lines mr-xs") to remove the duplicate so only one "icon" remains (e.g., "icon--document icon file-text-lines mr-xs"), leaving all other classes and attributes (aria_label, role) unchanged.app/forms/decidim/participatory_documents/admin/evaluation_assignment_form.rb (1)
25-31: Consider adding errors only once instead of per-role.The current implementation adds an error for each evaluator role that doesn't match the participatory space, which could result in duplicate error messages for the same field.
♻️ Suggested improvement
def same_participatory_space return if evaluator_roles.empty? || !current_component - evaluator_roles.each do |evaluator_role| - errors.add(:evaluator_role_ids, :invalid) if current_component.participatory_space != evaluator_role.participatory_space - end + invalid_roles = evaluator_roles.reject { |role| role.participatory_space == current_component.participatory_space } + errors.add(:evaluator_role_ids, :invalid) if invalid_roles.any? end🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/forms/decidim/participatory_documents/admin/evaluation_assignment_form.rb` around lines 25 - 31, The same_participatory_space validation currently adds an error for each mismatched evaluator_role, causing duplicate messages; update the method (same_participatory_space) to check whether any evaluator_role in evaluator_roles has a participatory_space different from current_component.participatory_space and, if so, call errors.add(:evaluator_role_ids, :invalid) only once (avoid per-role errors), keeping the early return when evaluator_roles.empty? or current_component is nil.app/views/decidim/participatory_documents/admin/suggestions/_bulk-actions.html.erb (1)
1-1: Redundant inline style with Tailwind class.The
style="display: flex"is redundant since theflexTailwind class already appliesdisplay: flex. Unless there's a specific CSS specificity issue being worked around, consider removing the inline style.♻️ Suggested fix
-<div class="flex align-middle gap-x-4 ml-auto" id="js-other-actions-wrapper" style="display: flex"> +<div class="flex align-middle gap-x-4 ml-auto" id="js-other-actions-wrapper">🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/views/decidim/participatory_documents/admin/suggestions/_bulk-actions.html.erb` at line 1, Remove the redundant inline style attribute from the div with id "js-other-actions-wrapper" (the element already uses the Tailwind "flex" class which sets display:flex); edit the markup in _bulk-actions.html.erb to drop style="display: flex" from the <div id="js-other-actions-wrapper" class="flex align-middle gap-x-4 ml-auto"> and verify no CSS specificity or JS relies on that inline style (if it does, migrate that behavior to a stylesheet or a utility class).app/views/decidim/participatory_documents/admin/suggestions/_suggestion.html.erb (1)
23-27: Make the displayed evaluator deterministic.Line 27 uses
.lastwithout an explicit order, so the shown evaluator can vary depending on relation ordering. Prefer explicit ordering when choosing which evaluator to display.Proposed refactor
- <% count = suggestion.evaluation_assignments.size %> + <% assignments = suggestion.evaluation_assignments.order(created_at: :asc) %> + <% count = assignments.size %> @@ - <%= suggestion.evaluation_assignments.last&.evaluator_role&.user&.name %> + <%= assignments.last&.evaluator_role&.user&.name %>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/views/decidim/participatory_documents/admin/suggestions/_suggestion.html.erb` around lines 23 - 27, The view uses suggestion.evaluation_assignments.last which is non-deterministic; explicitly order the association before picking an evaluator (for example order by created_at or updated_at) and then select the correct record (e.g., order(created_at: :desc).first or order(created_at: :asc).last) so the displayed evaluator (accessed via suggestion.evaluation_assignments...evaluator_role.user.name) is deterministic; update the code that references suggestion.evaluation_assignments.last to use an explicit order on the association.app/packs/src/decidim/participatory_documents/admin/manage_bulk_suggestions.js (1)
139-151: Potential TomSelect re-initialization on repeatedturbo:loadevents.When using Turbo, the
turbo:loadevent fires on every page navigation. If the same page is visited multiple times, TomSelect will be initialized again on elements that may already have an instance, potentially causing duplicate controls or memory leaks.Consider checking if TomSelect is already initialized or destroying previous instances:
♻️ Proposed fix to prevent duplicate initialization
document.addEventListener("turbo:load", () => { const evaluatorMultiselectContainers = document.querySelectorAll( ".js-evaluator-multiselect" ); evaluatorMultiselectContainers.forEach((container) => { + // Skip if already initialized + if (container.tomselect) { + return; + } + const config = { plugins: ["remove_button", "dropdown_input"], allowEmptyOption: true }; - return new TomSelect(container, config); + new TomSelect(container, config); }); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/packs/src/decidim/participatory_documents/admin/manage_bulk_suggestions.js` around lines 139 - 151, TomSelect instances are re-initialized on every turbo:load causing duplicate controls; inside the turbo:load handler iterate evaluatorMultiselectContainers and for each container check for an existing instance (e.g., container.tomselect or container._tomSelect) and if present call its destroy() method (or skip creation) before creating a new one, then create the instance with new TomSelect(container, config); update the loop around evaluatorMultiselectContainers and references to TomSelect to perform this guard/destroy to prevent duplicate initialization.spec/controllers/decidim/participatory_documents/admin/evaluation_assignments_controller_spec.rb (1)
35-62: Consider clarifying test context descriptions for readability.The context descriptions are slightly confusing. In the "when evaluator is assigned" context (line 35), an assignment exists for
evaluator_role, but thecreate_paramsuseevaluator_role2. The test verifies that a different evaluator can be assigned. Similarly, "when evaluator is not assigned" (line 50) actually tests that duplicate assignments are prevented.Consider renaming contexts for clarity:
Suggested description improvements
- context "when evaluator is assigned to a suggestion" do + context "when assigning a different evaluator to an already-assigned suggestion" do let!(:assignment) { create(:suggestion_evaluation_assignment, suggestion:, evaluator_role:) } - it "allows assigning an evaluator to a suggestion" do + it "allows assigning an additional evaluator" do ... - context "when evaluator is not assigned to a suggestion" do + context "when the same evaluator is already assigned" do let!(:assignment) { create(:suggestion_evaluation_assignment, suggestion:, evaluator_role: evaluator_role2) } - it "does not allow assigning an evaluator to a suggestion" do + it "prevents duplicate assignment" do🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@spec/controllers/decidim/participatory_documents/admin/evaluation_assignments_controller_spec.rb` around lines 35 - 62, Rename the RSpec context strings to accurately reflect what each test is asserting: update the first context (currently "when evaluator is assigned to a suggestion") to something like "when a different evaluator is already assigned" or "when another evaluator is assigned" because it creates an assignment with evaluator_role while create_params uses evaluator_role2; update the second context (currently "when evaluator is not assigned to a suggestion") to something like "when attempting to create a duplicate assignment" or "when same evaluator already assigned" since it ensures duplicate assignments are prevented; ensure the descriptions reference the fixture names used in the test (assignment, suggestion_evaluation_assignment, evaluator_role, evaluator_role2, create_params) so readers immediately understand the setup and expectation.spec/permissions/decidim/participatory_documents/admin/permissions_spec.rb (1)
58-66: Remove commented-out code.This commented-out shared example appears to be dead code left over from development. If it's no longer needed, consider removing it to keep the spec clean.
Proposed cleanup
- # shared_examples "cannot add evaluators to the suggestion" do - # describe "add other evaluators" do - # let(:action_subject) { :suggestions } - # let(:action_name) { :assign_to_evaluator } - # - # # it { expect { subject.allowed? }.to raise_error(Decidim::PermissionAction::PermissionNotSetError) } - # it { expect(subject.allowed?).to be true } - # end - # end🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@spec/permissions/decidim/participatory_documents/admin/permissions_spec.rb` around lines 58 - 66, Remove the dead commented-out shared examples block named "cannot add evaluators to the suggestion" (the commented describe "add other evaluators" and its let(:action_subject)/let(:action_name)/it lines) from the spec so the file no longer contains this leftover commented code; if you think the behavior might be needed later, rely on Git history instead of keeping commented code in the file.app/commands/decidim/participatory_documents/admin/assign_suggestions_to_evaluator.rb (1)
33-48: Bulk assign path does per-pair queries (N×M).Similar to the unassign command, the nested iteration with individual
find_assignmentcalls results in N×M database queries. Consider batching the lookup.💡 Proposed batching refactor
def assign_suggestions transaction do + existing_assignments = + Decidim::ParticipatoryDocuments::EvaluationAssignment + .where(suggestion: form.suggestions, evaluator_role: form.evaluator_roles) + .pluck(:suggestion_id, :evaluator_role_id) + .to_set + form.evaluator_roles.each do |evaluator_role| form.suggestions.each do |suggestion| - find_assignment(suggestion, evaluator_role) || assign_suggestion(suggestion, evaluator_role) + unless existing_assignments.include?([suggestion.id, evaluator_role.id]) + assign_suggestion(suggestion, evaluator_role) + end end end end end - - def find_assignment(suggestion, evaluator_role) - Decidim::ParticipatoryDocuments::EvaluationAssignment.find_by( - suggestion:, - evaluator_role: - ) - end🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/commands/decidim/participatory_documents/admin/assign_suggestions_to_evaluator.rb` around lines 33 - 48, The nested loops in assign_suggestions call find_assignment per pair causing N×M queries; instead prefetch existing assignments in one query using Decidim::ParticipatoryDocuments::EvaluationAssignment.where(suggestion: form.suggestions, evaluator_role: form.evaluator_roles), build a lookup keyed by [suggestion_id, evaluator_role_id], then inside the transaction iterate form.evaluator_roles and form.suggestions and call assign_suggestion(suggestion, evaluator_role) only when the lookup lacks that key; keep the transaction block and reuse assign_suggestion and find_assignment identifiers for clarity.app/models/decidim/participatory_documents/suggestion.rb (1)
162-166: Redundant.where(suggestion: self)clause.The
evaluation_assignmentsassociation is already scoped toselfvia the foreign key. The additional.where(suggestion: self)is unnecessary.Proposed simplification
def evaluators - evaluator_role_ids = evaluation_assignments.where(suggestion: self).pluck(:evaluator_role_id) + evaluator_role_ids = evaluation_assignments.pluck(:evaluator_role_id) user_ids = participatory_space.user_roles(:evaluator).where(id: evaluator_role_ids).pluck(:decidim_user_id) participatory_space.organization.users.where(id: user_ids) end🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/models/decidim/participatory_documents/suggestion.rb` around lines 162 - 166, In the evaluators method, remove the redundant `.where(suggestion: self)` because the evaluation_assignments association is already scoped to this Suggestion; change `evaluation_assignments.where(suggestion: self).pluck(:evaluator_role_id)` to simply `evaluation_assignments.pluck(:evaluator_role_id)` and keep the rest of the resolution (`participatory_space.user_roles(:evaluator).where(id: evaluator_role_ids).pluck(:decidim_user_id)` and `participatory_space.organization.users.where(id: user_ids)`) unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@app/forms/decidim/participatory_documents/admin/evaluation_assignment_form.rb`:
- Around line 16-19: In the suggestions method, replace the deprecated Ruby
Array de-dup `.uniq` call with ActiveRecord’s `.distinct` on the relation
returned by Decidim::ParticipatoryDocuments::Suggestion.where(id:
suggestion_ids) and then apply the component filter; because Suggestion uses a
polymorphic belongs_to :suggestable (not a direct document association), do not
try to filter by a non-existent `.document` association—if you need DB-level
filtering for suggestable type/id add a where(suggestable_type: ...,
suggestable_id: ...) before calling .distinct; keep using current_component for
the component comparison (or apply component in SQL if component is a column)
and memoize as `@suggestions` ||= ...distinct...filter { |s| s.component ==
current_component } if you must filter in Ruby.
In `@app/models/decidim/participatory_documents/suggestion.rb`:
- Around line 190-192: The method log_presenter_class_for in the Suggestion
model currently returns
Decidim::ParticipatoryDocuments::AdminLog::EvaluationAssignmentPresenter
incorrectly; create a new presenter class
Decidim::ParticipatoryDocuments::AdminLog::SuggestionPresenter (following the
pattern of other models' AdminLog presenters) and update log_presenter_class_for
to return Decidim::ParticipatoryDocuments::AdminLog::SuggestionPresenter so
Suggestion admin log entries use the correct presenter.
In `@app/packs/src/decidim/participatory_documents/pdf/suggestion_form.js`:
- Around line 86-89: The code mixes scoped lookup for the form
(this.div.querySelector) with global ID queries for the file UI, which can
attach to the wrong elements when multiple forms exist; update the file input
and filename container lookups to use the same scope as the form (e.g., use
form.querySelector("#file-upload-field") and
form.querySelector("#fileNameContainer") or this.div.querySelector when falling
back) so they reference the elements inside the current form container (refer to
the variables form and this.div and the IDs file-upload-field and
fileNameContainer).
In
`@app/presenters/decidim/participatory_documents/log/evaluation_assignment_presenter.rb`:
- Around line 13-15: Guard against a nil suggestion before calling its
presenter: in EvaluationAssignmentPresenter (the method that currently does
resource.present? then resource.suggestion.presenter.title(html_escape: true)),
check that resource.suggestion is present (e.g., resource.present? &&
resource.suggestion.present?) before dereferencing; if suggestion is nil, fall
back to calling super so the method returns the default behavior.
In `@package.json`:
- Around line 52-53: Move the runtime package "tom-select" out of
devDependencies into dependencies in package.json because it is imported/used at
runtime by
app/packs/src/decidim/participatory_documents/admin/manage_bulk_suggestions.js
(the evaluator multiselect initialization). Edit package.json: remove the
"tom-select" entry from devDependencies and add the same version string under
dependencies (keeping the existing semver), then run your package manager
lockfile update (npm/yarn/pnpm install) to regenerate lockfile and ensure the
package is available in production builds.
In `@spec/lib/component_spec.rb`:
- Around line 25-28: Update the example description to accurately reflect the
assertion logic: rename the it block title that currently reads "export only
evaluators from the user" to something like "exports only suggestions authored
by the user" (or similar wording) so it matches the expectations checking
collection includes suggestion and excludes another_suggestion; locate the it
block surrounding the variables collection, suggestion, and another_suggestion
in spec/lib/component_spec.rb and change only the description string.
- Around line 15-18: Update the example description string in the RSpec example
that currently starts with it "export evaluators and different users" to
accurately reflect the assertions (that the collection includes specific
suggestions and that those suggestions have two distinct authors); modify the it
description to something like "exports suggestions with different authors" in
the spec example where collection, suggestion, another_suggestion and the
map(&:author).uniq.count assertion are used.
In `@spec/models/decidim/participatory_documents/evaluation_assignment_spec.rb`:
- Line 19: Update the context description string in the spec so the grammar is
correct: change the context labeled "when participatory_process_user_role is a
evaluator" to "when participatory_process_user_role is an evaluator" in the
evaluation_assignment_spec (the context block in
spec/models/decidim/participatory_documents/evaluation_assignment_spec.rb).
In `@spec/system/admin/admin_manages_suggestion_evaluators_spec.rb`:
- Line 82: Update the RSpec example description to use the correct domain term:
change the example string in the `it` block that currently reads `it "only shows
the proposals assigned to the selected evaluator"` to `it "only shows the
suggestions assigned to the selected evaluator"` (in the
`admin_manages_suggestion_evaluators_spec` test), and search the same spec for
any other occurrences of "proposals" to replace with "suggestions" to keep
terminology consistent.
- Around line 179-180: Update the RSpec context description to use "evaluator"
instead of "validator" so terminology matches the PR-wide rename; specifically
change the context string currently written as "when admin to assign a
validator" to "when admin to assign an evaluator" in the
admin_manages_suggestion_evaluators_spec (the context block that wraps the
example "assigns the suggestions to the evaluator").
---
Outside diff comments:
In
`@app/packs/src/decidim/participatory_documents/admin/manage_bulk_suggestions.js`:
- Line 120: Remove the leftover debug console.log by deleting the line that
prints `.js-bulk-action-form .js-suggestion-id-${suggestionId}` in
manage_bulk_suggestions.js (the stray console.log call that references
suggestionId); ensure no other console.debug/console.log statements remain in
the same function or module so only intentional logging (if any) is left.
In
`@spec/commands/decidim/participatory_documents/admin/assign_suggestion_to_evaluator_spec.rb`:
- Around line 53-59: The test context incorrectly uses "valuator" and defines
let(:valuator_role) which doesn't override the form mock's evaluator_role;
change the context description and the helper to use the correct terminology and
variable name (replace let(:valuator_role) { nil } with let(:evaluator_role) {
nil }) so the form mock referenced on line 28 sees a nil evaluator_role, and
keep the example expectation (expect { subject.call }.to broadcast(:invalid))
as-is to validate the "missing evaluator" scenario.
In `@spec/system/admin/admin_manages_suggestions_spec.rb`:
- Line 227: Remove the debug puts call that prints first_section_suggestions;
specifically delete the line containing puts "first_section suggestions:
#{first_section_suggestions}" in the spec (no replacement needed), then run the
test suite to confirm nothing else relies on this output.
---
Duplicate comments:
In `@app/serializers/decidim/participatory_documents/suggestion_serializer.rb`:
- Line 27: The current evaluators serialization uses
suggestion.evaluation_assignments.map(&:evaluator).map(&:name).join(", ") which
will raise if any evaluation_assignment has a nil evaluator; update the
evaluators construction in SuggestionSerializer to safely handle nils by mapping
evaluation_assignments to evaluator names with nil-safe access (e.g., use
evaluator&.name or compact the evaluator objects before mapping) so the
resulting string joins only existing names; target the evaluators key and the
suggestion.evaluation_assignments / evaluator references when applying this
change.
In
`@app/views/decidim/participatory_documents/admin/suggestions/bulk_actions/_dropdown.html.erb`:
- Around line 10-11: The partial is rendering the same chevron twice by calling
the icon helper twice; remove the duplicate call to icon "arrow-down-s-line" in
the _dropdown.html.erb partial so only a single <%= icon "arrow-down-s-line" %>
remains (locate the two consecutive uses of the icon helper and delete one).
---
Nitpick comments:
In
`@app/commands/decidim/participatory_documents/admin/assign_suggestions_to_evaluator.rb`:
- Around line 33-48: The nested loops in assign_suggestions call find_assignment
per pair causing N×M queries; instead prefetch existing assignments in one query
using Decidim::ParticipatoryDocuments::EvaluationAssignment.where(suggestion:
form.suggestions, evaluator_role: form.evaluator_roles), build a lookup keyed by
[suggestion_id, evaluator_role_id], then inside the transaction iterate
form.evaluator_roles and form.suggestions and call assign_suggestion(suggestion,
evaluator_role) only when the lookup lacks that key; keep the transaction block
and reuse assign_suggestion and find_assignment identifiers for clarity.
In
`@app/forms/decidim/participatory_documents/admin/evaluation_assignment_form.rb`:
- Around line 25-31: The same_participatory_space validation currently adds an
error for each mismatched evaluator_role, causing duplicate messages; update the
method (same_participatory_space) to check whether any evaluator_role in
evaluator_roles has a participatory_space different from
current_component.participatory_space and, if so, call
errors.add(:evaluator_role_ids, :invalid) only once (avoid per-role errors),
keeping the early return when evaluator_roles.empty? or current_component is
nil.
In `@app/helpers/decidim/participatory_documents/admin/button_helper.rb`:
- Line 12: The class string passed to the icon helper contains a duplicated
token ("icon icon") in the icon(...) call inside button_helper.rb; edit that
call (the icon(...) invocation that currently uses "icon--document icon icon
file-text-lines mr-xs") to remove the duplicate so only one "icon" remains
(e.g., "icon--document icon file-text-lines mr-xs"), leaving all other classes
and attributes (aria_label, role) unchanged.
In `@app/models/decidim/participatory_documents/suggestion.rb`:
- Around line 162-166: In the evaluators method, remove the redundant
`.where(suggestion: self)` because the evaluation_assignments association is
already scoped to this Suggestion; change
`evaluation_assignments.where(suggestion: self).pluck(:evaluator_role_id)` to
simply `evaluation_assignments.pluck(:evaluator_role_id)` and keep the rest of
the resolution (`participatory_space.user_roles(:evaluator).where(id:
evaluator_role_ids).pluck(:decidim_user_id)` and
`participatory_space.organization.users.where(id: user_ids)`) unchanged.
In
`@app/packs/src/decidim/participatory_documents/admin/manage_bulk_suggestions.js`:
- Around line 139-151: TomSelect instances are re-initialized on every
turbo:load causing duplicate controls; inside the turbo:load handler iterate
evaluatorMultiselectContainers and for each container check for an existing
instance (e.g., container.tomselect or container._tomSelect) and if present call
its destroy() method (or skip creation) before creating a new one, then create
the instance with new TomSelect(container, config); update the loop around
evaluatorMultiselectContainers and references to TomSelect to perform this
guard/destroy to prevent duplicate initialization.
In
`@app/views/decidim/participatory_documents/admin/suggestions/_bulk-actions.html.erb`:
- Line 1: Remove the redundant inline style attribute from the div with id
"js-other-actions-wrapper" (the element already uses the Tailwind "flex" class
which sets display:flex); edit the markup in _bulk-actions.html.erb to drop
style="display: flex" from the <div id="js-other-actions-wrapper" class="flex
align-middle gap-x-4 ml-auto"> and verify no CSS specificity or JS relies on
that inline style (if it does, migrate that behavior to a stylesheet or a
utility class).
In
`@app/views/decidim/participatory_documents/admin/suggestions/_suggestion.html.erb`:
- Around line 23-27: The view uses suggestion.evaluation_assignments.last which
is non-deterministic; explicitly order the association before picking an
evaluator (for example order by created_at or updated_at) and then select the
correct record (e.g., order(created_at: :desc).first or order(created_at:
:asc).last) so the displayed evaluator (accessed via
suggestion.evaluation_assignments...evaluator_role.user.name) is deterministic;
update the code that references suggestion.evaluation_assignments.last to use an
explicit order on the association.
In
`@app/views/decidim/participatory_documents/section_suggestions/_form.html.erb`:
- Around line 1-2: The view is using a fully qualified constant
Decidim::EngineRouter while controllers use the unqualified EngineRouter; update
the call in the decidim_form_for invocation (the call to
Decidim::EngineRouter.main_proxy(...).document_section_suggestions_path) to use
EngineRouter.main_proxy(...) instead so the constant qualification matches the
rest of the codebase and behavior remains identical.
In `@config/assets.rb`:
- Line 19: The commented-out stylesheet registration
Decidim::Shakapacker.register_stylesheet_import("stylesheets/decidim/participatory_documents/participatory_documents_admin",
group: :admin) should either be removed or documented: either delete the
commented line if it is no longer needed, or replace it with a short inline
comment explaining why it is disabled and under what conditions to re-enable it
(e.g., required for admin CSS in participatory_documents), so future maintainers
know its purpose.
In
`@spec/controllers/decidim/participatory_documents/admin/evaluation_assignments_controller_spec.rb`:
- Around line 35-62: Rename the RSpec context strings to accurately reflect what
each test is asserting: update the first context (currently "when evaluator is
assigned to a suggestion") to something like "when a different evaluator is
already assigned" or "when another evaluator is assigned" because it creates an
assignment with evaluator_role while create_params uses evaluator_role2; update
the second context (currently "when evaluator is not assigned to a suggestion")
to something like "when attempting to create a duplicate assignment" or "when
same evaluator already assigned" since it ensures duplicate assignments are
prevented; ensure the descriptions reference the fixture names used in the test
(assignment, suggestion_evaluation_assignment, evaluator_role, evaluator_role2,
create_params) so readers immediately understand the setup and expectation.
In `@spec/permissions/decidim/participatory_documents/admin/permissions_spec.rb`:
- Around line 58-66: Remove the dead commented-out shared examples block named
"cannot add evaluators to the suggestion" (the commented describe "add other
evaluators" and its let(:action_subject)/let(:action_name)/it lines) from the
spec so the file no longer contains this leftover commented code; if you think
the behavior might be needed later, rely on Git history instead of keeping
commented code in the file.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: dc91fb79-04c5-40ec-ac05-46b93246c12c
⛔ Files ignored due to path filters (2)
Gemfile.lockis excluded by!**/*.lockpackage-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (81)
.github/workflows/lint.yml.github/workflows/test.yml.rubocop.yml.ruby-versionGemfileREADME.mdapp/commands/decidim/participatory_documents/admin/assign_suggestions_to_evaluator.rbapp/commands/decidim/participatory_documents/admin/create_suggestion_note.rbapp/commands/decidim/participatory_documents/admin/unassign_suggestions_from_evaluator.rbapp/controllers/decidim/participatory_documents/admin/evaluation_assignments_controller.rbapp/controllers/decidim/participatory_documents/admin/suggestions_controller.rbapp/controllers/decidim/participatory_documents/admin/valuation_assignments_controller.rbapp/controllers/decidim/participatory_documents/document_suggestions_controller.rbapp/controllers/decidim/participatory_documents/section_suggestions_controller.rbapp/forms/decidim/participatory_documents/admin/evaluation_assignment_form.rbapp/forms/decidim/participatory_documents/admin/valuation_assignment_form.rbapp/helpers/decidim/participatory_documents/admin/button_helper.rbapp/helpers/decidim/participatory_documents/admin/suggestion_helper.rbapp/jobs/decidim/participatory_documents/export_my_suggestions_job.rbapp/models/concerns/decidim/participatory_documents/evaluator_override.rbapp/models/concerns/decidim/participatory_documents/participatory_space_user_role_override.rbapp/models/decidim/participatory_documents/evaluation_assignment.rbapp/models/decidim/participatory_documents/suggestion.rbapp/packs/src/decidim/participatory_documents/admin/manage_bulk_suggestions.jsapp/packs/src/decidim/participatory_documents/pdf/suggestion_form.jsapp/packs/src/decidim/participatory_documents/upload_modal.jsapp/permissions/decidim/participatory_documents/admin/permissions.rbapp/presenters/decidim/participatory_documents/admin_log/evaluation_assignment_presenter.rbapp/presenters/decidim/participatory_documents/admin_log/value_types/evaluator_role_user_presenter.rbapp/presenters/decidim/participatory_documents/log/evaluation_assignment_presenter.rbapp/presenters/decidim/participatory_documents/suggestion_presenter.rbapp/serializers/decidim/participatory_documents/my_suggestion_serializer.rbapp/serializers/decidim/participatory_documents/suggestion_serializer.rbapp/views/decidim/participatory_documents/admin/suggestion_notes/_suggestion_notes.html.erbapp/views/decidim/participatory_documents/admin/suggestions/_add_evaluators.html.erbapp/views/decidim/participatory_documents/admin/suggestions/_bulk-actions.html.erbapp/views/decidim/participatory_documents/admin/suggestions/_evaluators.html.erbapp/views/decidim/participatory_documents/admin/suggestions/_suggestion.html.erbapp/views/decidim/participatory_documents/admin/suggestions/bulk_actions/_assign_to_evaluator.html.erbapp/views/decidim/participatory_documents/admin/suggestions/bulk_actions/_assign_to_valuator.html.erbapp/views/decidim/participatory_documents/admin/suggestions/bulk_actions/_dropdown.html.erbapp/views/decidim/participatory_documents/admin/suggestions/bulk_actions/_evaluators_picker.html.erbapp/views/decidim/participatory_documents/admin/suggestions/bulk_actions/_unassign_from_evaluator.html.erbapp/views/decidim/participatory_documents/admin/suggestions/bulk_actions/_unassign_from_valuator.html.erbapp/views/decidim/participatory_documents/admin/suggestions/index.html.erbapp/views/decidim/participatory_documents/admin/suggestions/show.html.erbapp/views/decidim/participatory_documents/document_suggestions/_form.html.erbapp/views/decidim/participatory_documents/section_suggestions/_form.html.erbconfig/assets.rbconfig/locales/en.ymldb/migrate/20260303220922_rename_valuation_assignments_to_evaluation_assignments.rbdecidim-participatory_documents.gemspeclib/decidim/participatory_documents/admin_engine.rblib/decidim/participatory_documents/component.rblib/decidim/participatory_documents/engine.rblib/decidim/participatory_documents/test/factories.rblib/decidim/participatory_documents/version.rblib/tasks/decidim_participatory_documents.rakepackage.jsonspec/commands/decidim/participatory_documents/admin/assign_suggestion_to_evaluator_spec.rbspec/controllers/decidim/participatory_documents/admin/annotations_controller_spec.rbspec/controllers/decidim/participatory_documents/admin/evaluation_assignments_controller_spec.rbspec/controllers/decidim/participatory_documents/admin/section_suggestions_controller_spec.rbspec/controllers/decidim/participatory_documents/document_suggestions_controller_spec.rbspec/controllers/decidim/participatory_documents/section_suggestions_controller_spec.rbspec/jobs/export_my_suggestions_job_spec.rbspec/lib/component_spec.rbspec/lib/module_config_spec.rbspec/models/decidim/participatory_documents/document_spec.rbspec/models/decidim/participatory_documents/evaluation_assignment_spec.rbspec/models/decidim/participatory_documents/evaluator_components_spec.rbspec/models/decidim/participatory_documents/valuation_assignment_spec.rbspec/permissions/decidim/participatory_documents/admin/permissions_spec.rbspec/serializers/my_suggestion_serialiser_spec.rbspec/serializers/suggestion_serialiser_spec.rbspec/system/admin/admin_manages_participatory_documents_spec.rbspec/system/admin/admin_manages_suggestion_evaluators_spec.rbspec/system/admin/admin_manages_suggestion_valuators_spec.rbspec/system/admin/admin_manages_suggestions_spec.rbspec/system/index_participatory_documents_spec.rbspec/system/user_interacts_with_pdf_viewer_spec.rb
💤 Files with no reviewable changes (10)
- spec/controllers/decidim/participatory_documents/document_suggestions_controller_spec.rb
- spec/controllers/decidim/participatory_documents/admin/section_suggestions_controller_spec.rb
- app/views/decidim/participatory_documents/admin/suggestions/bulk_actions/_unassign_from_valuator.html.erb
- spec/controllers/decidim/participatory_documents/admin/annotations_controller_spec.rb
- spec/controllers/decidim/participatory_documents/section_suggestions_controller_spec.rb
- app/views/decidim/participatory_documents/admin/suggestions/bulk_actions/_assign_to_valuator.html.erb
- spec/models/decidim/participatory_documents/valuation_assignment_spec.rb
- spec/system/admin/admin_manages_suggestion_valuators_spec.rb
- app/forms/decidim/participatory_documents/admin/valuation_assignment_form.rb
- app/controllers/decidim/participatory_documents/admin/valuation_assignments_controller.rb
There was a problem hiding this comment.
🧹 Nitpick comments (1)
config/locales/en.yml (1)
141-145: Optional copy tweak for clearer success/error phrasing.Current strings are understandable but slightly awkward (“assigned to an evaluator successfully”). Consider making them action-first for consistency with the rest of the locale file.
Suggested copy
- invalid: There was an error assigning suggestions to an evaluator - success: Suggestions assigned to an evaluator successfully + invalid: There was an error assigning suggestions to the evaluator + success: Suggestions were successfully assigned to the evaluator @@ - invalid: There was an error unassigning suggestions from an evaluator - success: Evaluator unassigned from suggestions successfully + invalid: There was an error unassigning suggestions from the evaluator + success: The evaluator was successfully unassigned from the suggestions🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@config/locales/en.yml` around lines 141 - 145, Update the locale strings to an action-first phrasing for consistency: change the keys under the relevant block (the "assign" and "delete" entries: "invalid" and "success") so the success messages begin with the action (e.g., "Successfully assigned suggestions to an evaluator" and "Successfully unassigned suggestions from an evaluator") while keeping the invalid messages as error statements; edit the "assign.success" and "delete.success" values accordingly in en.yml.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@config/locales/en.yml`:
- Around line 141-145: Update the locale strings to an action-first phrasing for
consistency: change the keys under the relevant block (the "assign" and "delete"
entries: "invalid" and "success") so the success messages begin with the action
(e.g., "Successfully assigned suggestions to an evaluator" and "Successfully
unassigned suggestions from an evaluator") while keeping the invalid messages as
error statements; edit the "assign.success" and "delete.success" values
accordingly in en.yml.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🎩 What? Why?
This PR upgrades to Decidim version 0.31
📌 Related Issues
Link your PR to an issue
Related to #?
Fixes #130
📷 Screenshots
Summary by CodeRabbit
New Features
Improvements
Chores
Style