Skip to content
Snippets Groups Projects

Backend support for advanced search multi-select project [RUN ALL RSPEC] [RUN AS-IF-FOSS]

Merged Terri Chu requested to merge 322793-backend-support-for-mutiple-projects into master

What does this MR do?

Related to Epic: &6110 and Issue: #332631

Backend support for allowing multi-select project search in Advanced Search only. Basic Search support will be handled in a separate MR. This feature is behind a disabled by default feature flag: advanced_search_multi_project_select for groups

Added a new param project_ids which will take the form of project_ids=1,2,3 or project_ids=1 and is only set if a group is selected and the Feature Flag is enabled for the selected group.

If any of the projects selected have Advanced Search enabled, the query will go to Elasticsearch. Once Basic Search supports multi-select projects, additional work will be done to combine results when projects results come from Postgres and Elasticsearch.

How to test

Note: You need to have Elasticsearch enabled in GDK, have Advanced Search enabled for search + indexing, and have created the index using rake commands or the Admin UI (rake gitlab:elastic:index)

  1. Enable the feature for a specific group in the rails console: Feature.enable(:advanced_search_multi_project_select, group)
  2. Navigate to the search page: http://localhost:3000/search
  3. Select the group you enabled the feature for in the drop down
  4. Take note of 2 project ids for that group
  5. Manually add &project_ids=AA,BB to the URL where AA and BB are the project_ids from the previous step
  6. perform some searches and make sure you are getting data back from both projects

Note: I also did some testing by turning on Namespace limiting and trying to send the project_ids parameter to groups and projects that only support Basic Search to make sure it didn't break anything.

Database

There is a count query and select query for each search scope and will detail both here. Project ids used: 278964 and 7764

Select

Explain plan: https://explain.depesz.com/s/PbPt

SELECT
    users. *
FROM
    users
WHERE (((users.name ILIKE %test%
            OR users.username ILIKE %test%)
            OR users.email = test)
            OR users.id = (
                SELECT
                    emails.user_id
                FROM
                    emails
                WHERE
                    emails.email = test
                LIMIT 1))
        AND users.id IN (
            SELECT
                users.id
            FROM
                users
                INNER JOIN project_authorizations ON project_authorizations.user_id = users.id
            WHERE
                project_authorizations.project_id IN (278964, 7764))
        ORDER BY
            CASE WHEN users.name = test THEN
                0
            WHEN users.username = test THEN
                1
            WHEN users.email = test THEN
                2
            ELSE
                3
            END,
            users.name ASC
        LIMIT 20 OFFSET 0

Count

Explain plan: https://explain.depesz.com/s/HWrU

SELECT
    COUNT(*)
FROM (
    SELECT
        1 AS one
    FROM
        users
    WHERE (((users.name ILIKE %test%
                OR users.username ILIKE %test%)
                OR users.email = test)
                OR users.id = (
                    SELECT
                        emails.user_id
                    FROM
                        emails
                    WHERE
                        emails.email = test
                    LIMIT 1))
            AND users.id IN (
                SELECT
                    users.id
                FROM
                    users
                    INNER JOIN project_authorizations ON project_authorizations.user_id = users.id
                WHERE
                    project_authorizations.project_id IN (278964, 7764))
            LIMIT 100) subquery_for_count

Screenshots (strongly suggested)

Tested with two project ids which map to gnome-1 and Fdroidclient projects (which you can see in the screenshots)

code

image

issues

image

merge requests

image

milestones

image

comments

image

commits

image

wiki

image

users

image

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

Security

Does this MR contain changes to processing or storing of credentials or tokens, authorization and authentication methods or other items described in the security review guidelines? If not, then delete this Security section.

  • 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 Terri Chu

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Ghost User
  • Terri Chu marked this merge request as draft

    marked this merge request as draft

  • Terri Chu added 1 commit

    added 1 commit

    • 1c0dbc12 - Backend support for multi-select project search

    Compare with previous version

  • Terri Chu added 1 commit

    added 1 commit

    • e04123c2 - Backend support for multi-select project search

    Compare with previous version

  • Terri Chu added 1 commit

    added 1 commit

    • c865e79a - Backend support for multi-select project search

    Compare with previous version

  • Terri Chu resolved all threads

    resolved all threads

  • Terri Chu added 1 commit

    added 1 commit

    • e8c1accf - Backend support for multi-select project search

    Compare with previous version

  • Terri Chu marked the checklist item I have included a changelog entry, or it's not needed. (Does this MR need a changelog?) as completed

    marked the checklist item I have included a changelog entry, or it's not needed. (Does this MR need a changelog?) as completed

  • Terri Chu marked the checklist item I have added/updated documentation, or it's not needed. (Is documentation required?) as completed

    marked the checklist item I have added/updated documentation, or it's not needed. (Is documentation required?) as completed

  • Terri Chu marked the checklist item I have properly separated EE content from FOSS, or this MR is FOSS only. (Where should EE code go?) as completed

    marked the checklist item I have properly separated EE content from FOSS, or this MR is FOSS only. (Where should EE code go?) as completed

  • Terri Chu added 1 commit

    added 1 commit

    • 1967e0f3 - Backend support for multi-select project search

    Compare with previous version

  • Terri Chu marked the checklist item This MR does not harm performance, or I have asked a reviewer to help assess the performance impact. (Merge request performance guidelines) as completed

    marked the checklist item This MR does not harm performance, or I have asked a reviewer to help assess the performance impact. (Merge request performance guidelines) as completed

  • marked the checklist item I have added information for database reviewers in the MR description, or it's not needed. (Does this MR have database related changes?) as completed

  • Terri Chu marked the checklist item I have followed the style guides. as completed

    marked the checklist item I have followed the style guides. as completed

  • Terri Chu marked the checklist item I have added/updated tests following the Testing Guide, or it's not needed. (Consider all test levels. See the Test Planning Process.) as completed

    marked the checklist item I have added/updated tests following the Testing Guide, or it's not needed. (Consider all test levels. See the Test Planning Process.) as completed

  • Terri Chu marked the checklist item I have tested this MR in all supported browsers, or it's not needed. as completed

    marked the checklist item I have tested this MR in all supported browsers, or it's not needed. as completed

  • Terri Chu marked the checklist item I have informed the Infrastructure department of a default or new setting change per definition of done, or it's not needed. as completed

    marked the checklist item I have informed the Infrastructure department of a default or new setting change per definition of done, or it's not needed. as completed

  • Terri Chu changed the description

    changed the description

  • Terri Chu added 1088 commits

    added 1088 commits

    Compare with previous version

  • Terri Chu
  • Allure report

    allure-report-publisher generated test report for 1ef24268!

    review-qa-smoke: :pencil: test report

  • Terri Chu changed the description

    changed the description

  • Terri Chu changed the description

    changed the description

  • Terri Chu changed the description

    changed the description

  • Terri Chu changed the description

    changed the description

  • Terri Chu added 1 commit

    added 1 commit

    • 2668a578 - Backend support for multi-select project search

    Compare with previous version

  • Terri Chu changed the description

    changed the description

  • added database label

  • A deleted user added databasereview pending label
  • Terri Chu added 1 commit

    added 1 commit

    • 9e065b3a - Backend support for multi-select project search

    Compare with previous version

  • Terri Chu
  • Terri Chu marked this merge request as ready

    marked this merge request as ready

    • Author Maintainer
      Resolved by Dmitry Gruzd

      @dgruzd would you mind the initial review here? I wanted to get your take on it first and hope that is ok since this is a complex feature :smile:

      I feel like I should be adding more tests, but am struggling with where. Do you have any suggestions?

  • Terri Chu added workflowin review label and removed workflowin dev label

    added workflowin review label and removed workflowin dev label

  • Terri Chu requested review from @dgruzd

    requested review from @dgruzd

  • Terri Chu marked the checklist item I have self-reviewed this MR per code review guidelines. as completed

    marked the checklist item I have self-reviewed this MR per code review guidelines. as completed

  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Please register or sign in to reply
    Loading