Skip to content

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

Screen_Shot_2021-12-14_at_9.43.42_AM

After

Screen_Shot_2021-12-15_at_10.51.35_AM

How to set up and validate locally

  1. Go to /-/profile/notifications
  2. Try to change your email for any subgroups you have (you need to have more than 1 email addresses).
  3. 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.

Edited by Manoj M J

Merge request reports