Resolve "Can't change email for group notifications"
What does this MR do and why?
This MR fixes the problem mentioned in #331139 (closed)
Root cause:
This route (PUT /-/profile/notifications/groups/:id
) currently works for top-level groups, but not for subgroups.
This is because subgroup paths have forward slashes in them, so the route formed is like, PUT /-/profile/notifications/groups/my-group/my-subgroup
.
However, the current definition of the route is of the form /profile/notifications/groups/:id
, and the :id
here does not support having forward slashes in them. When this route is currently hit for subgroups, it returns a 404. (However, for top level groups this works fine because there is no forward slash in the :id
part.)
If we wish to support forward slashes in params, we should use wildcard/glob params, ie *id
, so that the route definition becomes /profile/notifications/groups/*id
. To be RESTful, we have modified the route a bit to be of the form /profile/groups/*id/notifications
However, we can't have a param in resources
be a wildcard, so we have to make this specific route manually (using match
) without depending on resources
, which is what this MR does.
After this change, now that the route definition supports forward slashes because of the wildcard (*id
), the route starts working for subgroups.
Screenshots or screen recordings
Before
After
How to set up and validate locally
- Go to
/-/profile/notifications
- Try to change your email for any subgroups you have (you need to have more than 1 email addresses).
- You should be able to change the email successfully and not hit a 404.
MR acceptance checklist
This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.
-
I have evaluated the MR acceptance checklist for this MR.