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)
-
We restore !133391 (merged)
-
Add missing test cases for member_owners_excluding_project_bots
We add missing test cases for this method involving group share with groups
- 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
- Revert the latest commit (
Fix method returning invited members by mistake
) - 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.
-
I have evaluated the MR acceptance checklist for this MR.
Edited by Thong Kuah