Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

An issue with collections in nested_select, in v2 #505

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

dkniffin
Copy link
Contributor

Motivation / Background

I am working on upgrading activeadmin_addons to v2 (among other upgrades) in my codebase, and I am experiencing a weird issue. I have an nested_select input like this:

                p.input :foo, as: :nested_select,
                  level_1: {
                    attribute: :bar,
                    collection: Bar.all
                  },
                  level_2: {
                    attribute: :baz,
                    display_name: :title,
                    collection: Baz.all,
                    required: false,
                    method_model: Baz
                  }

When I render the page, slim-select renders correctly, and. the currently selected item shows up in the list. However, no other elements from the collection show in the list (Bar.all in this case)

I created this PR as a way to show a failing test case (assuming it actually fails. If not, then it's something wrong on my side 😅 )

Detail

This Pull Request adds a failing test case for the issue described above

Checklist

Before submitting the PR make sure the following are checked:

  • This Pull Request is related to one change. Changes that are unrelated should be opened in separate PRs.
  • Commit message has a concise description of what changed and why.
  • Tests are added or updated if you fix a bug or add a feature.
  • Documentation has been added or updated if you add a feature or modify an existing one.
  • CHANGELOG files are updated for the changed libraries if there is a behavior change or additional feature (under the "Unreleased" heading if this is not a version change).
  • My changes don't introduce any linter rule violations.

@dkniffin
Copy link
Contributor Author

Yep, looks like this is in fact a bug in the library. Here's the error:

  1) Nested Select Input with selected city shows filled select controls based on defined city_id
     Failure/Error: expect(page).to have_css(".ss-main", text: /#{text}/)
       expected to find visible css ".ss-main" with text /Antofagasta/ within #<Capybara::Node::Element tag="li" path="/HTML/BODY[1]/DIV[1]/DIV[4]/DIV[1]/DIV[1]/FORM[1]/FIELDSET[1]/OL[1]/LI[2]"> but there were no matches. Also found "Metropolitana", which matched the selector but not all filters. 
     # ./spec/support/capybara_helpers.rb:89:in `expect_slimselect_selection'
     # ./spec/features/inputs/nested_select_input_spec.rb:72:in `block (4 levels) in <top (required)>'

I'll see if I can find a fix

@dkniffin
Copy link
Contributor Author

I'm unable to test it in my local environment, but I think this is the fix:

diff --git a/app/javascript/activeadmin_addons/inputs/slim-select-nested.js b/app/javascript/activeadmin_addons/inputs/slim-select-nested.js
index f772baf..31a7a62 100644
--- a/app/javascript/activeadmin_addons/inputs/slim-select-nested.js
+++ b/app/javascript/activeadmin_addons/inputs/slim-select-nested.js
@@ -85,6 +85,9 @@ function settings(el) {
         return ajaxSearch(search, currentData, args);
       },
     },
+    data: el.dataset.collection.map((item) => {
+      return { text: item.text, value: item.id }
+    })
   };
 }

That said, I'm going to workaround this issue in my own app. But this is definitely with nested_select and should be resolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

1 participant