Skip to content

Remove user from rotation if removed from project/group

Sean Arnold requested to merge 323631-remove-participant-on-removal into master

What does this MR do?

This adds a service to remove a user from an On-call Rotation.

We ensure that the rotation is up to date by running the job top persist current shifts. Then, we update the is_removed boolean on the Participant model associated to the user/rotation.

Deletion from rotations are scoped to the member being removed:

  • If a member is a project member, only that project's rotations are affected
  • If a member is a group member, all of the projects within that group are affected

Why don't we use the existing EditRotationService service? The EditRotationService requires you to provide all participant params for all remaining participants, and in a format which requires the user, color_palette, color_weight etc. Rather than modify the EditRotationService to make this easier, I chose to make a specific service for this use case.

SQL queries for finder:

Single Project:

Explain: https://explain.depesz.com/s/QToy

SQL:

  User Load (0.5ms)  SELECT "users".* FROM "users" WHERE "users"."id" = 1 LIMIT 1 /*application:console,line:/ee/app/finders/incident_management/member_oncall_rotations_finder.rb:11:in `initialize'*/

  Project Load (0.5ms)  SELECT "projects".* FROM "projects" WHERE "projects"."id" = 4302 LIMIT 1 /*application:console,line:/ee/app/finders/incident_management/member_oncall_rotations_finder.rb:15:in `execute'*/
  
  IncidentManagement::OncallRotation Load (4.5ms)  SELECT "incident_management_oncall_rotations".* FROM "incident_management_oncall_rotations" INNER JOIN "incident_management_oncall_participants" ON "incident_management_oncall_rotations"."id" = "incident_management_oncall_participants"."oncall_rotation_id" INNER JOIN "incident_management_oncall_schedules" ON "incident_management_oncall_schedules"."id" = "incident_management_oncall_rotations"."oncall_schedule_id" WHERE "incident_management_oncall_participants"."user_id" = 1 AND "incident_management_oncall_schedules"."project_id" = 4302 

Group member, multiple projects:

Explain: https://explain.depesz.com/s/3sxa

SQL:

  User Load (0.7ms)  SELECT "users".* FROM "users" WHERE "users"."id" = 1 LIMIT 1

  Namespace Load (0.5ms)  SELECT "namespaces".* FROM "namespaces" WHERE "namespaces"."id" = 4374 LIMIT 1

  IncidentManagement::OncallRotation Load (1.1ms)  SELECT "incident_management_oncall_rotations".* FROM "incident_management_oncall_rotations" INNER JOIN "incident_management_oncall_participants" ON "incident_management_oncall_rotations"."id" = "incident_management_oncall_participants"."oncall_rotation_id" INNER JOIN "incident_management_oncall_schedules" ON "incident_management_oncall_schedules"."id" = "incident_management_oncall_rotations"."oncall_schedule_id" WHERE "incident_management_oncall_participants"."user_id" = 1 AND "incident_management_oncall_schedules"."project_id" IN (SELECT "projects"."id" FROM "projects" WHERE "projects"."namespace_id" = 4374)

Screenshots (strongly suggested)

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

  • Label as security and @ mention @gitlab-com/gl-security/appsec
  • The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
  • Security reports checked/validated by a reviewer from the AppSec team

Related to #323631 (closed)

Edited by Dmytro Zaporozhets (DZ)

Merge request reports