Backport 'Redirect non-admin users to core's root_path' to v0.27 #11935
Conversation
* Redirect non-admin users to core's root_path * Add spec * Remove redirection of non-admin users to core's root_path * Prevent redirection to TOS page if the user isn't allowed * Check that we're not leaking routes information on redirections * Extract method to check if a user is a moderator in a ParticipatorySpace * Check if the user has any role in the TOS acceptance logic * Remove unecessary return from method * Test for correct accesses of the spaces' admins pages Done for: * decidim-assemblies * decidim-conferences * decidim-participatory_spaces As they are mostly the same related in the permissions logics: there are participatory space admins and valuators. Others modules (such as decidim-initiatives and decidim-elections with the Votings spaces) don not have these roles. * Test for correct access of the decidim-initiatives module * Fix bug when there isn't any published conference We were redirecting (status code 30x) to a 404 page. What you need to do is to actually present a status code 404 (as that's the HTTP status code for a NotFound document) This was detected with the last specs changes :) * Check if the user is a votings' monitoring commitee member in the TOS acceptance * Check if the user is has a participatory space role in the TOS acceptance Done for: * decidim-assemblies * decidim-conferences * decidim-participatory_spaces * Test for correct accesses of the spaces' admins pages (part II) Add valuator role * Add spec for decidim-verifications admin route * Move routes definitions for decidim-templates to decidim-admin If we don't do this, it's not respecting the decidim-admin constraints that are already defined. This was detected with a redirect when there was a normal user. Instead of giving a 404 in /admin/templates/questionnaire_templates, it was giving a redirection (301) to /admin. Moving the definition of the routes to the decidim-admin module honors all the contracts from the admin. * Fix routes generation for decidim-templates move * Remove unecessary comment * Prefer usage of instead of Suggested by code review Co-authored-by: Alexandru Emil Lupu <contact@alecslupu.ro> * Use method Suggested by code review Co-authored-by: Alexandru Emil Lupu <contact@alecslupu.ro> * Require the examples only on the needed files * Rename shared example to 'accessing the participatory space' * Remove unecessary shared example for ParticipatoryProcesses * Fix typo when checking if module exists * Prefer usage of instead of Suggested by code review --------- Co-authored-by: Alexandru Emil Lupu <contact@alecslupu.ro>
andreslucena
added
backport
alecslupu
deleted the
backport/0.27/redirect-non-admin-users-to-co-10968
branch
|
This backport caused a change in the API (removed method @ |
IMO we should focus on not removing public methods for backports, but adding new methods it's OK if they're necessary (see #12425 as an example of this) Should we re-add the snippet for v0.27 then? |
Agreed, I would aim not to break the public API for patch releases. Adding new methods does not generally break the backwards compatibility but also as you said, only add them if absolutely necessary. The aim should be to keep API the same for PATCH releases. Changing the method implementation is totally fine if the API remains the same. Adding new method is acceptable if it is reasoned well and seems necessary, as long as backwards compatibility is not broken.
Up to you. This broke the customized notification settings view in the privacy module which is why I investigated what caused that. I don't think anyone else is using that module than us right now, so we can live with this. Just wanted to raise awareness to avoiding this type of changes. |
馃帺 What? Why?
Backport #10968 to v0.27