Skip to content

Add IssuableFinder support for OR filtering of authors [RUN ALL RSPEC] [RUN AS-IF-FOSS]

Heinrich Lee Yu requested to merge issuable-or-filters-author into master

What does this MR do?

Allows OR filtering with author_username. Also refactors author filtering into a separate class.

#296031 (closed)

Testing manually

  1. Enable the :or_issuable_queries feature flag.
  2. 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 with User.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

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
Edited by Heinrich Lee Yu

Merge request reports