Skip to content

Fix verification route issues#8146

Merged
leio10 merged 2 commits intodecidim:developfrom
mainio:fix/verification-route-issues
Jun 23, 2021
Merged

Fix verification route issues#8146
leio10 merged 2 commits intodecidim:developfrom
mainio:fix/verification-route-issues

Conversation

@ahukkanen
Copy link
Copy Markdown
Contributor

@ahukkanen ahukkanen commented Jun 18, 2021

🎩 What? Why?

There are some situations when the verification adapter raises very obfuscated error messages, such as:

  • The admin or main engine have not been defined for the verification workflow
  • The verification workflow is missing some of the routes assumed for "multi step" workflows (although it's not really necessary)
  • When trying to call these route methods for direct verification types as reported at InvalidDirectVerificationRoute is not defined  #8145

This improves the exceptions in these situations to better identify the problem with the workflow. This also adds specs for these cases.

📌 Related Issues

Testing

  • Create a multi-step workflow and define both main and admin engine for it
  • Do NOT define the "edit" and "renew" routes for the workflow
  • Configure an action where this workflow is required (e.g. voting)
  • Manually create an authorization for the current user in the database for this workflow but leave the granted_at field nil
  • Try to vote, you will see a very non-descriptive error message with non-descriptive stack trace in the console

📋 Checklist

  • 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

This is what the user sees in these situations:
Obfuscated user error

It does not really matter as it should never happen when the workflow is configured accordingly to the database data.

But the stacktrace exceptions are now improved in order to identify what causes this issue.

@leio10 leio10 added type: fix PRs that implement a fix for a bug module: verifications labels Jun 23, 2021
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 improvement!! 😍

@leio10 leio10 merged commit 4e1c599 into decidim:develop Jun 23, 2021
@ahukkanen ahukkanen deleted the fix/verification-route-issues branch June 23, 2021 14:40
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)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

InvalidDirectVerificationRoute is not defined

2 participants