Skip to content

Take 2 for Fix cross joins in member_owners_excluding_project_bots

What does this MR do and why?

Related issue: #427638 (closed), #417455 (closed)

  1. We restore !133391 (merged)

  2. Add missing test cases for member_owners_excluding_project_bots

We add missing test cases for this method involving group share with groups

  1. Fix broken code arising from step 2. See each commit

Screenshots or screen recordings

This is a normalized query from production while we had the bug of #427638 (closed). Note the second half of the union was missing WHERE... "invite_token" IS NULL.

SELECT "members".* FROM ((SELECT "members"."id", "members"."access_level", "members"."source_id", "members"."source_type", "members"."user_id", "members"."notification_level", "members"."type", "members"."created_at", "members"."updated_at", "members"."created_by_id", "members"."invite_email", "members"."invite_token", "members"."invite_accepted_at", "members"."requested_at", "members"."expires_at", "members"."ldap", "members"."override", "members"."invite_email_success", "members"."state", "members"."member_namespace_id", "members"."member_role_id", "members"."expiry_notified_at" FROM "members" WHERE "members"."type" = $1 AND "members"."source_type" = $2 AND (members.access_level > $3) AND "members"."source_id" IN (SELECT "namespaces"."id" FROM "namespaces" WHERE "namespaces"."type" = $4 AND "namespaces"."id" IN ($5, $6, $7)) AND "members"."state" = $8 AND "members"."requested_at" IS NULL AND "members"."invite_token" IS NULL AND (members.access_level > $9))
UNION
(WITH "group_group_links_cte" AS MATERIALIZED (SELECT "group_group_links".* FROM "group_group_links" WHERE "group_group_links"."shared_group_id" IN (SELECT "namespaces"."id" FROM "namespaces" WHERE "namespaces"."type" = $15 AND "namespaces"."id" IN ($16, $17, $18))) SELECT "members"."id", LEAST("group_group_links"."group_access", "members"."access_level") AS access_level, "members"."source_id", "members"."source_type", "members"."user_id", "members"."notification_level", "members"."type", "members"."created_at", "members"."updated_at", "members"."created_by_id", "members"."invite_email", "members"."invite_token", "members"."invite_accepted_at", "members"."requested_at", "members"."expires_at", "members"."ldap", "members"."override", "members"."invite_email_success", "members"."state", "members"."member_namespace_id", "members"."member_role_id", "members"."expiry_notified_at" FROM "members", "group_group_links_cte" AS "group_group_links" WHERE "members"."type" = $10 AND "members"."source_type" = $11 AND "members"."requested_at" IS NULL AND "members"."source_id" = "group_group_links"."shared_with_group_id" AND "members"."source_type" = $12 AND "members"."state" = $13 AND (members.access_level > $14))) members WHERE "members"."type" = $19 AND "members"."source_type" = $20

Here is a SQL from test from this branch. We now filter by WHERE... "invite_token" IS NULL.

SELECT "members"."id" FROM ((SELECT "members"."id", "members"."access_level", "members"."source_id", "members"."source_type", "members"."user_id", "members"."notification_level", "members"."type", "members"."created_at", "members"."updated_at", "members"."created_by_id", "members"."invite_email", "members"."invite_token", "members"."invite_accepted_at", "members"."requested_at", "members"."expires_at", "members"."ldap", "members"."override", "members"."state", "members"."invite_email_success", "members"."member_namespace_id", "members"."member_role_id", "members"."expiry_notified_at" FROM "members" WHERE "members"."type" = 'GroupMember' AND "members"."source_type" = 'Namespace' AND (members.access_level > 5) AND "members"."source_id" IN (SELECT "namespaces"."id" FROM "namespaces" WHERE "namespaces"."type" = 'Group' AND "namespaces"."id" IN (1507, 1509)) AND "members"."state" = 0 AND "members"."requested_at" IS NULL AND "members"."invite_token" IS NULL AND (members.access_level > 5))
UNION
(WITH "group_group_links_cte" AS MATERIALIZED (SELECT "group_group_links".* FROM "group_group_links" WHERE "group_group_links"."shared_group_id" IN (SELECT "namespaces"."id" FROM "namespaces" WHERE "namespaces"."type" = 'Group' AND "namespaces"."id" IN (1507, 1509))) SELECT "members"."id", LEAST("group_group_links"."group_access", "members"."access_level") AS access_level, "members"."source_id", "members"."source_type", "members"."user_id", "members"."notification_level", "members"."type", "members"."created_at", "members"."updated_at", "members"."created_by_id", "members"."invite_email", "members"."invite_token", "members"."invite_accepted_at", "members"."requested_at", "members"."expires_at", "members"."ldap", "members"."override", "members"."state", "members"."invite_email_success", "members"."member_namespace_id", "members"."member_role_id", "members"."expiry_notified_at" FROM "members", "group_group_links_cte" AS "group_group_links" WHERE "members"."type" = 'GroupMember' AND "members"."source_type" = 'Namespace' AND "members"."requested_at" IS NULL AND "members"."source_id" = "group_group_links"."shared_with_group_id" AND "members"."source_type" = 'Namespace' AND "members"."state" = 0 AND (members.access_level > 5))) members WHERE "members"."type" = 'GroupMember' AND "members"."source_type" = 'Namespace' AND "members"."access_level" = 50 AND "members"."invite_token" IS NULL AND "members"."id" >= 506 ORDER BY "members"."id" ASC LIMIT 1 OFFSET 1000 /*application:test,correlation_id:5464dd29b65dfc16cddc7a690482cb27,db_config_name:main,line:/app/models/concerns/each_batch.rb:81:in `block in each_batch'*/

How to set up and validate locally

  1. Revert the latest commit (Fix method returning invited members by mistake)
  2. Run bundle exec rspec member_owners_excluding_project_bots -e member_owners_excluding_project_bots. You should see a failure that matches #427638 (closed)

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 Thong Kuah

Merge request reports