Fix error when routing with array containing symbol. #5870
Merged
Conversation
This file contains 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
JWesorick
force-pushed
the
5869-allow-symbol-in-polymorphic-routes
branch
2 times, most recently
from
24c3cc7
to
701880e
Compare
|
@deivid-rodriguez I think this is ready to merge. The one failing spec looks like it just timed out. |
|
Thank you! I think if you rebase this PR on top of latest master, the timeout should go away, since it should have been fixed by #5858. Also, can you add a changelog entry following the existing changelog format for your fix? Thanks! ❤️ |
JWesorick
force-pushed
the
5869-allow-symbol-in-polymorphic-routes
branch
3 times, most recently
from
9e7e5e8
to
2cc6607
Compare
|
@deivid-rodriguez I think this is ready but something is up with the checks. They just never finished. |
|
@JWesorick I believe @deivid-rodriguez fixed the issue in #5883. We just merged that in. Can you please rebase again and push? That should hopefully fix the CI build. Thanks! ❤️ |
|
Yes, I think it should be all good now. |
JWesorick
force-pushed
the
5869-allow-symbol-in-polymorphic-routes
branch
from
2cc6607
to
40893c4
Compare
JWesorick
force-pushed
the
5869-allow-symbol-in-polymorphic-routes
branch
2 times, most recently
from
843235b
to
ec33961
Compare
JWesorick
force-pushed
the
5869-allow-symbol-in-polymorphic-routes
branch
from
ec33961
to
fde9a76
Compare
amilano
added a commit
to grafiti-io/activeadmin
that referenced
this pull request
Oct 28, 2019
…epo (#1) * Fix order of versions for diff of 2.1.0 release (activeadmin#5769) Putting the newer version last ensures that we get the expected diff. Otherwise, we see additions and removals switched. * Configure GitHub sponsors (activeadmin#5771) To give more visibility to the different funding options. * Update fr.yml (activeadmin#5748) * Update pundit links to canonical repo * Use bundler 2.0.2 in CI (activeadmin#5774) * Fix typo in authorization adapter documentation Co-Authored-By: Javier Julio <javierjulio@icloud.com> * Add a minimal git history to generated test apps (activeadmin#5775) It's very common to make experimental modifications to test applications. This makes it very easy to go back to the freshly generated application without having to regenerate it, by running `git checkout .`, or to even keep track of the experimental changes being done using git. * Fix pundit policy retrieving on an edge case (activeadmin#5777) When retrieving the policy for an `ActiveAdmin::Page`, and the pundit namespace is `:active_admin`, we need an extra check so that the right policy is properly picked up. * Add Macedonian (mk) locale (activeadmin#5710) * Drop unnecessary stuff (activeadmin#5780) We no longer need this, since we support rubies above or equal to 2.4. * Fix indent of permitted parameters boilerplate code * Symmetric spacing The blank line before class end is hard to eliminate without making the template messy. So I add a blank line before class begin so that the generated class is a bit prettier. * Use a consistent style for `permit_params` * Remove unneeded new lines * Move sample app page to its own file * Move sample app resources to their own page * Move test models to their own files * Generate migrations instead of full models The model generator only generates: * Model, which we are fully overwriting later. * Unit tests, which we don't use. * Migration. So using the migration generator seems better. * Add a binstub for rubocop * Enable `Layout/SpaceAroundOperators` cop * Fix boilerplate code indentation Broken when the template was reindented. * List candidate fields for `permit_params` Co-authored-by: coezbek <christopher@oezbek.org> * Add some extra docs Co-authored-by: coezbek <christopher@oezbek.org> * Also use the candidate list in alternative syntax * Relax dependency on railties (activeadmin#5786) This gets us ready to support the final release of Rails 6.0, and also allows activeadmin to be tried against Rails master more easily. * Re-use I18n translations of formtastic ~ Labels (activeadmin#5764) * Re-use I18n translations of formtastic ~ Labels * Undo unnecessary change and add a spec * Simplify * Fix migration generator against Rails master (activeadmin#5788) Current Rails master (6.1.0.alpha) adds a new --timestamps option to the migration generator that it's true by default. So to get migrations running under Rails master, we need to make sure we don't generate duplicate timestamp columns by using both explicit columns and the new (default) option at the same time. * Skip error when running `bin/rake local` (activeadmin#5789) Normally, you would run `bin/rake local server`, `bin/rake local console`, or any other Rails command as the second argument. That creates a test application with seed data if it doesn't already exist, and then runs the specified Rails command on it. However, if you just run `bin/rake local`, it will generate the application but then fail with the error `bundler: exec needs a command to run`. Instead of doing that, only generate the test application and exit without error. * Persist the correct Gemfile to each test app (activeadmin#5790) With this, we no longer need to mess with `BUNDLE_GEMFILE`'s when booting applications for different Rails versions, and each application will always use the right Gemfile. We can even boot applications from the root folder like this: `tmp/rails/rails-6.0.0.rc1/bin/rails s` and it will just work. * Use double quotes for consistency All other locales are using double quotes for the nested keys in the status_tag block except for this one. * Separate status_tag `nil` case for optional locale customization When using the `status_tag` component with boolean fields, both `nil` and `false` were treated the same and had the same output but without a way to modify that other than overriding the component. These 2 conditions are now split, so for the `nil` case it returns "Unset" but the locale will keep a display value of "No" for that locale key. That **will maintain backwards compatibility** so when `status_tag` is called with `nil` or `false` it will continue to display the same "No" label but provide the option to customize the label for the `nil` case only, leaving the `false` one as is. Further, we need to ensure the same CSS class for presentation due to the change, so the class list output now includes "no" and "unset". * Update all locales with status_tag "unset" key For each locale, the value of "unset" is just defaulted to the same value from the "no" key to preserve existing behavior but enabling a user to customize the display label. * bundle update yard (activeadmin#5798) * Update status_tag docs regarding boolean values * Add changelog for separate status_tag nil/false labels * Enable `Style/ParallelAssignment` cop (activeadmin#5803) * Make sure show page title is escaped (activeadmin#5802) * Extract an `app_name` method in the generator * Extract a `gemfile_from_env` method * Base test app names off the current Gemfile * Pass gemfile full path to Rails generator * Keep instantiated generator So we can ask any information about the generated application that we need. * Rename test application class Since it's used for more than generating the application, but also for asking information about it. * Ask expanded gemfile to test application * Ask full path to the test application class * Track coverage for test app creation too * bin/rake bundle[update,simplecov] * Add a way to forcefully create the test application I find myself running `rm -rf tmp/rails/rails_52` or whatever and then running `bin/rake setup` to recreate the test application. Now I can do `bin/rake setup:force`. * Consistency rename * Move test-report persistance to separate task Otherwise we will accidentally restore coverage files created by the test app creation jobs, and that will cause conflicts. * Revert "Ensure application gets reloaded only once" (activeadmin#5801) This reverts commit 21b6138, because it created a performance regression in development when reloading code. * Use non vulnerable `lodash` for `gherkin-lint` (activeadmin#5806) * Get ready for 2.2.0 release (activeadmin#5805) * Bump Rails to latest 6.0.0.rc2 Rails recently released version 6.0.0.rc2 https://github.com/rails/rails/releases/tag/v6.0.0.rc2 * fix csv Builder ignoring global humanize_name setting * add features testing global csv humanize_name setting * add changelog entry * Use the ransack master version because is compatible with Rails 6.0.0.rc2 * Remove unnecessary condition * Extract an utility method * Remove blank line for consistency * Fix crash when filtering by nested resource * Make posts a nested resource (activeadmin#5814) So we have an example of a nested resource in our test application. * Don't abort `bin/rake` when test application exists (activeadmin#5813) * Remove uneeded skips (activeadmin#5818) * Another test app generation fix (activeadmin#5819) * Fix `belongs_to` specification (activeadmin#5817) I needed to change to association name to make filtering by post in the user's index page, otherwise, the applied filter cannot be properly displayed. But then I need to make sure routes and parameters still use "user". * Hide "Last" link when `pagination_total` is false (activeadmin#5822) * Update documentation for including associated objects (activeadmin#5824) * No need to depend on specific plugins `github-pages` already depends on the most usual plugins. * Make sure the default layout is applied to pages Otherwise we get unstyled pages locally. * Make sure markdown relative links are handled Otherwise we get broken links to non converted .md pages in development. * Uncomment commented out step * Rename feature to better match what it tests * Remove unneeded blank line * Support optional belongs_to with renamed resources Co-authored-by: Kris Dekeyser <kris.dekeyser@libis.be> Co-authored-by: David Rodríguez <deivid.rodriguez@riseup.net> Co-authored-by: Nathan Broadbent <git@ndbroadbent.com> * Fix a few more issues By simplifying the setup to not use namespaces but use just renames of the actual classes, two more issues reproduced. * Not being able to infer the correct parent class to retrieve records from: ``` And I click "Author Articles" # features/step_definitions/index_views_steps.rb:1 undefined method `find' for nil:NilClass (NoMethodError) ./lib/active_admin/resource_controller/data_access.rb:74:in `scoped_collection' ./lib/active_admin/resource_controller/data_access.rb:57:in `find_collection' ./lib/active_admin/resource_controller/data_access.rb:44:in `collection' ./lib/active_admin/resource_controller/streaming.rb:12:in `index' ./features/step_definitions/index_views_steps.rb:2:in `/^I click "(.*?)"$/' features/renamed_resource.feature:66:in `And I click "Author Articles"' ``` Fixed by passing the correctly renamed class to inherited_resources's `belongs_to` helper. * Parent class was not being properly "translated" before calling the polymorphic route helpers: ``` And I click "Author Articles" # features/step_definitions/index_views_steps.rb:1 undefined method `new_admin_user_article_path' for #<Admin::ArticlesController:0x000055b6ee43c7b8> Did you mean? new_admin_article_path new_admin_user_password_path new_admin_author_article_path new_admin_user_session_path (ActionView::Template::Error) ./lib/active_admin/resource_controller/polymorphic_routes.rb:12:in `polymorphic_path' ``` Fixed that it by looking at the parent renamed resource to do the mapping so that the polymorphic helpers generate the correct route. * Point to devise master Since latest release is not yet Rails 6 final compatible. Changeset generated with `bundle && bin/rake bundle[update,devise]` after pointing devise to master. * Use the latest ransack release That should be compatible with all supported Rails versions. Changeset generated through `bin/rake bundle[update,ransack]` after removing the pin to ransack's master. * Test against Rails 6 final Changeset generated through `bin/rake bundle[update,rails,inherited_resources,rails-i18n]`. * Add some spacing in HTML for readability * Remove unnecessary spacing from snippet * Explain the Tidelift subscription in the homepage * Bump minimum ransack requirement to 2.3 (activeadmin#5831) * Bump nokogiri from 1.8.5 to 1.10.4 in /docs (activeadmin#5833) Bumps [nokogiri](https://github.com/sparklemotion/nokogiri) from 1.8.5 to 1.10.4. - [Release notes](https://github.com/sparklemotion/nokogiri/releases) - [Changelog](https://github.com/sparklemotion/nokogiri/blob/master/CHANGELOG.md) - [Commits](sparklemotion/nokogiri@v1.8.5...v1.10.4) Signed-off-by: dependabot[bot] <support@github.com> * Update banner text and link * Jruby 9.2.8.0 has been released (activeadmin#5834) Maybe it will perform better and we won't have to restart jruby builds anymore :) * update method name in documentation (activeadmin#5836) resource -> find_resource * Add a "For enterprise" section in README * Use a specific link for the support section To find out which kind of language resonates more. * Switch to apparition capybara driver (activeadmin#5840) To remove the chromedriver requirement. * Test against Rails 6 final * Use Rails 6 by default * Use released devise officially supporting Rails 6 * bin/rake bundle[update,rspec-rails] * bin/rake bundle[update,sqlite3] * Rails 5.0 still needs sqlite3 1.3 Make that explicit. * bin/rake bundle[update,cancancan] * bin/rake bundle[update,pry-byebug] * bin/rake bundle[update,cucumber-rails] * bin/rake bundle[update,parallel_tests] * bin/rake bundle[update,pundit] * bin/rake bundle[lock,--update=activerecord-jdbcsqlite3-adapter] * bin/rake bundle[lock,--update=jruby-openssl] * Use the standard cucumber formatter everywhere (activeadmin#5844) The "pretty" formatter could be helpful when debugging order dependent failures (maybe), but other than that, it's just too verbose (specially with parallel_tests), and it makes it hard to see the actual scenarios that failed and the error messages. * Make jruby hangs appear earlier (activeadmin#5847) The `parallel_tests` gem has this feature where it will output a dot every 60 seconds to avoid CIs that fail when no new output has not been displayed for a while. Our jruby build is hanging randomly and when that happens, this feature makes it take hours for the job to fail. In this commit, I'm essentially disabling this feature, by giving the heartbeat a high value, so that we actually get these failures earlier. * Normalize test app paths (activeadmin#5843) * Fix bundler cache path Since we run tests with the root user, CircleCI has been saving the bundler cache to the wrong place. * Remove unnecessary verbose flag Too verbose :) * Fully regenerate lock files Just to trigger caching changes, and see if the fix works. * Disable parallel_tests CI dots for RSpec too (activeadmin#5848) The jruby hang is happening there too. * Use rubygems 3.0.6 in CI * Bump MRI 2.4 patch level in CI to 2.4.7 * Bump MRI 2.5 patch level in CI to 2.5.6 * Reword step to read better * Use `When` instead of `Given` in a couple places Because it reads better and because the step is defined using that keyword. * Move helper method to its only usage It makes `selector_for` easier to understand. * Generate a sample application using turbolinks * Detach previous batch_action click handler before attaching new one * Detach everything before re-attaching clicks and confirm handlers * Add turbolinks entries to CircleCI * Changelog entry for the turbolinks fix * Workaround turbolinks flaky failure Apparently, when loading the posts index page, the "View" link is available without turbolinks behaviour attached for some time. I can even reproduce this on the test application by navigating to an index page, reloading the page, and quickly clicking on a "View" link: nothing happens. We would need to investigate this issue more deeply, but for now I'm changing the feature to navigate directly to the page where the tabs are, so that it doesn't hit this issue. * Remove only the events that ActiveAdmin uses Co-Authored-By: Javier Julio <JJfutbol@gmail.com> * Remove spurious comment on a locale file (activeadmin#5851) * Get ready for 2.3.0 release (activeadmin#5853) * Revert "Bump minimum ransack requirement to 2.3" (activeadmin#5854) This reverts commit d9d70c4, because ransack 2.3 has an outstanding bug that affects our supported Rails, and quite a lot of users. * Get ready for 2.3.1 release (activeadmin#5856) * Extract some common test logic for later reuse * Make count query optimisation work when using decorate_with * Remove now unneeded workaround (activeadmin#5859) * Completize missing Slovak translations (activeadmin#5861) * Make sure all PR links in changelog are implicit * Sort contributors in changelog * Avoid rendering huge associations (activeadmin#5548) Render a text filter instead of a select for large associations (opt-in) * Bump tested 2.6 ruby patch level to 2.6.5 * Bump tested 2.5 ruby patch level to 2.5.7 * Bump tested 2.5 ruby patch level to 2.4.8 * Get ready for 2.4.0 release (activeadmin#5872) * Remove redundant \_ (activeadmin#5873) * Improved german translations (activeadmin#5874) Added translations that were in en.yml but not in de.yml * Bump rubyzip from 1.2.2 to 2.0.0 in /docs (activeadmin#5875) Bumps [rubyzip](https://github.com/rubyzip/rubyzip) from 1.2.2 to 2.0.0. - [Release notes](https://github.com/rubyzip/rubyzip/releases) - [Changelog](https://github.com/rubyzip/rubyzip/blob/master/Changelog.md) - [Commits](rubyzip/rubyzip@v1.2.2...v2.0.0) Signed-off-by: dependabot[bot] <support@github.com> * Bump simplecov to 0.17.1 (activeadmin#5858) This will hopefully fix parallel_tests hanging on jruby right before completion. * Document last minute release improvement (activeadmin#5876) * Remove unneeded gitignore entry I don't think our development environment generates such folder on a non already gitignored location. * Remove another unnecessary gitignore entry I'm not sure how Rails handles this file, but we use the `BUNDLE_GEMFILE` env variable which indirectly controls the Rails version. * Remove unnecessary gitignore entry It's version manager specific, so it should live on each developer's gitignore since there's nothinhg in this repo specific to a particular ruby version manager. * Tighten some gitignore entries These folders are expected to be found on a single location at the root of the repo. Only gitignore those. * Lock sprockets in bug report template Because it's not yet working with sprockets 4. * Bump sassc-rails in bug report template * Bump rails version in bug report template * Remove redundant configuration * Move less informative assertion to the end If you can see the response body, you can figure out faster what's wrong. * Gitignore bug report template tmp folder * Drop Rails 5.0 and Rails 5.1 Since they are no longer maintained by the Rails team. * Centralize sqlite3 dev dependency again Since it's now the same requirement for all supported Rails versions. * Bump rubocop to 0.75.0 (activeadmin#5884) I needed to remove ```yaml Rails: Enabled: true ``` from the configuration, because whereas before (when Rails cops lived in the core repo) it seemed to mean "Allow manually enabling specific Rails cops", now (when Rails cops live as a separate extension) it apparently means "Enable every Rails cop". * Normalize link references in CONTRIBUNTING file Consistently use shortcut notation. * Remove unneeded stuff We commit our lockfiles, so this shouldn't happen. * Remove unintended numbering * Add a section explaining the changelog format * Add a pull request template with PR instructions * Remove duplicated sentence The bug report template starts with this exact text and link. I don't think we need to repeat it again. * Fix incorrect anchor * Use filter label when condition has a predicate (activeadmin#5886) * Move javascript sources * Change es6 extension to js * Rename 'src' to 'active_admin' within javascript * Add rollup config * Add bundled js * Replace window.activeadmin usage with es6 import * Remove sprockets-es6 as a dependency * Delete package-lock.json * Add javascript bundle checking to specs * Add auto-generated warning and blank line in bundled js * Update packaging to match current folder structure and expose js and scss * Add yarn binstub * Update documentation to explicit yarn/rollup/node requirements * Fix error when routing with array containing symbol. (activeadmin#5870) * Rename js files replacing underscore with hyphens * Move active-admin.js to utils.js * Invert js import logic * Split initializers and classes * Update base imports to use only initializers * Give each scenario only the setup it needs * Fix conflict between meta tags and `Tag` models * Remove Gemfile.common * Use existing gemfile in docs * Allow specifying custom input_html for DateRangeInput (activeadmin#5867) closes activeadmin#5864 * Move each Gemfile to its own folder * Bump bundler version in docs Gemfile * Add dependabot config * Rename some steps To distinguish them from other caching. * More explicit grouping to hint dependabot * Update `rake` to 13.0.0 It's handy to do this now to force the cache to be regenerated at the right place. * Bump rspec-rails from 3.8.2 to 3.9.0 in /gemfiles/rails_52 (activeadmin#5898) * Bump db-query-matchers from 0.9.0 to 0.10.0 in /gemfiles/rails_52 (activeadmin#5904) * Bump rspec-rails from 3.8.2 to 3.9.0 (activeadmin#5900) * Bump mdl from 0.5.0 to 0.6.0 (activeadmin#5906) * Bump db-query-matchers in /gemfiles/rails_60_turbolinks (activeadmin#5899) * Bump db-query-matchers from 0.9.0 to 0.10.0 (activeadmin#5913) * Bump rspec-rails from 3.8.2 to 3.9.0 in /gemfiles/rails_60_turbolinks (activeadmin#5912) * Bump jasmine-core from 2.9.1 to 2.99.2 (activeadmin#5911) * Bump jasmine-core from 2.9.1 to 2.99.2 in /gemfiles/rails_60_turbolinks (activeadmin#5907) * Bump jasmine-core from 2.9.1 to 2.99.2 in /gemfiles/rails_52 (activeadmin#5909) * Bump turbolinks from 5.2.0 to 5.2.1 in /gemfiles/rails_60_turbolinks (activeadmin#5905) * Use $sidebar-with to calculate right-margin of #main_content (activeadmin#5887) * Use $sidebar-with to calculate right-margin of #main_content * Add parenthesis to improve readability Co-Authored-By: Javier Julio <javierjulio@icloud.com> * Add changelog entry for activeadmin#5887 * Add definition of user and PR to changelog * Bump sprockets from 3.7.2 to 4.0.0 in /gemfiles/rails_60_turbolinks (activeadmin#5910) * Replace "Removals" with "Dependency Changes" Let's save the word "Removals" for removals that we consider to be "Breaking Changes". * Remove "Major" and "Minor" subsections This is quite subjective, and unnecessary complicates the nesting of the changelog structure. * Add some missing punctuation * Quote some codes * Bump sprockets from 3.7.2 to 4.0.0 (activeadmin#5908) * Bump github-pages from 192 to 202 in /docs (activeadmin#5897) * Bump jasmine from 2.9.0 to 2.99.0 in /gemfiles/rails_52 (activeadmin#5915) * Bump rubocop from 0.75.0 to 0.75.1 (activeadmin#5917) * Bump jasmine from 2.9.0 to 2.99.0 (activeadmin#5918) * Bump rubocop-rspec from 1.35.0 to 1.36.0 (activeadmin#5919) * Bump jasmine from 2.9.0 to 2.99.0 in /gemfiles/rails_60_turbolinks (activeadmin#5916)
toby-brilliant
added a commit
to BrilliantTechnology/activeadmin
that referenced
this pull request
Sep 7, 2022
This backports a fix that is unavailable to us because of our current Rails version (need 5.2 to bump). Addresses an issue that manifests as e.g.: > undefined local variable or method `parent' for #<Admin::StoresController:0x00007fea923418e8>
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.
See spec to reproduce issue.
Looks like this was introduced in d12dc9a#diff-9fd0a96ab1d1ed0f5d9e1258260e1cef
Fixes #5869