Commit 54f61bbf authored by charlie ablett's avatar charlie ablett

Retrieve epic list user preference

- Add table and model
- Move some logic into shared concern
- Move some tests into shared examples
parent 2f64b88d
...@@ -33,6 +33,18 @@ module Boards ...@@ -33,6 +33,18 @@ module Boards
self.class.movable_types.include?(list_type&.to_sym) self.class.movable_types.include?(list_type&.to_sym)
end 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 def title
if label? if label?
label.name label.name
......
...@@ -39,18 +39,6 @@ class List < ApplicationRecord ...@@ -39,18 +39,6 @@ class List < ApplicationRecord
end end
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 = {}) def as_json(options = {})
super(options).tap do |json| super(options).tap do |json|
json[:collapsed] = false 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]
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
disable_ddl_transaction!
def up
create_table :boards_epic_list_user_preferences do |t|
t.references :user, index: true, null: false, foreign_key: { on_delete: :cascade }
t.references :epic_list, index: true, null: false, foreign_key: { to_table: :boards_epic_lists, on_delete: :cascade }
t.timestamps_with_timezone null: false
t.boolean :collapsed, null: false, default: false
end
add_concurrent_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
909aee5ed0ad447fec425f7252fc6dbec827a66ff720620bae1bf3a32536cb96
...@@ -9935,6 +9935,24 @@ CREATE SEQUENCE boards_epic_boards_id_seq ...@@ -9935,6 +9935,24 @@ CREATE SEQUENCE boards_epic_boards_id_seq
ALTER SEQUENCE boards_epic_boards_id_seq OWNED BY boards_epic_boards.id; 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 ( CREATE TABLE boards_epic_lists (
id bigint NOT NULL, id bigint NOT NULL,
created_at timestamp with time zone NOT NULL, created_at timestamp with time zone NOT NULL,
...@@ -18764,6 +18782,8 @@ ALTER TABLE ONLY boards_epic_board_positions ALTER COLUMN id SET DEFAULT nextval ...@@ -18764,6 +18782,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_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_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); ALTER TABLE ONLY boards_epic_user_preferences ALTER COLUMN id SET DEFAULT nextval('boards_epic_user_preferences_id_seq'::regclass);
...@@ -19846,6 +19866,9 @@ ALTER TABLE ONLY boards_epic_board_positions ...@@ -19846,6 +19866,9 @@ ALTER TABLE ONLY boards_epic_board_positions
ALTER TABLE ONLY boards_epic_boards ALTER TABLE ONLY boards_epic_boards
ADD CONSTRAINT boards_epic_boards_pkey PRIMARY KEY (id); 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 ALTER TABLE ONLY boards_epic_lists
ADD CONSTRAINT boards_epic_lists_pkey PRIMARY KEY (id); ADD CONSTRAINT boards_epic_lists_pkey PRIMARY KEY (id);
...@@ -21594,6 +21617,10 @@ CREATE INDEX index_boards_epic_board_positions_on_scoped_relative_position ON bo ...@@ -21594,6 +21617,10 @@ 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_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_list_user_preferences_on_user_id ON boards_epic_list_user_preferences USING btree (user_id);
CREATE INDEX index_boards_epic_lists_on_epic_board_id ON boards_epic_lists USING btree (epic_board_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); 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);
...@@ -22116,6 +22143,8 @@ CREATE INDEX index_environments_on_project_id_state_environment_type ON environm ...@@ -22116,6 +22143,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 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 ON epic_issues USING btree (epic_id);
CREATE UNIQUE INDEX index_epic_issues_on_issue_id ON epic_issues USING btree (issue_id); CREATE UNIQUE INDEX index_epic_issues_on_issue_id ON epic_issues USING btree (issue_id);
...@@ -25733,6 +25762,9 @@ ALTER TABLE ONLY packages_debian_project_distributions ...@@ -25733,6 +25762,9 @@ ALTER TABLE ONLY packages_debian_project_distributions
ALTER TABLE ONLY packages_rubygems_metadata ALTER TABLE ONLY packages_rubygems_metadata
ADD CONSTRAINT fk_rails_95a3f5ce78 FOREIGN KEY (package_id) REFERENCES packages_packages(id) ON DELETE CASCADE; ADD CONSTRAINT fk_rails_95a3f5ce78 FOREIGN KEY (package_id) REFERENCES packages_packages(id) ON DELETE CASCADE;
ALTER TABLE ONLY boards_epic_list_user_preferences
ADD CONSTRAINT fk_rails_95eac55851 FOREIGN KEY (epic_list_id) REFERENCES boards_epic_lists(id) ON DELETE CASCADE;
ALTER TABLE ONLY packages_pypi_metadata ALTER TABLE ONLY packages_pypi_metadata
ADD CONSTRAINT fk_rails_9698717cdd FOREIGN KEY (package_id) REFERENCES packages_packages(id) ON DELETE CASCADE; ADD CONSTRAINT fk_rails_9698717cdd FOREIGN KEY (package_id) REFERENCES packages_packages(id) ON DELETE CASCADE;
...@@ -26246,6 +26278,9 @@ ALTER TABLE ONLY resource_state_events ...@@ -26246,6 +26278,9 @@ ALTER TABLE ONLY resource_state_events
ALTER TABLE ONLY packages_debian_group_components ALTER TABLE ONLY packages_debian_group_components
ADD CONSTRAINT fk_rails_f5f1ef54c6 FOREIGN KEY (distribution_id) REFERENCES packages_debian_group_distributions(id) ON DELETE CASCADE; ADD CONSTRAINT fk_rails_f5f1ef54c6 FOREIGN KEY (distribution_id) REFERENCES packages_debian_group_distributions(id) ON DELETE CASCADE;
ALTER TABLE ONLY boards_epic_list_user_preferences
ADD CONSTRAINT fk_rails_f5f2fe5c1f FOREIGN KEY (user_id) REFERENCES users(id) ON DELETE CASCADE;
ALTER TABLE ONLY incident_management_oncall_shifts ALTER TABLE ONLY incident_management_oncall_shifts
ADD CONSTRAINT fk_rails_f6eef06841 FOREIGN KEY (participant_id) REFERENCES incident_management_oncall_participants(id) ON DELETE CASCADE; ADD CONSTRAINT fk_rails_f6eef06841 FOREIGN KEY (participant_id) REFERENCES incident_management_oncall_participants(id) ON DELETE CASCADE;
...@@ -1825,6 +1825,7 @@ Represents an epic board list. ...@@ -1825,6 +1825,7 @@ Represents an epic board list.
| Field | Type | Description | | Field | Type | Description |
| ----- | ---- | ----------- | | ----- | ---- | ----------- |
| `collapsed` | Boolean | Indicates if list is collapsed for this user. |
| `epics` | EpicConnection | List epics. | | `epics` | EpicConnection | List epics. |
| `id` | BoardsEpicListID! | Global ID of the board list. | | `id` | BoardsEpicListID! | Global ID of the board list. |
| `label` | Label | Label of the list. | | `label` | Label | Label of the list. |
......
...@@ -24,9 +24,16 @@ module Types ...@@ -24,9 +24,16 @@ module Types
field :label, Types::LabelType, null: true, field :label, Types::LabelType, null: true,
description: 'Label of the list.' description: 'Label of the list.'
field :collapsed, GraphQL::BOOLEAN_TYPE, null: true,
description: 'Indicates if list is collapsed for this user.'
field :epics, Types::EpicType.connection_type, null: true, field :epics, Types::EpicType.connection_type, null: true,
resolver: Resolvers::Boards::BoardListEpicsResolver, resolver: Resolvers::Boards::BoardListEpicsResolver,
description: 'List epics.' description: 'List epics.'
def collapsed
object.collapsed?(current_user)
end
end end
# rubocop: enable Graphql/AuthorizeTypes # rubocop: enable Graphql/AuthorizeTypes
end end
......
...@@ -6,9 +6,25 @@ module Boards ...@@ -6,9 +6,25 @@ module Boards
belongs_to :epic_board, optional: false, inverse_of: :epic_lists belongs_to :epic_board, optional: false, inverse_of: :epic_lists
belongs_to :label, 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? validates :label_id, uniqueness: { scope: :epic_board_id }, if: :label?
enum list_type: { backlog: 0, label: 1, closed: 2 } 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
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 ...@@ -6,7 +6,7 @@ RSpec.describe GitlabSchema.types['EpicList'] do
specify { expect(described_class.graphql_name).to eq('EpicList') } specify { expect(described_class.graphql_name).to eq('EpicList') }
it 'has specific fields' do 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) expect(described_class).to include_graphql_fields(*expected_fields)
end end
......
...@@ -4,12 +4,18 @@ require 'spec_helper' ...@@ -4,12 +4,18 @@ require 'spec_helper'
RSpec.describe Boards::EpicList do RSpec.describe Boards::EpicList do
it_behaves_like 'boards listable model', :epic_list it_behaves_like 'boards listable model', :epic_list
it_behaves_like 'list_preferences_for user', :epic_list, :epic_list_id, ::Boards::EpicListUserPreference
describe 'associations' do describe 'associations' do
subject { build(:epic_list) } subject { build(:epic_list) }
it { is_expected.to belong_to(:epic_board).required.inverse_of(:epic_lists) } 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 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) } it { is_expected.to validate_uniqueness_of(:label_id).scoped_to(:epic_board_id) }
end end
describe 'validations' do
it { is_expected.to validate_presence_of(:label) }
end
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 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
...@@ -53,4 +53,29 @@ RSpec.describe 'get list of epics for an epic board list' do ...@@ -53,4 +53,29 @@ RSpec.describe 'get list of epics for an epic board list' do
let(:first_param) { 2 } let(:first_param) { 2 }
end end
end end
describe 'field values' do
def query(params = {})
graphql_query_for(:group, { full_path: group.full_path },
<<~BOARDS
epicBoard(id: "#{board.to_global_id}") {
lists(id: "#{list.to_global_id}") {
nodes {
#{all_graphql_fields_for('epic_list'.classify)}
}
}
}
BOARDS
)
end
it 'returns the correct values for collapsed' do
board.epic_lists.first.update_preferences_for(current_user, collapsed: true)
post_graphql(query, current_user: current_user)
pp graphql_dig_at(graphql_data, 'group', 'epicBoard', 'lists', 'nodes', 0)
expect(graphql_dig_at(graphql_data, 'group', 'epicBoard', 'lists', 'nodes', 0, 'collapsed')).to eq list.collapsed?(current_user)
end
end
end end
...@@ -5,6 +5,7 @@ require 'spec_helper' ...@@ -5,6 +5,7 @@ require 'spec_helper'
RSpec.describe List do RSpec.describe List do
it_behaves_like 'having unique enum values' it_behaves_like 'having unique enum values'
it_behaves_like 'boards listable model', :list it_behaves_like 'boards listable model', :list
it_behaves_like 'list_preferences_for user', :list, :list_id, ListUserPreference
describe 'relationships' do describe 'relationships' do
it { is_expected.to belong_to(:board) } it { is_expected.to belong_to(:board) }
...@@ -29,71 +30,4 @@ RSpec.describe List do ...@@ -29,71 +30,4 @@ RSpec.describe List do
expect(lists.where(board: board)).to match_array([backlog_list]) expect(lists.where(board: board)).to match_array([backlog_list])
end end
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 end
# frozen_string_literal: true
RSpec.shared_examples 'list_preferences_for user' do |list_factory, list_id, preference_klass|
subject { create(list_factory) } # rubocop:disable Rails/SaveBang
let(: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.persisted?).to eq(false)
expect(preferences.send(list_id)).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.persisted?).to eq(false)
expect(preferences.user_id).to eq(user.id)
expect(preferences.send(list_id)).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 { preference_klass.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 { preference_klass.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 { preference_klass.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