Skip to content

Epic boards epic list

Jan Provaznik requested to merge epic_boards_epic_list into master

What does this MR do?

DB migration

$ bundle exec rake db:migrate
== 20210104163218 AddEpicBoardPositionIndex: migrating ========================
-- transaction_open?()
   -> 0.0000s
-- index_exists?(:boards_epic_board_positions, [:epic_board_id, :epic_id, :relative_position], {:name=>"index_boards_epic_board_positions_on_scoped_relative_position", :algorithm=>:concurrently})
   -> 0.0016s
-- add_index(:boards_epic_board_positions, [:epic_board_id, :epic_id, :relative_position], {:name=>"index_boards_epic_board_positions_on_scoped_relative_position", :algorithm=>:concurrently})
   -> 0.0195s
== 20210104163218 AddEpicBoardPositionIndex: migrated (0.0213s) ===============

$ bundle exec rake db:rollback
== 20210104163218 AddEpicBoardPositionIndex: reverting ========================
-- transaction_open?()
   -> 0.0000s
-- indexes(:boards_epic_board_positions)
   -> 0.0029s
-- remove_index(:boards_epic_board_positions, {:algorithm=>:concurrently, :name=>"index_boards_epic_board_positions_on_scoped_relative_position"})
   -> 0.0051s
== 20210104163218 AddEpicBoardPositionIndex: reverted (0.0083s) ===============

Query plan

SQL query to get epics for an epic board lists ordered by the board's position (with null position last). Query plan is from a local deployment because we don't have epic board records yet: https://explain.depesz.com/s/xBx8
SELECT "epics".*
FROM "epics"
LEFT OUTER JOIN "boards_epic_board_positions" ON "boards_epic_board_positions"."epic_id" = "epics"."id"
WHERE "epics"."group_id" IN
    (WITH RECURSIVE "base_and_descendants" AS (
                                                 (SELECT "namespaces".*
                                                  FROM "namespaces"
                                                  WHERE "namespaces"."type" = 'Group'
                                                    AND "namespaces"."id" = 28)
                                               UNION
                                                 (SELECT "namespaces".*
                                                  FROM "namespaces",
                                                       "base_and_descendants"
                                                  WHERE "namespaces"."type" = 'Group'
                                                    AND "namespaces"."parent_id" = "base_and_descendants"."id")) SELECT "namespaces"."id"
     FROM "base_and_descendants" AS "namespaces")
  AND "epics"."state_id" = 1
  AND (EXISTS
         (SELECT "label_links".*
          FROM "label_links"
          WHERE (label_links.target_type = 'Epic'
                 AND label_links.target_id = epics.id)
            AND "label_links"."label_id" = 63
          LIMIT 1))
  AND ("boards_epic_board_positions"."epic_board_id" = 1
       OR "boards_epic_board_positions"."epic_board_id" IS NULL)
ORDER BY boards_epic_board_positions.relative_position ASC NULLS LAST,
                                                           epics.id DESC
LIMIT 100;

Architecture solution question

It was discussed on the related issue why we don't reuse the existing board/list issue's API/model. This is a valid question and I have to admit it's very tempting to reuse it because adding extra tables/models/services adds additional overhead. On the other side I think having both separated is the clean approach (details in the linked discussion). I'm open to refactor this if others feedback is we should reuse existing models/tables/services.

Related to #233436 (closed)

Screenshots (strongly suggested)

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 Jan Provaznik

Merge request reports

Loading