Skip to content

Use symbols for polymorphic route arguments in Scope Type admin#8052

Merged
leio10 merged 1 commit intodecidim:developfrom
CodiTramuntana:fix/scope_types_admin
May 25, 2021
Merged

Use symbols for polymorphic route arguments in Scope Type admin#8052
leio10 merged 1 commit intodecidim:developfrom
CodiTramuntana:fix/scope_types_admin

Conversation

@tramuntanal
Copy link
Copy Markdown
Contributor

@tramuntanal tramuntanal commented May 25, 2021 •
edited
Loading

🎩 What? Why?

Please describe your pull request.
Scope Type admin is crashing due to the use of strings for polymorphic route arguments in Scope type related admin views.

ActionView::Template::Error (Please use symbols for polymorphic route arguments.):
    1: <div class="card">
    2:   <div class="card-divider">
    3:     <% if allowed_to? :creste, :scope_type %>
    4:       <h2 class="card-title"><%= t "decidim.admin.titles.scope_types" %> <%= link_to t("actions.add", scope: "decidim.admin"), ["new", "scope_type"], class: "button tiny button--title" %></h2>
    5:     <% end %>
    6:   </div>
    7:   <div class="card-section">
  
actionpack (5.2.6) lib/action_dispatch/routing/polymorphic_routes.rb:296:in `block in handle_list'
actionpack (5.2.6) lib/action_dispatch/routing/polymorphic_routes.rb:291:in `map'
actionpack (5.2.6) lib/action_dispatch/routing/polymorphic_routes.rb:291:in `handle_list'
actionpack (5.2.6) lib/action_dispatch/routing/polymorphic_routes.rb:219:in `polymorphic_method'
actionpack (5.2.6) lib/action_dispatch/routing/polymorphic_routes.rb:140:in `polymorphic_path'
actionview (5.2.6) lib/action_view/routing_url_for.rb:103:in `url_for'
actionview (5.2.6) lib/action_view/helpers/url_helper.rb:202:in `link_to'
/home/oliver/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/bundler/gems/decidim-70444b614758/decidim-admin/app/views/decidim/admin/scope_types/index.html.erb:4:in `__home_oliver__rbenv_versions_______lib_ruby_gems_______bundler_gems_decidim______b_______decidim_admin_app_views_decidim_admin_scope_types_index_html_erb___4149322209680745505_245540'

📌 Related Issues

Link your PR to an issue

Testing

Describe the best way to test or validate your PR.
As an admin, I should be able to manage scope types.

📋 Checklist

🚨 Please review the guidelines for contributing to this repository.

  • ❓ CONSIDER adding a unit test if your PR resolves an issue.
  • ✔️ DO check open PR's to avoid duplicates.
  • ✔️ DO keep pull requests small so they can be easily reviewed.
  • ✔️ DO build locally before pushing.
  • ✔️ DO make sure tests pass.
  • ✔️ DO make sure any new changes are documented in docs/.
  • ✔️ DO add and modify seeds if necessary.
  • ✔️ DO add CHANGELOG upgrade notes if required.
  • ✔️ DO add to GraphQL API if there are new public fields.
  • ✔️ DO add link to MetaDecidim if it's a new feature.
  • ❌AVOID breaking the continuous integration build.
  • ❌AVOID making significant changes to the overall architecture.

📷 Screenshots

Please add screenshots of the changes you're proposing
Description

♥️ Thank you!

@tramuntanal
Copy link
Copy Markdown
Contributor Author

tramuntanal commented May 25, 2021

Hi @leio10 the failing test is due to

npm ERR! code ECONNRESET
npm ERR! network aborted
npm ERR! network This is a problem related to network connectivity.

can you re-run it please?

@mrcasals mrcasals mentioned this pull request May 25, 2021
Closed
@leio10 leio10 added in-review module: admin type: fix PRs that implement a fix for a bug labels May 25, 2021
leio10
leio10 approved these changes May 25, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@leio10 leio10 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch!

@leio10 leio10 merged commit be9fd5c into decidim:develop May 25, 2021
@mrcasals
Copy link
Copy Markdown
Contributor

mrcasals commented May 25, 2021

It's worth noting that we need to add some system tests to this page. This failing means we don't have any test there :(

@leio10 leio10 mentioned this pull request May 25, 2021
Merged
@tramuntanal tramuntanal deleted the fix/scope_types_admin branch May 26, 2021 07:52
@tramuntanal tramuntanal mentioned this pull request May 26, 2021
Merged
entantoencuanto added a commit that referenced this pull request May 31, 2021
@entantoencuanto
Merge branch 'develop' into active_storage_migration
cad4728 
* develop: (59 commits)
  Update supported versions in docs (#8079)
  Meetings merge minutes and close actions (#7968)
  Meeting calendars providers (#7944)
  Fix broken test on meetings after merging PR without rebase (#8076)
  Show participants list in meetings (#7933)
  Security feature external link warning (#7397)
  Add missing tests for scope types admin page (#8053)
  Use symbols for polymorphic route arguments (#8052)
  Mockup design for Participation statistics tables in Votings (#7879)
  Fix boolean fields for .reported? and .hidden? which is nil if no report exists (#7990)
  Fix redirects broken by Terms and Conditions redirect (#8036)
  Amend CSS overwritting (#8007)
  New Crowdin updates (#8048)
  Fix undetected broken tests because of missing dependencies (#8050)
  Validate results by Monitoring Committee Members (#7899)
  Electoral certificate validation by Monitoring Committee Members (#7871)
  Publish and unpublish a meeting (#7893)
  New Crowdin updates (#8005)
  Polling station closure attach the physical electoral closure certificate (#7929)
  Fix attachment title migration generating possibly invalid values (#8020)
  ...
@moustachu moustachu mentioned this pull request Jun 18, 2021
Closed
@armandfardeau armandfardeau mentioned this pull request Jun 21, 2021
Merged
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@leio10 leio10 leio10 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

in-review module: admin type: fix PRs that implement a fix for a bug

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Rails 6.0.3.7 Regression - Please use symbols for polymorphic route arguments.

3 participants

@tramuntanal @mrcasals @leio10