Skip to content

Fix access to import CSV results in accountability#8132

Merged
leio10 merged 2 commits intodecidim:developfrom
CodiTramuntana:fix/no_method_error-when_importing_results
Jun 23, 2021
Merged

Fix access to import CSV results in accountability#8132
leio10 merged 2 commits intodecidim:developfrom
CodiTramuntana:fix/no_method_error-when_importing_results

Conversation

@tramuntanal
Copy link
Copy Markdown
Contributor

@tramuntanal tramuntanal commented Jun 14, 2021

🎩 What? Why?

decidim-accountability/app/controllers/decidim/accountability/admin/import_results_controller.rb was incorrectly overriding the methods current_component & current_participatory_process, this PR removes this overrides. The override was not taking into account when accountability was in an assembly.

📌 Related Issues

Link your PR to an issue

Testing

Go to the accountability component of an assembly and click the "Import CSV" button.

📋 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

In an Assembly
imatge

In a Participatory Process
imatge

♥️ Thank you!

The override was not taking into account when accountability was in an assembly.
@tramuntanal
Copy link
Copy Markdown
Contributor Author

Hi @leio10 I've added a test to cover both scenarios, when the component is in a participatory_process and when it is in an assembly. It seems codecov/patch is asking for tests in resuts_controller which was not affected by the PR.

Let me know if you need something else.

@tramuntanal tramuntanal changed the title Do not override already existing methods Fix import CSV results to accountability Jun 17, 2021
@tramuntanal tramuntanal changed the title Fix import CSV results to accountability Fix access to import CSV results in accountability Jun 17, 2021
@leio10 leio10 added module: accountability type: fix PRs that implement a fix for a bug in-review labels Jun 23, 2021
@leio10 leio10 merged commit 2ce24ba into decidim:develop Jun 23, 2021
@leio10
Copy link
Copy Markdown
Contributor

leio10 commented Jun 23, 2021

Thanks @tramuntanal! ❤️ Can you backport this PR for the version 0.24?

@tramuntanal tramuntanal deleted the fix/no_method_error-when_importing_results branch June 28, 2021 10:25
tramuntanal added a commit to CodiTramuntana/decidim that referenced this pull request Jun 28, 2021
entantoencuanto added a commit that referenced this pull request Jun 29, 2021
* develop: (47 commits)
  New Crowdin updates (#8150)
  Move the webpacker config override to @decidim/webpacker (#8158)
  Fix admin stylesheet dynamic imports (#8154)
  Fix session timeout conflicting with remember me (#7467)
  Allow to create online meetings without an URL (#8152)
  Fix verification route issues (#8146)
  Fix dont save timeout path to session (#8142)
  Fix access to import CSV results in accountability (#8132)
  Fix user report notification reported user name (#8130)
  Allow users to comment and delete their own comments (#8072)
  New Crowdin updates (#8124)
  Fix webpacker issues (#8136)
  Add comments in participatory space presentation page stats block (#8034)
  Split NPM dependencies to more granular packages (#8121)
  Metric is not shown when value is zero for blocked and reported users (#8117)
  Add missing templates translations (#8133)
  Add missing translation for authorization_modals (#8129)
  Polls in meetings (#8065)
  Fix flaky test on initiatives (#8128)
  Filter participants admin (#8104)
  ...
tramuntanal added a commit to CodiTramuntana/decidim that referenced this pull request Jul 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

NoMethodError when importing accountability results

2 participants