Add Benchmarking page for cross-space comparison#21
Merged
microstudi merged 10 commits intomainfrom Mar 5, 2026
Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds an admin benchmarking feature: new controller, views, helpers, presenters, heatmap utility, JS/CSS, routes and i18n to select participatory spaces and render comparative pivot/heatmap tables with permission checks and multiselect-driven submission. Changes
Sequence Diagram(s)sequenceDiagram
participant Admin as AdminBrowser
participant JS as TomSelectModule
participant Controller as BenchmarkingController
participant Builder as PivotTableBuilder
participant Presenter as ComparativePivotPresenter
participant View as ERB_Renderer
Admin->>JS: select spaces/metric (TomSelect)
JS->>Admin: form.requestSubmit()
Admin->>Controller: GET /admin/.../benchmarking?spaces[]=...
Controller->>Builder: build pivot tables per selected space (metric,row,col)
Builder-->>Controller: pivot_tables
Controller->>Presenter: initialize(pivot_tables, row_field, col_field)
Presenter-->>Controller: unified rows/cols and global ranges
Controller->>View: render show with presenter and helpers
View-->>Admin: HTML table (heatmap styles, totals, legend)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
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 unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
app/presenters/decidim/extra_user_fields/comparative_pivot_presenter.rb (1)
147-149: Minor duplication withBenchmarkingHelper#space_option_label.The title translation logic is duplicated between this method and
space_option_labelinBenchmarkingHelper(line 10). Consider extracting a shared helper if this pattern appears elsewhere.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/presenters/decidim/extra_user_fields/comparative_pivot_presenter.rb` around lines 147 - 149, The method translated_name duplicates the title translation logic found in BenchmarkingHelper#space_option_label; extract the shared logic into a new helper method (e.g., SpaceTitleHelper#localized_title or a module method) that accepts a space and returns the locale-aware title string, then replace translated_name and the logic in BenchmarkingHelper#space_option_label to call that new helper (update references in ComparativePivotPresenter#translated_name and BenchmarkingHelper#space_option_label to use the new helper).app/presenters/decidim/extra_user_fields/heatmap_intensity.rb (1)
12-20: Edge case: negative intensity ifmin > max.If the caller passes an inverted range where
min > max, the computed intensity would be negative. Whilepositive_minmaxinComparativePivotPresentershould prevent this, consider adding a guard for defensive coding.🛡️ Optional defensive guard
def intensity_vars(value, min, max) return "" if value.zero? || max.zero? + return "" if min > max range = max - min🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/presenters/decidim/extra_user_fields/heatmap_intensity.rb` around lines 12 - 20, The intensity_vars method can produce negative or >1 intensities if callers pass min > max; update intensity_vars to defensively handle inverted ranges by normalizing or clamping the range and resulting intensity: ensure range = (max - min).abs or swap min/max when min > max, compute intensity as (value - min).to_f / range only after normalization, then clamp intensity to 0.0..1.0 before rounding and building the CSS string; reference intensity_vars and ComparativePivotPresenter::positive_minmax for context.app/controllers/decidim/extra_user_fields/admin/benchmarking_controller.rb (1)
44-48: Consider caching or paginatingavailable_spacesfor large deployments.This method iterates all participatory space manifests and loads all spaces for the organization into memory. For organizations with hundreds of spaces, this could become slow. Consider adding caching or limiting the number of spaces returned.
💡 Optional: Add caching or limits
def available_spaces - `@available_spaces` ||= Decidim.participatory_space_manifests.flat_map do |manifest| - manifest.model_class_name.constantize.where(organization: current_organization) + `@available_spaces` ||= Rails.cache.fetch(["benchmarking_spaces", current_organization.cache_key], expires_in: 5.minutes) do + Decidim.participatory_space_manifests.flat_map do |manifest| + manifest.model_class_name.constantize.where(organization: current_organization).to_a + end end end🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/controllers/decidim/extra_user_fields/admin/benchmarking_controller.rb` around lines 44 - 48, The available_spaces method currently loads all spaces by iterating Decidim.participatory_space_manifests and calling manifest.model_class_name.constantize.where(organization: current_organization), which can blow memory for large orgs; change it to either (a) paginate/limit the query by accepting params (e.g., page, per_page) and call .limit/.offset or integrate with Kaminari/WillPaginate before collecting results, or (b) cache the result in Rails.cache.fetch keyed by current_organization.id and a timestamp/version (or use a short TTL) to avoid repeated full loads; update the available_spaces method to use the chosen approach and keep the manifest.model_class_name.constantize.where(...) call but with .select/.pluck for lighter payloads if only ids/names are needed.app/helpers/decidim/extra_user_fields/admin/benchmarking_helper.rb (1)
8-12: Consider handlingniltitle gracefully.If
space.titleisnil(e.g., a space with missing data), the Hash check and.values.firstcould raise an error. Consider adding a nil guard.🛡️ Optional nil-safe handling
def space_option_label(space) type = space.class.model_name.human - title = space.title.is_a?(Hash) ? (space.title[I18n.locale.to_s] || space.title.values.first) : space.title.to_s + title = case space.title + when Hash then space.title[I18n.locale.to_s] || space.title.values.first || "" + when nil then "" + else space.title.to_s + end "[#{type}] #{title}" end🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/helpers/decidim/extra_user_fields/admin/benchmarking_helper.rb` around lines 8 - 12, space_option_label can raise when space.title is nil; update the method to guard against nil before treating title as a Hash or calling .to_s/.values.first. Inside space_option_label, check if space.title is nil and provide a safe fallback (e.g., empty string or a localized "untitled") and only then handle the Hash branch (use safe navigation or conditional to access space.title[I18n.locale.to_s] || space.title.values.first). Ensure the returned label still uses "[#{type}] #{title}" with the nil-safe title.
🤖 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/packs/src/decidim/extra_user_fields/benchmarking_multiselect.js`:
- Line 3: The change handler currently calls form.submit() directly (via the
ts.on callback), which bypasses submit event handlers and built-in validation;
update the ts.on("change", ...) callback to find the enclosing form with
select.closest("form") and call form.requestSubmit() when available, falling
back to form.submit() for older browsers so the normal submit event flow and
validations are preserved.
---
Nitpick comments:
In `@app/controllers/decidim/extra_user_fields/admin/benchmarking_controller.rb`:
- Around line 44-48: The available_spaces method currently loads all spaces by
iterating Decidim.participatory_space_manifests and calling
manifest.model_class_name.constantize.where(organization: current_organization),
which can blow memory for large orgs; change it to either (a) paginate/limit the
query by accepting params (e.g., page, per_page) and call .limit/.offset or
integrate with Kaminari/WillPaginate before collecting results, or (b) cache the
result in Rails.cache.fetch keyed by current_organization.id and a
timestamp/version (or use a short TTL) to avoid repeated full loads; update the
available_spaces method to use the chosen approach and keep the
manifest.model_class_name.constantize.where(...) call but with .select/.pluck
for lighter payloads if only ids/names are needed.
In `@app/helpers/decidim/extra_user_fields/admin/benchmarking_helper.rb`:
- Around line 8-12: space_option_label can raise when space.title is nil; update
the method to guard against nil before treating title as a Hash or calling
.to_s/.values.first. Inside space_option_label, check if space.title is nil and
provide a safe fallback (e.g., empty string or a localized "untitled") and only
then handle the Hash branch (use safe navigation or conditional to access
space.title[I18n.locale.to_s] || space.title.values.first). Ensure the returned
label still uses "[#{type}] #{title}" with the nil-safe title.
In `@app/presenters/decidim/extra_user_fields/comparative_pivot_presenter.rb`:
- Around line 147-149: The method translated_name duplicates the title
translation logic found in BenchmarkingHelper#space_option_label; extract the
shared logic into a new helper method (e.g., SpaceTitleHelper#localized_title or
a module method) that accepts a space and returns the locale-aware title string,
then replace translated_name and the logic in
BenchmarkingHelper#space_option_label to call that new helper (update references
in ComparativePivotPresenter#translated_name and
BenchmarkingHelper#space_option_label to use the new helper).
In `@app/presenters/decidim/extra_user_fields/heatmap_intensity.rb`:
- Around line 12-20: The intensity_vars method can produce negative or >1
intensities if callers pass min > max; update intensity_vars to defensively
handle inverted ranges by normalizing or clamping the range and resulting
intensity: ensure range = (max - min).abs or swap min/max when min > max,
compute intensity as (value - min).to_f / range only after normalization, then
clamp intensity to 0.0..1.0 before rounding and building the CSS string;
reference intensity_vars and ComparativePivotPresenter::positive_minmax for
context.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a75a4408-4442-469f-8ef8-b618004b15d6
📒 Files selected for processing (14)
app/controllers/decidim/extra_user_fields/admin/benchmarking_controller.rbapp/helpers/decidim/extra_user_fields/admin/benchmarking_helper.rbapp/packs/entrypoints/decidim_extra_user_fields.jsapp/packs/src/decidim/extra_user_fields/benchmarking_multiselect.jsapp/packs/stylesheets/decidim/extra_user_fields/insights.scssapp/presenters/decidim/extra_user_fields/comparative_pivot_presenter.rbapp/presenters/decidim/extra_user_fields/heatmap_intensity.rbapp/presenters/decidim/extra_user_fields/pivot_table_presenter.rbapp/views/decidim/extra_user_fields/admin/benchmarking/_comparison_table.html.erbapp/views/decidim/extra_user_fields/admin/benchmarking/_selectors.html.erbapp/views/decidim/extra_user_fields/admin/benchmarking/show.html.erbconfig/locales/en.ymlconfig/locales/fr.ymllib/decidim/extra_user_fields/admin_engine.rb
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/presenters/decidim/extra_user_fields/comparative_pivot_presenter.rb`:
- Line 129: The code uses Set.new (in ComparativePivotPresenter) but doesn't
require Ruby's stdlib 'set', which can cause NameError; add require "set" near
the top of comparative_pivot_presenter.rb (before the class/module definition or
before the first use of Set) so Set.new is defined at runtime.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f14a3b3b-4374-4dda-a1e6-f34f7dad53ca
📒 Files selected for processing (4)
app/helpers/decidim/extra_user_fields/admin/insights_helper.rbapp/packs/src/decidim/extra_user_fields/benchmarking_multiselect.jsapp/presenters/decidim/extra_user_fields/comparative_pivot_presenter.rbconfig/i18n-tasks.yml
🚧 Files skipped from review as they are similar to previous changes (1)
- app/packs/src/decidim/extra_user_fields/benchmarking_multiselect.js
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
spec/system/admin_views_benchmarking_spec.rb (1)
200-203: Harden the unauthorized-access expectation.The assertion at Line 202 is weak (string absence). Prefer asserting user is blocked from the benchmarking route (redirect/forbidden target), so this test catches real auth regressions.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@spec/system/admin_views_benchmarking_spec.rb` around lines 200 - 203, The test currently uses a weak negative string check; update the spec around visit decidim_extra_user_fields.benchmarking_path to assert an explicit authorization outcome instead of have_no_content: after calling visit decidim_extra_user_fields.benchmarking_path, assert that the user was redirected or forbidden (for example assert current_path is not decidim_extra_user_fields.benchmarking_path and/or expect(page).to have_content("You are not authorized") or, for rack_test, check page.driver.status_code == 403) so the spec fails on real auth regressions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@spec/presenters/decidim/extra_user_fields/heatmap_intensity_spec.rb`:
- Around line 48-56: Update the example descriptions to match what the tests
actually assert: for the case calling wrapper.intensity_vars(8, 0, 10)
(intensity 0.8) change the it description to state it is checking behavior for
intensity above 0.6 (e.g., "uses ... when intensity is > 0.6"), and for
wrapper.intensity_vars(6, 0, 10) (intensity 0.6) change the it description to
state it is checking the boundary/exact 0.6 behavior (e.g., "uses ... when
intensity is exactly 0.6 / at the threshold"), so the text matches the numeric
conditions being tested.
---
Nitpick comments:
In `@spec/system/admin_views_benchmarking_spec.rb`:
- Around line 200-203: The test currently uses a weak negative string check;
update the spec around visit decidim_extra_user_fields.benchmarking_path to
assert an explicit authorization outcome instead of have_no_content: after
calling visit decidim_extra_user_fields.benchmarking_path, assert that the user
was redirected or forbidden (for example assert current_path is not
decidim_extra_user_fields.benchmarking_path and/or expect(page).to
have_content("You are not authorized") or, for rack_test, check
page.driver.status_code == 403) so the spec fails on real auth regressions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ec660a30-af23-4ba9-b775-7822742f2aa7
📒 Files selected for processing (4)
spec/helpers/decidim/extra_user_fields/admin/benchmarking_helper_spec.rbspec/presenters/decidim/extra_user_fields/comparative_pivot_presenter_spec.rbspec/presenters/decidim/extra_user_fields/heatmap_intensity_spec.rbspec/system/admin_views_benchmarking_spec.rb
spec/presenters/decidim/extra_user_fields/heatmap_intensity_spec.rb
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
spec/system/admin_views_benchmarking_spec.rb (1)
215-222: Consider adding explicit redirect/access denied verification.The test verifies that admin content isn't visible, but doesn't explicitly check the expected behavior (redirect to login, 403 forbidden, or redirect to root). This would make the test more robust and document the intended access control behavior.
Optional: Add explicit access verification
context "when user is not admin" do let(:user) { create(:user, :confirmed, organization:) } it "cannot access the benchmarking page" do visit decidim_extra_user_fields.benchmarking_path expect(page).to have_no_content("Comparative stats across spaces") + # Verify redirect or access denied behavior + expect(page).to have_current_path(decidim.root_path).or have_http_status(:forbidden) end endNote: Adjust the expected path/status based on your application's actual access control behavior.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@spec/system/admin_views_benchmarking_spec.rb` around lines 215 - 222, The test only checks that admin-only content is absent; update the spec around the non-admin context (context "when user is not admin", the let(:user) and the visit to decidim_extra_user_fields.benchmarking_path) to assert the explicit access control outcome your app uses—e.g., expect a redirect to the sign-in page or root (use have_current_path(...) with the appropriate path helper) or expect a 403/forbidden response or a visible access-denied flash/message—so add one clear expectation matching your app’s behavior alongside the existing content absence check.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@spec/system/admin_views_benchmarking_spec.rb`:
- Around line 53-78: The test's age-bucket assertion is mismatched:
user_male_old is created with 55.years.ago (age 55) but the spec expects "41 to
50"; update the fixture or the expectation to match — recommended: change
user_male_old's extended_data date_of_birth to 45.years.ago.to_date.to_s so the
user falls into the "41 to 50" bucket (look for the let(:user_male_old)
definition), or alternatively update the expectation in the spec
(within("tbody") expect to have_content "51 to 60") to reflect the existing
55-year-old.
- Around line 148-165: The test sets the user's extended_data country as a name
("france") which bypasses the Country insight's ISO handling; update the
let(:user_with_data) fixture (the extended_data hash in the user_with_data
factory setup) to use the ISO code "FR" instead of "france" so the Country
translation logic is exercised (look for the user_with_data let and the
extended_data key "country" in the spec).
---
Nitpick comments:
In `@spec/system/admin_views_benchmarking_spec.rb`:
- Around line 215-222: The test only checks that admin-only content is absent;
update the spec around the non-admin context (context "when user is not admin",
the let(:user) and the visit to decidim_extra_user_fields.benchmarking_path) to
assert the explicit access control outcome your app uses—e.g., expect a redirect
to the sign-in page or root (use have_current_path(...) with the appropriate
path helper) or expect a 403/forbidden response or a visible access-denied
flash/message—so add one clear expectation matching your app’s behavior
alongside the existing content absence check.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: dec32826-7124-484d-a8d5-9fa949a40af3
📒 Files selected for processing (4)
spec/helpers/decidim/extra_user_fields/admin/benchmarking_helper_spec.rbspec/helpers/decidim/extra_user_fields/admin/insights_helper_spec.rbspec/presenters/decidim/extra_user_fields/heatmap_intensity_spec.rbspec/system/admin_views_benchmarking_spec.rb
🚧 Files skipped from review as they are similar to previous changes (2)
- spec/helpers/decidim/extra_user_fields/admin/benchmarking_helper_spec.rb
- spec/presenters/decidim/extra_user_fields/heatmap_intensity_spec.rb
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
spec/system/admin_views_benchmarking_spec.rb (2)
63-65: Extract repeated space-key construction into a helper method.
"#{process.class.name}:#{process.id}"is repeated across many examples; extracting it to a small helper in the spec will reduce duplication and make future changes safer.Also applies to: 86-88, 97-99, 111-113, 130-132, 139-140, 158-160, 171-173, 195-196, 205-206
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@spec/system/admin_views_benchmarking_spec.rb` around lines 63 - 65, Extract the repeated string interpolation into a small helper method (e.g., define a helper like space_key(process) that returns "#{process.class.name}:#{process.id}") and replace occurrences where space_a_value and space_b_value are set (and other similar lines) to call that helper (e.g., space_key(process_a), space_key(process_b)); update all listed examples (the lines constructing space values and the visit call using decidim_extra_user_fields.benchmarking_path) to use the helper to remove duplication and centralize the format.
235-238: Tighten the non-admin access assertion to avoid false positives.This spec only checks that one title string is absent. Please also assert an explicit denied outcome (redirect target or authorization feedback) so the test fails for the right reason.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@spec/system/admin_views_benchmarking_spec.rb` around lines 235 - 238, The test that visits decidim_extra_user_fields.benchmarking_path currently only checks a missing title which can give false positives; update the example in the it block to assert an explicit denied outcome instead — e.g., after visit decidim_extra_user_fields.benchmarking_path assert the current_path is the expected redirect (such as root_path or the sign-in path) or assert presence of an authorization/flash message (e.g., expect(page).to have_css('.alert', text: /not authorized|access denied/i)) so the spec fails for an actual access denial rather than a missing title.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@config/locales/fr.yml`:
- Around line 30-35: Replace the English labels with French translations for
consistency: change the value of menu_title from "Benchmarking" to a French
label (e.g., "Comparaison" or "Analyse comparative") and update table_label from
"Tableau de comparaison benchmarking" to a fully French phrase (e.g., "Tableau
de comparaison" or "Tableau d'analyse comparative"); edit the string values for
the keys menu_title and table_label accordingly to match the rest of the fr.yml
translations.
- Around line 29-36: The menu_title value for the translation key
decidim.admin.extra_user_fields.benchmarking.menu_title is still in English;
update the French locale entry to a proper French term (e.g., "Comparaison" or
another product-appropriate translation) by changing the value under the
benchmarking block (menu_title) in the fr.yml file so it no longer reads
"Benchmarking".
---
Nitpick comments:
In `@spec/system/admin_views_benchmarking_spec.rb`:
- Around line 63-65: Extract the repeated string interpolation into a small
helper method (e.g., define a helper like space_key(process) that returns
"#{process.class.name}:#{process.id}") and replace occurrences where
space_a_value and space_b_value are set (and other similar lines) to call that
helper (e.g., space_key(process_a), space_key(process_b)); update all listed
examples (the lines constructing space values and the visit call using
decidim_extra_user_fields.benchmarking_path) to use the helper to remove
duplication and centralize the format.
- Around line 235-238: The test that visits
decidim_extra_user_fields.benchmarking_path currently only checks a missing
title which can give false positives; update the example in the it block to
assert an explicit denied outcome instead — e.g., after visit
decidim_extra_user_fields.benchmarking_path assert the current_path is the
expected redirect (such as root_path or the sign-in path) or assert presence of
an authorization/flash message (e.g., expect(page).to have_css('.alert', text:
/not authorized|access denied/i)) so the spec fails for an actual access denial
rather than a missing title.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: bf2724b0-dba0-4ee7-ae8d-2e4bbb1d0070
📒 Files selected for processing (2)
config/locales/fr.ymlspec/system/admin_views_benchmarking_spec.rb
There was a problem hiding this comment.
Pull request overview
Adds an admin “Benchmarking” page under Insights to compare participation metrics across multiple participatory spaces side-by-side, using a comparative pivot table with shared heatmap normalization.
Changes:
- Introduces a new admin benchmarking route/controller + views (selectors, comparison table, legend) for cross-space comparisons.
- Adds comparative presenter + shared heatmap normalization module, plus helper methods for table cells, totals, truncation/tooltip space names.
- Adds TomSelect-based multi-select auto-submit, styling updates, i18n (EN/FR), and a comprehensive test suite.
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| spec/system/admin_views_benchmarking_spec.rb | System coverage for admin benchmarking page rendering and behaviors |
| spec/presenters/decidim/extra_user_fields/heatmap_intensity_spec.rb | Unit tests for shared heatmap intensity calculations |
| spec/presenters/decidim/extra_user_fields/comparative_pivot_presenter_spec.rb | Unit tests for cross-space pivot merging, totals, and styles |
| spec/helpers/decidim/extra_user_fields/admin/insights_helper_spec.rb | Updates expectations for selector auto-submit behavior |
| spec/helpers/decidim/extra_user_fields/admin/benchmarking_helper_spec.rb | Tests helper rendering for benchmarking table cells/totals and space name truncation |
| lib/decidim/extra_user_fields/admin_engine.rb | Adds benchmarking route + admin Insights menu entry |
| config/locales/en.yml | Adds benchmarking UI translations (EN) |
| config/locales/fr.yml | Adds benchmarking UI translations (FR) |
| config/i18n-tasks.yml | Ignores benchmarking keys in unused key checks |
| app/views/decidim/extra_user_fields/admin/benchmarking/show.html.erb | New benchmarking page layout + conditional rendering |
| app/views/decidim/extra_user_fields/admin/benchmarking/_selectors.html.erb | Multi-select spaces + row/col/metric selectors form |
| app/views/decidim/extra_user_fields/admin/benchmarking/_comparison_table.html.erb | Side-by-side pivot table with space grouping/dividers + totals + legend |
| app/presenters/decidim/extra_user_fields/pivot_table_presenter.rb | Refactors heatmap logic into shared module |
| app/presenters/decidim/extra_user_fields/heatmap_intensity.rb | New shared heatmap normalization helpers |
| app/presenters/decidim/extra_user_fields/comparative_pivot_presenter.rb | New presenter combining multiple pivot tables with global normalization |
| app/packs/stylesheets/decidim/extra_user_fields/insights.scss | Adds benchmarking table header/divider + selector styling |
| app/packs/src/decidim/extra_user_fields/benchmarking_multiselect.js | New TomSelect multi-select + auto-submit on change |
| app/packs/entrypoints/decidim_extra_user_fields.js | Loads benchmarking multiselect JS |
| app/helpers/decidim/extra_user_fields/admin/insights_helper.rb | Uses requestSubmit() for selector auto-submit |
| app/helpers/decidim/extra_user_fields/admin/benchmarking_helper.rb | New helper methods for table cell rendering, totals, truncation/tooltip |
| app/controllers/decidim/extra_user_fields/admin/benchmarking_controller.rb | New controller: permission enforcement, param parsing, pivot construction |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
app/views/decidim/extra_user_fields/admin/benchmarking/_selectors.html.erb
Outdated
Show resolved
Hide resolved
app/views/decidim/extra_user_fields/admin/benchmarking/show.html.erb
Outdated
Show resolved
Hide resolved
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.
Fixes #18
Summary by CodeRabbit
New Features
Localization
Improvements
Tests