decidim-admin/app/packs/src/decidim/admin/controllers/language-change/controller.js (1)

3-11: Consider using Stimulus conventions instead of manual event listener management.

Stimulus provides a declarative data-action attribute for binding events and lifecycle-managed method binding. Manual addEventListener/removeEventListener + .bind(this) is unnecessary boilerplate when the framework handles it. For example, the form builder could emit data-action="change->language-change#handleChange" on the <select>, and you could remove the connect/disconnect hooks entirely.

decidim-admin/spec/system/admin_manages_taxonomies_spec.rb (2)

192-192: Avoid sleep(2) in system specs — use Capybara's waiting assertions instead.

Hard sleeps make tests slow and flaky. Capybara's matchers already retry/wait. Consider waiting on a visible UI change (e.g., a success flash message or the updated content) before asserting on the database:

expect(page).to have_content("My english value") # wait for UI update
expect(taxonomy.reload.children.last.name["en"]).to eq("My english value")

The same applies to line 206.

---

143-145: Redundant let(:organization) — identical to the outer definition on line 6.

Line 144 redefines organization with the same factory call as line 6. If no customization is intended, this can be removed.

decidim-admin/spec/system/admin_manages_taxonomies_spec.rb (3)

157-164: Hardcoded locale reset in after may drift from actual defaults.

The after block resets locales to %w(en ca es), but if the test suite's default locales ever change, this will silently diverge. Consider capturing the original values in before and restoring them in after.

♻️ Suggested approach

+      let(:original_i18n_locales) { I18n.available_locales }
+      let(:original_decidim_locales) { Decidim.available_locales }
+
       before do
+        # originals are captured by the lets above before mutation
         I18n.available_locales = available_locales
         ...
       end
 
       after do
-        I18n.available_locales = %w(en ca es)
-        Decidim.available_locales = %w(en ca es)
+        I18n.available_locales = original_i18n_locales
+        Decidim.available_locales = original_decidim_locales
         I18n.backend.reload!
         ...
       end

---

191-193: Replace sleep(2) with Capybara waiting assertions.

sleep is a common source of flaky tests in CI — too short on slow runners, too long everywhere else. Since the create/update actions likely produce a flash message or redirect, wait on a visible indicator instead, then assert on the DB.

-        sleep(2)
+        expect(page).to have_content("Item successfully created") # or whatever flash text is shown
         expect(taxonomy.reload.children.last.name["en"]).to eq("My english value")

-        sleep(2)
+        expect(page).to have_content("Item successfully updated") # or whatever flash text is shown
         expect(taxonomy.reload.children.last.name["en"]).to eq("My modified english value")

Also applies to: 205-207

---

174-182: first('input[type="text"]', visible: true) could be fragile if tab switch hasn't completed.

Consider adding a brief assertion that the expected tab panel is active before interacting with the input, or use a more specific selector targeting the locale panel (e.g., find('[id*="panel"]', visible: true) input). This guards against race conditions where the previous panel's input is still visible.

decidim-admin/spec/system/admin_manages_taxonomies_spec.rb (2)

146-164: remove_const + load is fragile; consider extracting a shared helper.

The send(:remove_const, :TaxonomyForm) / load dance appears both in the before and after blocks. If more specs need multi-locale form testing, this boilerplate will be duplicated. A small shared helper (e.g., reload_form_class(Decidim::Admin, :TaxonomyForm)) would reduce duplication and keep the teardown in sync with setup.

Also, the after block hardcodes %w(en ca es) — if the default test locales ever change, this will silently produce the wrong reset. Consider capturing the original values before overriding:

Sketch

# inside the context block
let(:original_i18n_locales) { I18n.available_locales }
let(:original_decidim_locales) { Decidim.available_locales }

after do
  I18n.available_locales = original_i18n_locales
  Decidim.available_locales = original_decidim_locales
  # ...
end

---

174-182: first('input[type="text"]', visible: true) is fragile — scope to the active tab panel.

If the form ever gains another visible text input (e.g., a code or slug field), this selector will silently fill the wrong field without failing. A safer approach is to scope within the active tab panel:

Example

-          first('input[type="text"]', visible: true).set("My english value")
+          find(".tabs-content [aria-hidden='false'] input[type='text']").set("My english value")

(Adjust the panel selector to match the actual DOM structure produced by translated / the Stimulus controller.)

The same applies to lines 179, 197, and 200.

decidim-admin/spec/system/admin_manages_taxonomies_spec.rb (2)

25-32: Consider saving and restoring original locale values instead of hardcoding.

The after block hardcodes %w(en ca es) as the restored defaults. If the project's default test locales ever change, this will silently drift. Capturing the originals before mutation is more resilient:

Sketch

# at the top of the describe block:
let(:original_i18n_locales) { I18n.available_locales }
let(:original_decidim_locales) { Decidim.available_locales }

after do
  I18n.available_locales = original_i18n_locales
  Decidim.available_locales = original_decidim_locales
  # ...
end

Note: let is lazily evaluated, so these would be captured on first access — you'd need to force evaluation before the before block mutates them (e.g., with let! or by referencing them in before before the mutation).

---

172-180: The first('input[type="text"]', visible: true) selector is fragile.

If the form ever gains additional visible text inputs (e.g., a slug or code field), this will silently target the wrong element. Consider scoping to the active tab panel or using a more specific selector tied to the locale field name, e.g.:

find('input[name*="[name_en]"]')
# or scope within the visible panel:
within(".tabs-panel.is-active") { fill_in ... }

This is fine for now given the current form structure.

decidim-admin/spec/system/admin_manages_taxonomies_spec.rb (1)

145-166: Restore original locales instead of hard-coding defaults.

Hard-coding %w(en ca es) in the teardown couples this spec to a specific global default and can leak into later specs if defaults change. Capture and restore the previous values for better test isolation.

♻️ Suggested change

       before do
+        `@original_i18n_locales` = I18n.available_locales
+        `@original_decidim_locales` = Decidim.available_locales
         I18n.available_locales = %w(en ca es fr it)
         Decidim.available_locales = %w(en ca es fr it)
         I18n.backend.reload!
@@
       after do
-        I18n.available_locales = %w(en ca es)
-        Decidim.available_locales = %w(en ca es)
+        I18n.available_locales = `@original_i18n_locales`
+        Decidim.available_locales = `@original_decidim_locales`
         I18n.backend.reload!

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@decidim-admin/spec/system/admin_manages_taxonomies_spec.rb` around lines 145
- 166, Store the current I18n.available_locales and Decidim.available_locales
into local variables at the start of the spec (before you change them), then in
the after block restore those saved values and call I18n.backend.reload!; update
the teardown to use the saved variables instead of the hard-coded %w(en ca es)
so I18n.available_locales and Decidim.available_locales are returned to their
original states (keep the existing remove_const/load and organization.update!
handling unchanged).