Commit 2e1a420f authored by Heinrich Lee Yu's avatar Heinrich Lee Yu

Merge branch '301018-cablett-collapsed-epic-board-list' into 'master'

Retrieve epic list user preference collapsed value

See merge request gitlab-org/gitlab!54541
parents 206e057a 4211b495
......@@ -17,7 +17,7 @@ module Mutations
argument :collapsed, GraphQL::BOOLEAN_TYPE,
required: false,
description: 'Indicates if list is collapsed for this user.'
description: 'Indicates if the list is collapsed for this user.'
field :list,
Types::BoardListType,
......
......@@ -19,7 +19,7 @@ module Types
field :label, Types::LabelType, null: true,
description: 'Label of the list.'
field :collapsed, GraphQL::BOOLEAN_TYPE, null: true,
description: 'Indicates if list is collapsed for this user.'
description: 'Indicates if the list is collapsed for this user.'
field :issues_count, GraphQL::INT_TYPE, null: true,
description: 'Count of issues in the list.'
......
......@@ -13,6 +13,14 @@ module Boards
scope :ordered, -> { order(:list_type, :position) }
scope :destroyable, -> { where(list_type: list_types.slice(*destroyable_types).values) }
scope :movable, -> { where(list_type: list_types.slice(*movable_types).values) }
class << self
def preload_preferences_for_user(lists, user)
return unless user
lists.each { |list| list.preferences_for(user) }
end
end
end
class_methods do
......@@ -33,6 +41,18 @@ module Boards
self.class.movable_types.include?(list_type&.to_sym)
end
def collapsed?(user)
preferences = preferences_for(user)
preferences.collapsed?
end
def update_preferences_for(user, preferences = {})
return unless user
preferences_for(user).update(preferences)
end
def title
if label?
label.name
......
......@@ -18,14 +18,6 @@ class List < ApplicationRecord
alias_method :preferences, :list_user_preferences
class << self
def preload_preferences_for_user(lists, user)
return unless user
lists.each { |list| list.preferences_for(user) }
end
end
def preferences_for(user)
return preferences.build unless user
......@@ -39,18 +31,6 @@ class List < ApplicationRecord
end
end
def update_preferences_for(user, preferences = {})
return unless user
preferences_for(user).update(preferences)
end
def collapsed?(user)
preferences = preferences_for(user)
preferences.collapsed?
end
def as_json(options = {})
super(options).tap do |json|
json[:collapsed] = false
......
---
title: Expose epic board list collapsed value via GraphQL
merge_request: 54541
author:
type: added
# frozen_string_literal: true
class CreateEpicListUserPreferences < ActiveRecord::Migration[6.0]
DOWNTIME = false
def up
create_table :boards_epic_list_user_preferences do |t|
t.bigint :user_id, null: false
t.bigint :epic_list_id, index: true, null: false
t.timestamps_with_timezone null: false
t.boolean :collapsed, null: false, default: false
end
add_index :boards_epic_list_user_preferences, [:user_id, :epic_list_id], unique: true, name: 'index_epic_board_list_preferences_on_user_and_list'
end
def down
drop_table :boards_epic_list_user_preferences
end
end
# frozen_string_literal: true
class AddEpicBoardUserPreferenceUserFk < ActiveRecord::Migration[6.0]
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
disable_ddl_transaction!
def up
add_concurrent_foreign_key :boards_epic_list_user_preferences, :users, column: :user_id, on_delete: :cascade
end
def down
with_lock_retries do
remove_foreign_key_if_exists :boards_epic_list_user_preferences, :users
end
end
end
# frozen_string_literal: true
class AddEpicBoardUserPreferenceEpicListFk < ActiveRecord::Migration[6.0]
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
disable_ddl_transaction!
def up
add_concurrent_foreign_key :boards_epic_list_user_preferences, :boards_epic_lists, column: :epic_list_id, on_delete: :cascade
end
def down
with_lock_retries do
remove_foreign_key_if_exists :boards_epic_list_user_preferences, :boards_epic_lists
end
end
end
909aee5ed0ad447fec425f7252fc6dbec827a66ff720620bae1bf3a32536cb96
\ No newline at end of file
858cd59ea324e3653801055c7f3fae2152b04ac175945a59faa00d67ae7fa223
\ No newline at end of file
9e6f99ed0c3d4d76a8c290308805cabf84aa7e5fb6dc2b06d973d9d8726fc4d8
\ No newline at end of file
......@@ -9359,39 +9359,39 @@ CREATE TABLE application_settings (
elasticsearch_indexed_file_size_limit_kb integer DEFAULT 1024 NOT NULL,
enforce_namespace_storage_limit boolean DEFAULT false NOT NULL,
container_registry_delete_tags_service_timeout integer DEFAULT 250 NOT NULL,
kroki_url character varying,
kroki_enabled boolean,
elasticsearch_client_request_timeout integer DEFAULT 0 NOT NULL,
gitpod_enabled boolean DEFAULT false NOT NULL,
gitpod_url text DEFAULT 'https://gitpod.io/'::text,
elasticsearch_client_request_timeout integer DEFAULT 0 NOT NULL,
abuse_notification_email character varying,
require_admin_approval_after_user_signup boolean DEFAULT true NOT NULL,
help_page_documentation_base_url text,
automatic_purchased_storage_allocation boolean DEFAULT false NOT NULL,
container_registry_expiration_policies_worker_capacity integer DEFAULT 0 NOT NULL,
encrypted_ci_jwt_signing_key text,
encrypted_ci_jwt_signing_key_iv text,
container_registry_expiration_policies_worker_capacity integer DEFAULT 0 NOT NULL,
elasticsearch_analyzers_smartcn_enabled boolean DEFAULT false NOT NULL,
elasticsearch_analyzers_smartcn_search boolean DEFAULT false NOT NULL,
elasticsearch_analyzers_kuromoji_enabled boolean DEFAULT false NOT NULL,
elasticsearch_analyzers_kuromoji_search boolean DEFAULT false NOT NULL,
secret_detection_token_revocation_enabled boolean DEFAULT false NOT NULL,
secret_detection_token_revocation_url text,
encrypted_secret_detection_token_revocation_token text,
encrypted_secret_detection_token_revocation_token_iv text,
elasticsearch_analyzers_smartcn_enabled boolean DEFAULT false NOT NULL,
elasticsearch_analyzers_smartcn_search boolean DEFAULT false NOT NULL,
elasticsearch_analyzers_kuromoji_enabled boolean DEFAULT false NOT NULL,
elasticsearch_analyzers_kuromoji_search boolean DEFAULT false NOT NULL,
new_user_signups_cap integer,
domain_denylist_enabled boolean DEFAULT false,
domain_denylist text,
domain_allowlist text,
new_user_signups_cap integer,
encrypted_cloud_license_auth_token text,
encrypted_cloud_license_auth_token_iv text,
secret_detection_revocation_token_types_url text,
cloud_license_enabled boolean DEFAULT false NOT NULL,
kroki_url text,
kroki_enabled boolean DEFAULT false NOT NULL,
disable_feed_token boolean DEFAULT false NOT NULL,
personal_access_token_prefix text,
rate_limiting_response_text text,
invisible_captcha_enabled boolean DEFAULT false NOT NULL,
container_registry_cleanup_tags_service_max_list_size integer DEFAULT 200 NOT NULL,
invisible_captcha_enabled boolean DEFAULT false NOT NULL,
enforce_ssh_key_expiration boolean DEFAULT false NOT NULL,
git_two_factor_session_expiry integer DEFAULT 15 NOT NULL,
asset_proxy_allowlist text,
......@@ -9402,7 +9402,7 @@ CREATE TABLE application_settings (
in_product_marketing_emails_enabled boolean DEFAULT true NOT NULL,
CONSTRAINT app_settings_container_reg_cleanup_tags_max_list_size_positive CHECK ((container_registry_cleanup_tags_service_max_list_size >= 0)),
CONSTRAINT app_settings_registry_exp_policies_worker_capacity_positive CHECK ((container_registry_expiration_policies_worker_capacity >= 0)),
CONSTRAINT check_17d9558205 CHECK ((char_length((kroki_url)::text) <= 1024)),
CONSTRAINT check_17d9558205 CHECK ((char_length(kroki_url) <= 1024)),
CONSTRAINT check_2dba05b802 CHECK ((char_length(gitpod_url) <= 255)),
CONSTRAINT check_51700b31b5 CHECK ((char_length(default_branch_name) <= 255)),
CONSTRAINT check_57123c9593 CHECK ((char_length(help_page_documentation_base_url) <= 255)),
......@@ -9919,6 +9919,24 @@ CREATE SEQUENCE boards_epic_boards_id_seq
ALTER SEQUENCE boards_epic_boards_id_seq OWNED BY boards_epic_boards.id;
CREATE TABLE boards_epic_list_user_preferences (
id bigint NOT NULL,
user_id bigint NOT NULL,
epic_list_id bigint NOT NULL,
created_at timestamp with time zone NOT NULL,
updated_at timestamp with time zone NOT NULL,
collapsed boolean DEFAULT false NOT NULL
);
CREATE SEQUENCE boards_epic_list_user_preferences_id_seq
START WITH 1
INCREMENT BY 1
NO MINVALUE
NO MAXVALUE
CACHE 1;
ALTER SEQUENCE boards_epic_list_user_preferences_id_seq OWNED BY boards_epic_list_user_preferences.id;
CREATE TABLE boards_epic_lists (
id bigint NOT NULL,
created_at timestamp with time zone NOT NULL,
......@@ -18755,6 +18773,8 @@ ALTER TABLE ONLY boards_epic_board_positions ALTER COLUMN id SET DEFAULT nextval
ALTER TABLE ONLY boards_epic_boards ALTER COLUMN id SET DEFAULT nextval('boards_epic_boards_id_seq'::regclass);
ALTER TABLE ONLY boards_epic_list_user_preferences ALTER COLUMN id SET DEFAULT nextval('boards_epic_list_user_preferences_id_seq'::regclass);
ALTER TABLE ONLY boards_epic_lists ALTER COLUMN id SET DEFAULT nextval('boards_epic_lists_id_seq'::regclass);
ALTER TABLE ONLY boards_epic_user_preferences ALTER COLUMN id SET DEFAULT nextval('boards_epic_user_preferences_id_seq'::regclass);
......@@ -19834,6 +19854,9 @@ ALTER TABLE ONLY boards_epic_board_positions
ALTER TABLE ONLY boards_epic_boards
ADD CONSTRAINT boards_epic_boards_pkey PRIMARY KEY (id);
ALTER TABLE ONLY boards_epic_list_user_preferences
ADD CONSTRAINT boards_epic_list_user_preferences_pkey PRIMARY KEY (id);
ALTER TABLE ONLY boards_epic_lists
ADD CONSTRAINT boards_epic_lists_pkey PRIMARY KEY (id);
......@@ -21574,6 +21597,8 @@ CREATE INDEX index_boards_epic_board_positions_on_scoped_relative_position ON bo
CREATE INDEX index_boards_epic_boards_on_group_id ON boards_epic_boards USING btree (group_id);
CREATE INDEX index_boards_epic_list_user_preferences_on_epic_list_id ON boards_epic_list_user_preferences USING btree (epic_list_id);
CREATE INDEX index_boards_epic_lists_on_epic_board_id ON boards_epic_lists USING btree (epic_board_id);
CREATE UNIQUE INDEX index_boards_epic_lists_on_epic_board_id_and_label_id ON boards_epic_lists USING btree (epic_board_id, label_id) WHERE (list_type = 1);
......@@ -22098,6 +22123,8 @@ CREATE INDEX index_environments_on_project_id_state_environment_type ON environm
CREATE INDEX index_environments_on_state_and_auto_stop_at ON environments USING btree (state, auto_stop_at) WHERE ((auto_stop_at IS NOT NULL) AND ((state)::text = 'available'::text));
CREATE UNIQUE INDEX index_epic_board_list_preferences_on_user_and_list ON boards_epic_list_user_preferences USING btree (user_id, epic_list_id);
CREATE INDEX index_epic_issues_on_epic_id ON epic_issues USING btree (epic_id);
CREATE INDEX index_epic_issues_on_epic_id_and_issue_id ON epic_issues USING btree (epic_id, issue_id);
......@@ -24510,6 +24537,9 @@ ALTER TABLE ONLY milestones
ALTER TABLE ONLY vulnerabilities
ADD CONSTRAINT fk_959d40ad0a FOREIGN KEY (confirmed_by_id) REFERENCES users(id) ON DELETE SET NULL;
ALTER TABLE ONLY boards_epic_list_user_preferences
ADD CONSTRAINT fk_95eac55851 FOREIGN KEY (epic_list_id) REFERENCES boards_epic_lists(id) ON DELETE CASCADE;
ALTER TABLE ONLY application_settings
ADD CONSTRAINT fk_964370041d FOREIGN KEY (usage_stats_set_by_user_id) REFERENCES users(id) ON DELETE SET NULL;
......@@ -24810,6 +24840,9 @@ ALTER TABLE ONLY design_management_designs_versions
ALTER TABLE ONLY analytics_devops_adoption_segments
ADD CONSTRAINT fk_f5aa768998 FOREIGN KEY (namespace_id) REFERENCES namespaces(id) ON DELETE CASCADE;
ALTER TABLE ONLY boards_epic_list_user_preferences
ADD CONSTRAINT fk_f5f2fe5c1f FOREIGN KEY (user_id) REFERENCES users(id) ON DELETE CASCADE;
ALTER TABLE ONLY cluster_agents
ADD CONSTRAINT fk_f7d43dee13 FOREIGN KEY (created_by_user_id) REFERENCES users(id) ON DELETE SET NULL;
......@@ -669,7 +669,7 @@ Represents a list for an issue board.
| Field | Type | Description |
| ----- | ---- | ----------- |
| `assignee` | User | Assignee in the list. |
| `collapsed` | Boolean | Indicates if list is collapsed for this user. |
| `collapsed` | Boolean | Indicates if the list is collapsed for this user. |
| `id` | ID! | ID (global ID) of the list. |
| `issues` | IssueConnection | Board issues. |
| `issuesCount` | Int | Count of issues in the list. |
......@@ -1971,6 +1971,7 @@ Represents an epic board list.
| Field | Type | Description |
| ----- | ---- | ----------- |
| `collapsed` | Boolean | Indicates if this list is collapsed for this user. |
| `epics` | EpicConnection | List epics. |
| `id` | BoardsEpicListID! | Global ID of the board list. |
| `label` | Label | Label of the list. |
......
......@@ -22,6 +22,11 @@ module Resolvers
# point there is not reason to introduce a ListService
# https://gitlab.com/gitlab-org/gitlab/-/issues/294043
lists = epic_board.epic_lists
if load_preferences?(lookahead)
::Boards::EpicList.preload_preferences_for_user(lists, current_user)
end
lists = lists.where(id: id.model_id) if id # rubocop: disable CodeReuse/ActiveRecord
offset_pagination(apply_lookahead(lists))
......@@ -29,6 +34,11 @@ module Resolvers
private
def load_preferences?(lookahead)
lookahead&.selection(:edges)&.selection(:node)&.selects?(:collapsed) ||
lookahead&.selection(:nodes)&.selects?(:collapsed)
end
def authorize!
Ability.allowed?(context[:current_user], :read_epic_board_list, epic_board.group) || raise_resource_not_available_error!
end
......
......@@ -24,9 +24,16 @@ module Types
field :label, Types::LabelType, null: true,
description: 'Label of the list.'
field :collapsed, GraphQL::BOOLEAN_TYPE, null: true,
description: 'Indicates if this list is collapsed for this user.'
field :epics, Types::EpicType.connection_type, null: true,
resolver: Resolvers::Boards::BoardListEpicsResolver,
description: 'List epics.'
def collapsed
object.collapsed?(current_user)
end
end
# rubocop: enable Graphql/AuthorizeTypes
end
......
......@@ -6,9 +6,25 @@ module Boards
belongs_to :epic_board, optional: false, inverse_of: :epic_lists
belongs_to :label, inverse_of: :epic_lists
has_many :epic_list_user_preferences, inverse_of: :epic_list
validates :label_id, uniqueness: { scope: :epic_board_id }, if: :label?
enum list_type: { backlog: 0, label: 1, closed: 2 }
alias_method :preferences, :epic_list_user_preferences
def preferences_for(user)
return preferences.build unless user
BatchLoader.for(epic_list_id: id, user_id: user.id).batch(default_value: preferences.build(user: user)) do |items, loader|
list_ids = items.map { |i| i[:epic_list_id] }
user_ids = items.map { |i| i[:user_id] }
::Boards::EpicListUserPreference.where(epic_list_id: list_ids.uniq, user_id: user_ids.uniq).find_each do |preference|
loader.call({ epic_list_id: preference.epic_list_id, user_id: preference.user_id }, preference)
end
end
end
end
end
# frozen_string_literal: true
module Boards
class EpicListUserPreference < ApplicationRecord
belongs_to :user
belongs_to :epic_list, foreign_key: :epic_list_id, inverse_of: :epic_list_user_preferences
validates :user, presence: true
validates :epic_list, presence: true
validates :user_id, uniqueness: { scope: :epic_list_id, message: "should have only one epic list preference per user" }
end
end
......@@ -6,7 +6,7 @@ RSpec.describe GitlabSchema.types['EpicList'] do
specify { expect(described_class.graphql_name).to eq('EpicList') }
it 'has specific fields' do
expected_fields = %w[id title list_type position label epics]
expected_fields = %w[id title list_type position label epics collapsed]
expect(described_class).to include_graphql_fields(*expected_fields)
end
......
......@@ -4,12 +4,18 @@ require 'spec_helper'
RSpec.describe Boards::EpicList do
it_behaves_like 'boards listable model', :epic_list
it_behaves_like 'list_preferences_for user', :epic_list, :epic_list_id
describe 'associations' do
subject { build(:epic_list) }
it { is_expected.to belong_to(:epic_board).required.inverse_of(:epic_lists) }
it { is_expected.to belong_to(:label).inverse_of(:epic_lists) }
it { is_expected.to have_many(:epic_list_user_preferences).inverse_of(:epic_list) }
it { is_expected.to validate_uniqueness_of(:label_id).scoped_to(:epic_board_id) }
end
describe 'validations' do
it { is_expected.to validate_presence_of(:label) }
end
end
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe Boards::EpicListUserPreference do
let_it_be(:user) { create(:user) }
let_it_be(:epic_list) { create(:epic_list) }
before do
epic_list.update_preferences_for(user, { collapsed: true })
end
describe 'relationships' do
it { is_expected.to belong_to(:epic_list) }
it { is_expected.to belong_to(:user) }
it { is_expected.to validate_presence_of(:epic_list) }
it { is_expected.to validate_presence_of(:user) }
it do
is_expected.to validate_uniqueness_of(:user_id).scoped_to(:epic_list_id)
.with_message("should have only one epic list preference per user")
end
end
end
......@@ -54,5 +54,34 @@ RSpec.describe 'get list of epic boards' do
let(:first_param) { 2 }
end
end
it 'avoids N+1 queries' do
list1.update_preferences_for(current_user, collapsed: true)
control = ActiveRecord::QueryRecorder.new { post_graphql(pagination_query, current_user: current_user) }
list2.update_preferences_for(current_user, collapsed: true)
expect { post_graphql(pagination_query, current_user: current_user) }.not_to exceed_query_limit(control)
end
describe 'field values' do
let_it_be(:other_user) { create(:user) }
it 'returns the correct values for collapsed' do
list1.update_preferences_for(current_user, collapsed: true)
list1.update_preferences_for(other_user, collapsed: false)
post_graphql(pagination_query, current_user: current_user)
# ordered by list_type then position - backlog first and closed last.
assert_field_value('id', [global_id_of(list3), global_id_of(list1), global_id_of(list2)])
assert_field_value('collapsed', [false, true, false])
end
end
end
def assert_field_value(field, expected_value)
expect(graphql_dig_at(graphql_data, 'group', 'epicBoard', 'lists', 'nodes', field)).to eq(expected_value)
end
end
......@@ -5,6 +5,7 @@ require 'spec_helper'
RSpec.describe List do
it_behaves_like 'having unique enum values'
it_behaves_like 'boards listable model', :list
it_behaves_like 'list_preferences_for user', :list, :list_id
describe 'relationships' do
it { is_expected.to belong_to(:board) }
......@@ -29,71 +30,4 @@ RSpec.describe List do
expect(lists.where(board: board)).to match_array([backlog_list])
end
end
describe '#update_preferences_for' do
let(:user) { create(:user) }
let(:list) { create(:list) }
context 'when user is present' do
context 'when there are no preferences for user' do
it 'creates new user preferences' do
expect { list.update_preferences_for(user, collapsed: true) }.to change { ListUserPreference.count }.by(1)
expect(list.preferences_for(user).collapsed).to eq(true)
end
end
context 'when there are preferences for user' do
it 'updates user preferences' do
list.update_preferences_for(user, collapsed: false)
expect { list.update_preferences_for(user, collapsed: true) }.not_to change { ListUserPreference.count }
expect(list.preferences_for(user).collapsed).to eq(true)
end
end
context 'when user is nil' do
it 'does not create user preferences' do
expect { list.update_preferences_for(nil, collapsed: true) }.not_to change { ListUserPreference.count }
end
end
end
end
describe '#preferences_for' do
let(:user) { create(:user) }
let(:list) { create(:list) }
context 'when user is nil' do
it 'returns not persisted preferences' do
preferences = list.preferences_for(nil)
expect(preferences.persisted?).to eq(false)
expect(preferences.list_id).to eq(list.id)
expect(preferences.user_id).to be_nil
end
end
context 'when a user preference already exists' do
before do
list.update_preferences_for(user, collapsed: true)
end
it 'loads preference for user' do
preferences = list.preferences_for(user)
expect(preferences).to be_persisted
expect(preferences.collapsed).to eq(true)
end
end
context 'when preferences for user does not exist' do
it 'returns not persisted preferences' do
preferences = list.preferences_for(user)
expect(preferences.persisted?).to eq(false)
expect(preferences.user_id).to eq(user.id)
expect(preferences.list_id).to eq(list.id)
end
end
end
end
# frozen_string_literal: true
RSpec.shared_examples 'list_preferences_for user' do |list_factory, list_id_attribute|
subject { create(list_factory) } # rubocop:disable Rails/SaveBang
let_it_be(:user) { create(:user) }
describe '#preferences_for' do
context 'when user is nil' do
it 'returns not persisted preferences' do
preferences = subject.preferences_for(nil)
expect(preferences).not_to be_persisted
expect(preferences[list_id_attribute]).to eq(subject.id)
expect(preferences.user_id).to be_nil
end
end
context 'when a user preference already exists' do
before do
subject.update_preferences_for(user, collapsed: true)
end
it 'loads preference for user' do
preferences = subject.preferences_for(user)
expect(preferences).to be_persisted
expect(preferences.collapsed).to eq(true)
end
end
context 'when preferences for user does not exist' do
it 'returns not persisted preferences' do
preferences = subject.preferences_for(user)
expect(preferences).not_to be_persisted
expect(preferences.user_id).to eq(user.id)
expect(preferences.public_send(list_id_attribute)).to eq(subject.id)
end
end
end
describe '#update_preferences_for' do
context 'when user is present' do
context 'when there are no preferences for user' do
it 'creates new user preferences' do
expect { subject.update_preferences_for(user, collapsed: true) }.to change { subject.preferences.count }.by(1)
expect(subject.preferences_for(user).collapsed).to eq(true)
end
end
context 'when there are preferences for user' do
it 'updates user preferences' do
subject.update_preferences_for(user, collapsed: false)
expect { subject.update_preferences_for(user, collapsed: true) }.not_to change { subject.preferences.count }
expect(subject.preferences_for(user).collapsed).to eq(true)
end
end
context 'when user is nil' do
it 'does not create user preferences' do
expect { subject.update_preferences_for(nil, collapsed: true) }.not_to change { subject.preferences.count }
end
end
end
end
end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment