Add IssuableFinder support for OR filtering of authors [RUN ALL RSPEC] [RUN AS-IF-FOSS]
What does this MR do?
Allows OR filtering with author_username
. Also refactors author filtering into a separate class.
Testing manually
- Enable the
:or_issuable_queries
feature flag. - Visit
http://localhost:3000/h5bp/html5-boilerplate/-/issues?or[author_username][]=user1&or[author_username][]=user2
Query changes
SELECT "issues".*
FROM "issues"
WHERE "issues"."project_id" = 2880930
AND ("issues"."state_id" IN (1))
AND "issues"."author_id" IN (
SELECT "users"."id" FROM "users" WHERE (LOWER("users"."username") IN (LOWER('engwan')))
)
AND "issues"."issue_type" IN (0, 1)
ORDER BY "issues"."created_at" DESC, "issues"."id" DESC
LIMIT 20 OFFSET 0
https://explain.depesz.com/s/0mb4
Time: 1.534 ms
- planning: 1.272 ms
- execution: 0.262 ms
- I/O read: N/A
- I/O write: N/A
Shared buffers:
- hits: 17 (~136.00 KiB) from the buffer pool
- reads: 0 from the OS file cache, including disk I/O
- dirtied: 0
- writes: 0
- The old code just did
issues.author_id = xxx
because it ran another query withUser.find_by_username
to get the ID. - The new code uses
LOWER
when finding by username because we're reusing a scope. I see that this is also indexed so I think this is fine. - With the
:or_issuable_queries
feature flag enabled, multiple usernames can be passed in to the subquery. Example query and query plan with 3 usernames: https://explain.depesz.com/s/LbPP
Does this MR meet the acceptance criteria?
Conformity
-
Changelog entry -
Documentation (if required) -
Code review guidelines -
Merge request performance guidelines -
Style guides -
Database guides -
Separation of EE specific content
Availability and Testing
-
Review and add/update tests for this feature/bug. Consider all test levels. See the Test Planning Process. -
Tested in all supported browsers -
Informed Infrastructure department of a default or new setting change, if applicable per definition of done
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
Edited by Heinrich Lee Yu