diff options
| author | GitLab Bot <gitlab-bot@gitlab.com> | 2019-12-13 15:08:02 +0000 |
|---|---|---|
| committer | GitLab Bot <gitlab-bot@gitlab.com> | 2019-12-13 15:08:02 +0000 |
| commit | 4eeb6b0d16021ab4a730eec4610eff2606421147 (patch) | |
| tree | 488db828fe58f1e80dc5415970e4c929db7e4c4b | |
| parent | 8cc5f2790908ba9bb8eecba2b78a3c5a88c77b90 (diff) | |
| download | gitlab-ce-4eeb6b0d16021ab4a730eec4610eff2606421147.tar.gz | |
Add latest changes from gitlab-org/gitlab@master
31 files changed, 957 insertions, 664 deletions
diff --git a/app/controllers/concerns/membership_actions.rb b/app/controllers/concerns/membership_actions.rb index 0b2756c0c6a..993f091b0e6 100644 --- a/app/controllers/concerns/membership_actions.rb +++ b/app/controllers/concerns/membership_actions.rb @@ -144,4 +144,15 @@ module MembershipActions end end end + + def requested_relations + case params[:with_inherited_permissions].presence + when 'exclude' + [:direct] + when 'only' + [:inherited] + else + [:inherited, :direct] + end + end end diff --git a/app/controllers/groups/group_members_controller.rb b/app/controllers/groups/group_members_controller.rb index 1b1416a72d7..dcdf9aced1a 100644 --- a/app/controllers/groups/group_members_controller.rb +++ b/app/controllers/groups/group_members_controller.rb @@ -24,8 +24,7 @@ class Groups::GroupMembersController < Groups::ApplicationController @sort = params[:sort].presence || sort_value_name @project = @group.projects.find(params[:project_id]) if params[:project_id] - - @members = GroupMembersFinder.new(@group).execute + @members = find_members if can_manage_members @invited_members = @members.invite @@ -52,6 +51,12 @@ class Groups::GroupMembersController < Groups::ApplicationController # MembershipActions concern alias_method :membershipable, :group + + private + + def find_members + GroupMembersFinder.new(@group).execute(include_relations: requested_relations) + end end Groups::GroupMembersController.prepend_if_ee('EE::Groups::GroupMembersController') diff --git a/app/controllers/projects/project_members_controller.rb b/app/controllers/projects/project_members_controller.rb index b01d48ca3d3..7bd084458d1 100644 --- a/app/controllers/projects/project_members_controller.rb +++ b/app/controllers/projects/project_members_controller.rb @@ -17,7 +17,7 @@ class Projects::ProjectMembersController < Projects::ApplicationController @skip_groups << @project.namespace_id unless @project.personal? @skip_groups += @project.group.ancestors.pluck(:id) if @project.group - @project_members = MembersFinder.new(@project, current_user).execute + @project_members = MembersFinder.new(@project, current_user).execute(include_relations: requested_relations) if params[:search].present? @project_members = @project_members.joins(:user).merge(User.search(params[:search])) diff --git a/app/finders/group_members_finder.rb b/app/finders/group_members_finder.rb index 165d9adae31..d8739c350e4 100644 --- a/app/finders/group_members_finder.rb +++ b/app/finders/group_members_finder.rb @@ -6,15 +6,15 @@ class GroupMembersFinder < UnionFinder end # rubocop: disable CodeReuse/ActiveRecord - def execute(include_descendants: false) + def execute(include_relations: [:inherited, :direct]) group_members = @group.members relations = [] - return group_members unless @group.parent || include_descendants + return group_members if include_relations == [:direct] - relations << group_members + relations << group_members if include_relations.include?(:direct) - if @group.parent + if include_relations.include?(:inherited) && @group.parent parents_members = GroupMember.non_request .where(source_id: @group.ancestors.select(:id)) .where.not(user_id: @group.users.select(:id)) @@ -22,7 +22,7 @@ class GroupMembersFinder < UnionFinder relations << parents_members end - if include_descendants + if include_relations.include?(:descendants) descendant_members = GroupMember.non_request .where(source_id: @group.descendants.select(:id)) .where.not(user_id: @group.users.select(:id)) diff --git a/app/finders/members_finder.rb b/app/finders/members_finder.rb index e8c7f9622a9..a919ff5bf8a 100644 --- a/app/finders/members_finder.rb +++ b/app/finders/members_finder.rb @@ -9,14 +9,18 @@ class MembersFinder @group = project.group end - def execute(include_descendants: false, include_invited_groups_members: false) + def execute(include_relations: [:inherited, :direct]) project_members = project.project_members project_members = project_members.non_invite unless can?(current_user, :admin_project, project) - union_members = group_union_members(include_descendants, include_invited_groups_members) + return project_members if include_relations == [:direct] + + union_members = group_union_members(include_relations) + + union_members << project_members if include_relations.include?(:direct) if union_members.any? - distinct_union_of_members(union_members << project_members) + distinct_union_of_members(union_members) else project_members end @@ -28,15 +32,17 @@ class MembersFinder private - def group_union_members(include_descendants, include_invited_groups_members) + def group_union_members(include_relations) [].tap do |members| - members << direct_group_members(include_descendants) if group - members << project_invited_groups_members if include_invited_groups_members + members << direct_group_members(include_relations.include?(:descendants)) if group + members << project_invited_groups_members if include_relations.include?(:invited_groups_members) end end def direct_group_members(include_descendants) - GroupMembersFinder.new(group).execute(include_descendants: include_descendants).non_invite # rubocop: disable CodeReuse/Finder + requested_relations = [:inherited, :direct] + requested_relations << :descendants if include_descendants + GroupMembersFinder.new(group).execute(include_relations: requested_relations).non_invite # rubocop: disable CodeReuse/Finder end def project_invited_groups_members diff --git a/app/views/shared/members/_sort_dropdown.html.haml b/app/views/shared/members/_sort_dropdown.html.haml index 5f3d49adff7..50a55565c3c 100644 --- a/app/views/shared/members/_sort_dropdown.html.haml +++ b/app/views/shared/members/_sort_dropdown.html.haml @@ -8,3 +8,13 @@ %li = link_to filter_group_project_member_path(sort: value), class: ("is-active" if @sort == value) do = title + %li.divider + %li{ data: { 'qa-selector': 'filter-members-with-inherited-permissions' } } + = link_to filter_group_project_member_path(with_inherited_permissions: nil), class: ("is-active" unless params[:with_inherited_permissions].present?) do + = _("Show all members") + %li{ data: { 'qa-selector': 'filter-members-with-inherited-permissions' } } + = link_to filter_group_project_member_path(with_inherited_permissions: 'exclude'), class: ("is-active" if params[:with_inherited_permissions] == 'exclude') do + = _("Show only direct members") + %li{ data: { 'qa-selector': 'filter-members-with-inherited-permissions' } } + = link_to filter_group_project_member_path(with_inherited_permissions: 'only'), class: ("is-active" if params[:with_inherited_permissions] == 'only') do + = _("Show only inherited members") diff --git a/changelogs/unreleased/xanf-improve-members-filtering.yml b/changelogs/unreleased/xanf-improve-members-filtering.yml new file mode 100644 index 00000000000..b6510ddbd59 --- /dev/null +++ b/changelogs/unreleased/xanf-improve-members-filtering.yml @@ -0,0 +1,5 @@ +--- +title: Added filtering of inherited members for subgroups +merge_request: 18842 +author: +type: added diff --git a/doc/ci/img/collapsible_log.png b/doc/ci/img/collapsible_log.png Binary files differdeleted file mode 100644 index 088928ddede..00000000000 --- a/doc/ci/img/collapsible_log.png +++ /dev/null diff --git a/doc/ci/img/collapsible_log_v12_6.png b/doc/ci/img/collapsible_log_v12_6.png Binary files differnew file mode 100644 index 00000000000..294421c645d --- /dev/null +++ b/doc/ci/img/collapsible_log_v12_6.png diff --git a/doc/ci/pipelines.md b/doc/ci/pipelines.md index 4538d1e7849..d1e50039417 100644 --- a/doc/ci/pipelines.md +++ b/doc/ci/pipelines.md @@ -157,7 +157,7 @@ In the following example: - Two sections are collapsed and can be expanded. - Three sections are expanded and can be collapsed. - + ## Configuring pipelines diff --git a/doc/development/background_migrations.md b/doc/development/background_migrations.md index 0a08360b727..5e16f83b63c 100644 --- a/doc/development/background_migrations.md +++ b/doc/development/background_migrations.md @@ -16,10 +16,15 @@ migrations automatically reschedule themselves for a later point in time. > the migrations. In the vast majority of cases you will want to use a regular Rails migration -instead. Background migrations should _only_ be used when migrating _data_ in +instead. Background migrations should be used when migrating _data_ in tables that have so many rows this process would take hours when performed in a regular Rails migration. +Background migrations _may_ also be used when executing numerous single-row queries +for every item on a large dataset. Typically, for single-record patterns, runtime is +largely dependent on the size of the dataset, hence it should be split accordingly +and put into background migrations. + Background migrations _may not_ be used to perform schema migrations, they should only be used for data migrations. diff --git a/doc/development/database_review.md b/doc/development/database_review.md index 38785897361..b1c3ed47976 100644 --- a/doc/development/database_review.md +++ b/doc/development/database_review.md @@ -103,10 +103,13 @@ and details for a database reviewer: need to fit comfortably within `15s` - preferably much less than that - on GitLab.com. - For column removals, make sure the column has been [ignored in a previous release](what_requires_downtime.md#dropping-columns) - Check [background migrations](background_migrations.md): - - Establish a time estimate for execution on GitLab.com. - - They should only be used when migrating data in larger tables. - - If a single `update` is below than `1s` the query can be placed + - Establish a time estimate for execution on GitLab.com. For historical purposes, + it's highly recommended to include this estimation on the merge request description. + - If a single `update` is below than `1s` the query can be placed directly in a regular migration (inside `db/migrate`). + - Background migrations are normally used, but not limited to: + - Migrating data in larger tables. + - Making numerous SQL queries per record in a dataset. - Review queries (for example, make sure batch sizes are fine) - Because execution time can be longer than for a regular migration, it's suggested to treat background migrations as post migrations: diff --git a/doc/user/group/subgroups/img/group_members_filter_v12_6.png b/doc/user/group/subgroups/img/group_members_filter_v12_6.png Binary files differnew file mode 100644 index 00000000000..0207515ded0 --- /dev/null +++ b/doc/user/group/subgroups/img/group_members_filter_v12_6.png diff --git a/doc/user/group/subgroups/index.md b/doc/user/group/subgroups/index.md index 52b7035389a..997cb1ba6c5 100644 --- a/doc/user/group/subgroups/index.md +++ b/doc/user/group/subgroups/index.md @@ -4,7 +4,7 @@ type: reference, howto, concepts # Subgroups ->[Introduced](https://gitlab.com/gitlab-org/gitlab-foss/issues/2772) in GitLab 9.0. +> [Introduced](https://gitlab.com/gitlab-org/gitlab-foss/issues/2772) in GitLab 9.0. Subgroups, also known as nested groups or hierarchical groups, allow you to have up to 20 levels of groups. @@ -142,6 +142,16 @@ From the image above, we can deduce the following things: - Administrator is the Owner and member of **all** subgroups and for that reason, as with User3, there is no indication of an ancestor group. +[From](https://gitlab.com/gitlab-org/gitlab/issues/21727) GitLab 12.6, you can filter +this list using dropdown on the right side: + + + +- **Show only direct members** displays only Administrator and User3, since these are + the only users that belong to group `four`, which is the one we're inspecting. +- **Show only inherited members** displays User0, User1 and User2, no matter which group + above the hierarchy is the source of inherited permissions. + ### Overriding the ancestor group membership NOTE: **Note:** @@ -186,7 +196,7 @@ Here's a list of what you can't do with subgroups: [ce-2772]: https://gitlab.com/gitlab-org/gitlab-foss/issues/2772 [permissions]: ../../permissions.md#group-members-permissions -[reserved]: ../../reserved_names.md +[reserved]: ../../reserved_names.md [issue]: https://gitlab.com/gitlab-org/gitlab-foss/issues/30472#note_27747600 <!-- ## Troubleshooting diff --git a/doc/user/project/members/img/project_members.png b/doc/user/project/members/img/project_members.png Binary files differnew file mode 100644 index 00000000000..5d44b5d957e --- /dev/null +++ b/doc/user/project/members/img/project_members.png diff --git a/doc/user/project/members/img/project_members_filter_v12_6.png b/doc/user/project/members/img/project_members_filter_v12_6.png Binary files differnew file mode 100644 index 00000000000..0207515ded0 --- /dev/null +++ b/doc/user/project/members/img/project_members_filter_v12_6.png diff --git a/doc/user/project/members/index.md b/doc/user/project/members/index.md index 2f8394eb104..c069882e38f 100644 --- a/doc/user/project/members/index.md +++ b/doc/user/project/members/index.md @@ -10,6 +10,31 @@ or import a new user to your project. To view, edit, add, and remove project's members, go to your project's **Settings > Members**. +## Inherited membership + +When your project belongs to the group, group members inherit the membership and permission +level for the project from the group. + + + +From the image above, we can deduce the following things: + +- There are 3 members that have access to the project. +- User0 is a Reporter and has inherited their permissions from group `demo` + which contains current project. +- For User1 there is no indication of a group, therefore they belong directly + to the project we're inspecting. +- Administrator is the Owner and member of **all** groups and for that reason, + there is an indication of an ancestor group and inherited Owner permissions. + +[From](https://gitlab.com/gitlab-org/gitlab/issues/21727), you can filter this list +using dropdown on the right side: + + + +- **Show only direct members** displays only User1. +- **Show only inherited members** displays User0 and Administrator. + ## Add a user Right next to **People**, start typing the name or username of the user you diff --git a/lib/api/helpers/members_helpers.rb b/lib/api/helpers/members_helpers.rb index 5bebf7ef570..9e624903a62 100644 --- a/lib/api/helpers/members_helpers.rb +++ b/lib/api/helpers/members_helpers.rb @@ -29,7 +29,7 @@ module API end def find_all_members_for_project(project) - MembersFinder.new(project, current_user).execute(include_invited_groups_members: true) + MembersFinder.new(project, current_user).execute(include_relations: [:inherited, :direct, :invited_groups_members]) end def find_all_members_for_group(group) diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 28210dad9bb..80a0966d069 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -16189,6 +16189,9 @@ msgstr "" msgid "Show all activity" msgstr "" +msgid "Show all members" +msgstr "" + msgid "Show archived projects" msgstr "" @@ -16216,6 +16219,12 @@ msgstr "" msgid "Show latest version" msgstr "" +msgid "Show only direct members" +msgstr "" + +msgid "Show only inherited members" +msgstr "" + msgid "Show parent pages" msgstr "" @@ -36,6 +36,7 @@ module QA autoload :GPG, 'qa/runtime/gpg' autoload :MailHog, 'qa/runtime/mail_hog' autoload :IPAddress, 'qa/runtime/ip_address' + autoload :Search, 'qa/runtime/search' module API autoload :Client, 'qa/runtime/api/client' diff --git a/qa/qa/runtime/search.rb b/qa/qa/runtime/search.rb new file mode 100644 index 00000000000..faa110c96e7 --- /dev/null +++ b/qa/qa/runtime/search.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +module QA + module Runtime + module Search + extend self + extend Support::Api + + ElasticSearchServerError = Class.new(RuntimeError) + + def elasticsearch_responding? + QA::Runtime::Logger.debug("Attempting to search via Elasticsearch...") + + QA::Support::Retrier.retry_on_exception do + # We don't care about the results of the search, we just need + # any search that uses Elasticsearch, not the native search + # The Elasticsearch-only scopes are blobs, wiki_blobs, and commits. + request = Runtime::API::Request.new(api_client, "/search?scope=blobs&search=foo") + response = get(request.url) + + unless response.code == singleton_class::HTTP_STATUS_OK + raise ElasticSearchServerError, "Search attempt failed. Request returned (#{response.code}): `#{response}`." + end + + true + end + end + + private + + def api_client + @api_client ||= Runtime::API::Client.new(:gitlab) + end + end + end +end diff --git a/spec/controllers/groups/group_members_controller_spec.rb b/spec/controllers/groups/group_members_controller_spec.rb index 22f970133e3..a144d9e0786 100644 --- a/spec/controllers/groups/group_members_controller_spec.rb +++ b/spec/controllers/groups/group_members_controller_spec.rb @@ -5,7 +5,7 @@ require 'spec_helper' describe Groups::GroupMembersController do include ExternalAuthorizationServiceHelpers - let(:user) { create(:user) } + let(:user) { create(:user) } let(:group) { create(:group, :public) } let(:membership) { create(:group_member, group: group) } @@ -49,6 +49,35 @@ describe Groups::GroupMembersController do expect(assigns(:invited_members).count).to eq(1) end end + + context 'when user has owner access to subgroup' do + let(:nested_group) { create(:group, parent: group) } + let(:nested_group_user) { create(:user) } + + before do + group.add_owner(user) + nested_group.add_owner(nested_group_user) + sign_in(user) + end + + it 'lists inherited group members by default' do + get :index, params: { group_id: nested_group } + + expect(assigns(:members).map(&:user_id)).to contain_exactly(user.id, nested_group_user.id) + end + + it 'lists direct group members only' do + get :index, params: { group_id: nested_group, with_inherited_permissions: 'exclude' } + + expect(assigns(:members).map(&:user_id)).to contain_exactly(nested_group_user.id) + end + + it 'lists inherited group members only' do + get :index, params: { group_id: nested_group, with_inherited_permissions: 'only' } + + expect(assigns(:members).map(&:user_id)).to contain_exactly(user.id) + end + end end describe 'POST create' do diff --git a/spec/controllers/projects/project_members_controller_spec.rb b/spec/controllers/projects/project_members_controller_spec.rb index 072df1f5060..0df19aa6e44 100644 --- a/spec/controllers/projects/project_members_controller_spec.rb +++ b/spec/controllers/projects/project_members_controller_spec.rb @@ -4,6 +4,7 @@ require('spec_helper') describe Projects::ProjectMembersController do let(:user) { create(:user) } + let(:group) { create(:group, :public) } let(:project) { create(:project, :public) } describe 'GET index' do @@ -12,6 +13,35 @@ describe Projects::ProjectMembersController do expect(response).to have_gitlab_http_status(200) end + + context 'when project belongs to group' do + let(:user_in_group) { create(:user) } + let(:project_in_group) { create(:project, :public, group: group) } + + before do + group.add_owner(user_in_group) + project_in_group.add_maintainer(user) + sign_in(user) + end + + it 'lists inherited project members by default' do + get :index, params: { namespace_id: project_in_group.namespace, project_id: project_in_group } + + expect(assigns(:project_members).map(&:user_id)).to contain_exactly(user.id, user_in_group.id) + end + + it 'lists direct project members only' do + get :index, params: { namespace_id: project_in_group.namespace, project_id: project_in_group, with_inherited_permissions: 'exclude' } + + expect(assigns(:project_members).map(&:user_id)).to contain_exactly(user.id) + end + + it 'lists inherited project members only' do + get :index, params: { namespace_id: project_in_group.namespace, project_id: project_in_group, with_inherited_permissions: 'only' } + + expect(assigns(:project_members).map(&:user_id)).to contain_exactly(user_in_group.id) + end + end end describe 'POST create' do diff --git a/spec/features/groups/members/filter_members_spec.rb b/spec/features/groups/members/filter_members_spec.rb index fc62c92db4e..cba86f7e651 100644 --- a/spec/features/groups/members/filter_members_spec.rb +++ b/spec/features/groups/members/filter_members_spec.rb @@ -3,42 +3,71 @@ require 'spec_helper' describe 'Groups > Members > Filter members' do - let(:user) { create(:user) } - let(:user_with_2fa) { create(:user, :two_factor_via_otp) } - let(:group) { create(:group) } + let(:user) { create(:user) } + let(:nested_group_user) { create(:user) } + let(:user_with_2fa) { create(:user, :two_factor_via_otp) } + let(:group) { create(:group) } + let(:nested_group) { create(:group, parent: group) } before do group.add_owner(user) group.add_maintainer(user_with_2fa) + nested_group.add_maintainer(nested_group_user) sign_in(user) end it 'shows all members' do - visit_members_list + visit_members_list(group) - expect(first_member).to include(user.name) - expect(second_member).to include(user_with_2fa.name) + expect(member(0)).to include(user.name) + expect(member(1)).to include(user_with_2fa.name) expect(page).to have_css('.member-filter-2fa-dropdown .dropdown-toggle-text', text: 'Everyone') end it 'shows only 2FA members' do - visit_members_list(two_factor: 'enabled') + visit_members_list(group, two_factor: 'enabled') - expect(first_member).to include(user_with_2fa.name) + expect(member(0)).to include(user_with_2fa.name) expect(members_list.size).to eq(1) expect(page).to have_css('.member-filter-2fa-dropdown .dropdown-toggle-text', text: 'Enabled') end it 'shows only non 2FA members' do - visit_members_list(two_factor: 'disabled') + visit_members_list(group, two_factor: 'disabled') - expect(first_member).to include(user.name) + expect(member(0)).to include(user.name) expect(members_list.size).to eq(1) expect(page).to have_css('.member-filter-2fa-dropdown .dropdown-toggle-text', text: 'Disabled') end - def visit_members_list(options = {}) + it 'shows inherited members by default' do + visit_members_list(nested_group) + + expect(member(0)).to include(user.name) + expect(member(1)).to include(user_with_2fa.name) + expect(member(2)).to include(nested_group_user.name) + expect(members_list.size).to eq(3) + + expect(page).to have_css('[data-qa-selector="filter-members-with-inherited-permissions"] a.is-active', text: 'Show all members') + end + + it 'shows only group members' do + visit_members_list(nested_group, with_inherited_permissions: 'exclude') + expect(member(0)).to include(nested_group_user.name) + expect(members_list.size).to eq(1) + expect(page).to have_css('[data-qa-selector="filter-members-with-inherited-permissions"] a.is-active', text: 'Show only direct members') + end + + it 'shows only inherited members' do + visit_members_list(nested_group, with_inherited_permissions: 'only') + expect(member(0)).to include(user.name) + expect(member(1)).to include(user_with_2fa.name) + expect(members_list.size).to eq(2) + expect(page).to have_css('[data-qa-selector="filter-members-with-inherited-permissions"] a.is-active', text: 'Show only inherited members') + end + + def visit_members_list(group, options = {}) visit group_group_members_path(group.to_param, options) end @@ -46,11 +75,7 @@ describe 'Groups > Members > Filter members' do page.all('ul.content-list > li') end - def first_member - members_list.first.text - end - - def second_member - members_list.last.text + def member(number) + members_list[number].text end end diff --git a/spec/features/projects/members/group_members_spec.rb b/spec/features/projects/members/group_members_spec.rb index 4ecc3db78b3..d37f912a2bc 100644 --- a/spec/features/projects/members/group_members_spec.rb +++ b/spec/features/projects/members/group_members_spec.rb @@ -31,6 +31,41 @@ describe 'Projects members' do end end + context 'with a group' do + it 'shows group and project members by default' do + visit project_project_members_path(project) + + page.within first('.content-list') do + expect(page).to have_content(developer.name) + + expect(page).to have_content(user.name) + expect(page).to have_content(group.name) + end + end + + it 'shows project members only if requested' do + visit project_project_members_path(project, with_inherited_permissions: 'exclude') + + page.within first('.content-list') do + expect(page).to have_content(developer.name) + + expect(page).not_to have_content(user.name) + expect(page).not_to have_content(group.name) + end + end + + it 'shows group members only if requested' do + visit project_project_members_path(project, with_inherited_permissions: 'only') + + page.within first('.content-list') do + expect(page).not_to have_content(developer.name) + + expect(page).to have_content(user.name) + expect(page).to have_content(group.name) + end + end + end + context 'with a group and a project invitee' do before do group_invitee diff --git a/spec/finders/group_members_finder_spec.rb b/spec/finders/group_members_finder_spec.rb index 08f3b4024b3..f161a1df9c3 100644 --- a/spec/finders/group_members_finder_spec.rb +++ b/spec/finders/group_members_finder_spec.rb @@ -3,12 +3,13 @@ require 'spec_helper' describe GroupMembersFinder, '#execute' do - let(:group) { create(:group) } + let(:group) { create(:group) } let(:nested_group) { create(:group, parent: group) } - let(:user1) { create(:user) } - let(:user2) { create(:user) } - let(:user3) { create(:user) } - let(:user4) { create(:user) } + let(:deeper_nested_group) { create(:group, parent: nested_group) } + let(:user1) { create(:user) } + let(:user2) { create(:user) } + let(:user3) { create(:user) } + let(:user4) { create(:user) } it 'returns members for top-level group' do member1 = group.add_maintainer(user1) @@ -20,7 +21,7 @@ describe GroupMembersFinder, '#execute' do expect(result.to_a).to match_array([member3, member2, member1]) end - it 'returns members for nested group' do + it 'returns members & inherited members for nested group by default' do group.add_developer(user2) nested_group.request_access(user4) member1 = group.add_maintainer(user1) @@ -32,6 +33,29 @@ describe GroupMembersFinder, '#execute' do expect(result.to_a).to match_array([member1, member3, member4]) end + it 'does not return inherited members for nested group if requested' do + group.add_maintainer(user1) + group.add_developer(user2) + member2 = nested_group.add_maintainer(user2) + member3 = nested_group.add_maintainer(user3) + + result = described_class.new(nested_group).execute(include_relations: [:direct]) + + expect(result.to_a).to match_array([member2, member3]) + end + + it 'returns only inherited members for nested group if requested' do + group.add_developer(user2) + nested_group.request_access(user4) + member1 = group.add_maintainer(user1) + nested_group.add_maintainer(user2) + nested_group.add_maintainer(user3) + + result = described_class.new(nested_group).execute(include_relations: [:inherited]) + + expect(result.to_a).to match_array([member1]) + end + it 'returns members for descendant groups if requested' do member1 = group.add_maintainer(user2) member2 = group.add_maintainer(user1) @@ -39,7 +63,7 @@ describe GroupMembersFinder, '#execute' do member3 = nested_group.add_maintainer(user3) member4 = nested_group.add_maintainer(user4) - result = described_class.new(group).execute(include_descendants: true) + result = described_class.new(group).execute(include_relations: [:direct, :descendants]) expect(result.to_a).to match_array([member1, member2, member3, member4]) end diff --git a/spec/finders/members_finder_spec.rb b/spec/finders/members_finder_spec.rb index f9b8fee6f2d..2bc37606a25 100644 --- a/spec/finders/members_finder_spec.rb +++ b/spec/finders/members_finder_spec.rb @@ -22,22 +22,64 @@ describe MembersFinder, '#execute' do expect(result).to contain_exactly(member1, member2, member3) end + it 'includes only non-invite members if user do not have amdin permissions on project' do + create(:project_member, :invited, project: project, invite_email: create(:user).email) + member1 = project.add_maintainer(user1) + member2 = project.add_developer(user2) + + result = described_class.new(project, user2).execute(include_relations: [:direct]) + + expect(result).to contain_exactly(member1, member2) + end + + it 'includes invited members if user have admin permissions on project' do + member_invite = create(:project_member, :invited, project: project, invite_email: create(:user).email) + member1 = project.add_maintainer(user1) + member2 = project.add_maintainer(user2) + + result = described_class.new(project, user2).execute(include_relations: [:direct]) + + expect(result).to contain_exactly(member1, member2, member_invite) + end + it 'includes nested group members if asked', :nested_groups do nested_group.request_access(user1) member1 = group.add_maintainer(user2) member2 = nested_group.add_maintainer(user3) member3 = project.add_maintainer(user4) - result = described_class.new(project, user2).execute(include_descendants: true) + result = described_class.new(project, user2).execute(include_relations: [:direct, :descendants]) expect(result).to contain_exactly(member1, member2, member3) end + it 'returns only members of project if asked' do + nested_group.request_access(user1) + group.add_maintainer(user2) + nested_group.add_maintainer(user3) + member4 = project.add_maintainer(user4) + + result = described_class.new(project, user2).execute(include_relations: [:direct]) + + expect(result).to contain_exactly(member4) + end + + it 'returns only inherited members of project if asked' do + nested_group.request_access(user1) + member2 = group.add_maintainer(user2) + member3 = nested_group.add_maintainer(user3) + project.add_maintainer(user4) + + result = described_class.new(project, user2).execute(include_relations: [:inherited]) + + expect(result).to contain_exactly(member2, member3) + end + it 'returns the members.access_level when the user is invited', :nested_groups do member_invite = create(:project_member, :invited, project: project, invite_email: create(:user).email) member1 = group.add_maintainer(user2) - result = described_class.new(project, user2).execute(include_descendants: true) + result = described_class.new(project, user2).execute(include_relations: [:direct, :descendants]) expect(result).to contain_exactly(member1, member_invite) expect(result.last.access_level).to eq(member_invite.access_level) @@ -48,14 +90,14 @@ describe MembersFinder, '#execute' do group.add_developer(user1) nested_group.add_reporter(user1) - result = described_class.new(project, user1).execute(include_descendants: true) + result = described_class.new(project, user1).execute(include_relations: [:direct, :descendants]) expect(result).to contain_exactly(member1) expect(result.first.access_level).to eq(Gitlab::Access::DEVELOPER) end context 'when include_invited_groups_members == true' do - subject { described_class.new(project, user2).execute(include_invited_groups_members: true) } + subject { described_class.new(project, user2).execute(include_relations: [:inherited, :direct, :invited_groups_members]) } set(:linked_group) { create(:group, :public) } set(:nested_linked_group) { create(:group, parent: linked_group) } diff --git a/spec/frontend/boards/boards_store_spec.js b/spec/frontend/boards/boards_store_spec.js index f8e5873c87b..3588197ebdc 100644 --- a/spec/frontend/boards/boards_store_spec.js +++ b/spec/frontend/boards/boards_store_spec.js @@ -1,7 +1,23 @@ -import { TEST_HOST } from 'helpers/test_constants'; import AxiosMockAdapter from 'axios-mock-adapter'; +import { TEST_HOST } from 'helpers/test_constants'; import axios from '~/lib/utils/axios_utils'; import boardsStore from '~/boards/stores/boards_store'; +import eventHub from '~/boards/eventhub'; +import { listObj, listObjDuplicate } from './mock_data'; + +import ListIssue from '~/boards/models/issue'; +import '~/boards/models/list'; + +jest.mock('js-cookie'); + +const createTestIssue = () => ({ + title: 'Testing', + id: 1, + iid: 1, + confidential: false, + labels: [], + assignees: [], +}); describe('boardsStore', () => { const dummyResponse = "without type checking this doesn't matter"; @@ -23,6 +39,19 @@ describe('boardsStore', () => { }); }); + afterEach(() => { + axiosMock.restore(); + jest.clearAllMocks(); + }); + + const setupDefaultResponses = () => { + axiosMock + .onGet(`${endpoints.listsEndpoint}/${listObj.id}/issues?id=${listObj.id}&page=1`) + .reply(200, { issues: [createTestIssue()] }); + axiosMock.onPost(endpoints.listsEndpoint).reply(200, listObj); + axiosMock.onPut(); + }; + describe('all', () => { it('makes a request to fetch lists', () => { axiosMock.onGet(endpoints.listsEndpoint).replyOnce(200, dummyResponse); @@ -546,4 +575,464 @@ describe('boardsStore', () => { return expect(boardsStore.deleteBoard({ id })).rejects.toThrow(); }); }); + + describe('when created', () => { + beforeEach(() => { + setupDefaultResponses(); + + jest.spyOn(boardsStore, 'moveIssue').mockReturnValue(Promise.resolve()); + jest.spyOn(boardsStore, 'moveMultipleIssues').mockReturnValue(Promise.resolve()); + + boardsStore.create(); + }); + + it('starts with a blank state', () => { + expect(boardsStore.state.lists.length).toBe(0); + }); + + describe('addList', () => { + it('sorts by position', () => { + boardsStore.addList({ position: 2 }); + boardsStore.addList({ position: 1 }); + + expect(boardsStore.state.lists.map(({ position }) => position)).toEqual([1, 2]); + }); + }); + + describe('toggleFilter', () => { + const dummyFilter = 'x=42'; + let updateTokensSpy; + + beforeEach(() => { + updateTokensSpy = jest.fn(); + eventHub.$once('updateTokens', updateTokensSpy); + + // prevent using window.history + jest.spyOn(boardsStore, 'updateFiltersUrl').mockReturnValue(); + }); + + it('adds the filter if it is not present', () => { + boardsStore.filter.path = 'something'; + + boardsStore.toggleFilter(dummyFilter); + + expect(boardsStore.filter.path).toEqual(`something&${dummyFilter}`); + expect(updateTokensSpy).toHaveBeenCalled(); + expect(boardsStore.updateFiltersUrl).toHaveBeenCalled(); + }); + + it('removes the filter if it is present', () => { + boardsStore.filter.path = `something&${dummyFilter}`; + + boardsStore.toggleFilter(dummyFilter); + + expect(boardsStore.filter.path).toEqual('something'); + expect(updateTokensSpy).toHaveBeenCalled(); + expect(boardsStore.updateFiltersUrl).toHaveBeenCalled(); + }); + }); + + describe('lists', () => { + it('creates new list without persisting to DB', () => { + expect(boardsStore.state.lists.length).toBe(0); + + boardsStore.addList(listObj); + + expect(boardsStore.state.lists.length).toBe(1); + }); + + it('finds list by ID', () => { + boardsStore.addList(listObj); + const list = boardsStore.findList('id', listObj.id); + + expect(list.id).toBe(listObj.id); + }); + + it('finds list by type', () => { + boardsStore.addList(listObj); + const list = boardsStore.findList('type', 'label'); + + expect(list).toBeDefined(); + }); + + it('finds list by label ID', () => { + boardsStore.addList(listObj); + const list = boardsStore.findListByLabelId(listObj.label.id); + + expect(list.id).toBe(listObj.id); + }); + + it('gets issue when new list added', () => { + boardsStore.addList(listObj); + const list = boardsStore.findList('id', listObj.id); + + expect(boardsStore.state.lists.length).toBe(1); + + return axios.waitForAll().then(() => { + expect(list.issues.length).toBe(1); + expect(list.issues[0].id).toBe(1); + }); + }); + + it('persists new list', () => { + boardsStore.new({ + title: 'Test', + list_type: 'label', + label: { + id: 1, + title: 'Testing', + color: 'red', + description: 'testing;', + }, + }); + + expect(boardsStore.state.lists.length).toBe(1); + + return axios.waitForAll().then(() => { + const list = boardsStore.findList('id', listObj.id); + + expect(list).toEqual( + expect.objectContaining({ + id: listObj.id, + position: 0, + }), + ); + }); + }); + + it('check for blank state adding', () => { + expect(boardsStore.shouldAddBlankState()).toBe(true); + }); + + it('check for blank state not adding', () => { + boardsStore.addList(listObj); + + expect(boardsStore.shouldAddBlankState()).toBe(false); + }); + + it('check for blank state adding when closed list exist', () => { + boardsStore.addList({ + list_type: 'closed', + }); + + expect(boardsStore.shouldAddBlankState()).toBe(true); + }); + + it('adds the blank state', () => { + boardsStore.addBlankState(); + + const list = boardsStore.findList('type', 'blank', 'blank'); + + expect(list).toBeDefined(); + }); + + it('removes list from state', () => { + boardsStore.addList(listObj); + + expect(boardsStore.state.lists.length).toBe(1); + + boardsStore.removeList(listObj.id, 'label'); + + expect(boardsStore.state.lists.length).toBe(0); + }); + + it('moves the position of lists', () => { + const listOne = boardsStore.addList(listObj); + boardsStore.addList(listObjDuplicate); + + expect(boardsStore.state.lists.length).toBe(2); + + boardsStore.moveList(listOne, [listObjDuplicate.id, listObj.id]); + + expect(listOne.position).toBe(1); + }); + + it('moves an issue from one list to another', () => { + const listOne = boardsStore.addList(listObj); + const listTwo = boardsStore.addList(listObjDuplicate); + + expect(boardsStore.state.lists.length).toBe(2); + + return axios.waitForAll().then(() => { + expect(listOne.issues.length).toBe(1); + expect(listTwo.issues.length).toBe(1); + + boardsStore.moveIssueToList(listOne, listTwo, listOne.findIssue(1)); + + expect(listOne.issues.length).toBe(0); + expect(listTwo.issues.length).toBe(1); + }); + }); + + it('moves an issue from backlog to a list', () => { + const backlog = boardsStore.addList({ + ...listObj, + list_type: 'backlog', + }); + const listTwo = boardsStore.addList(listObjDuplicate); + + expect(boardsStore.state.lists.length).toBe(2); + + return axios.waitForAll().then(() => { + expect(backlog.issues.length).toBe(1); + expect(listTwo.issues.length).toBe(1); + + boardsStore.moveIssueToList(backlog, listTwo, backlog.findIssue(1)); + + expect(backlog.issues.length).toBe(0); + expect(listTwo.issues.length).toBe(1); + }); + }); + + it('moves issue to top of another list', () => { + const listOne = boardsStore.addList(listObj); + const listTwo = boardsStore.addList(listObjDuplicate); + + expect(boardsStore.state.lists.length).toBe(2); + + return axios.waitForAll().then(() => { + listOne.issues[0].id = 2; + + expect(listOne.issues.length).toBe(1); + expect(listTwo.issues.length).toBe(1); + + boardsStore.moveIssueToList(listOne, listTwo, listOne.findIssue(2), 0); + + expect(listOne.issues.length).toBe(0); + expect(listTwo.issues.length).toBe(2); + expect(listTwo.issues[0].id).toBe(2); + expect(boardsStore.moveIssue).toHaveBeenCalledWith(2, listOne.id, listTwo.id, null, 1); + }); + }); + + it('moves issue to bottom of another list', () => { + const listOne = boardsStore.addList(listObj); + const listTwo = boardsStore.addList(listObjDuplicate); + + expect(boardsStore.state.lists.length).toBe(2); + + return axios.waitForAll().then(() => { + listOne.issues[0].id = 2; + + expect(listOne.issues.length).toBe(1); + expect(listTwo.issues.length).toBe(1); + + boardsStore.moveIssueToList(listOne, listTwo, listOne.findIssue(2), 1); + + expect(listOne.issues.length).toBe(0); + expect(listTwo.issues.length).toBe(2); + expect(listTwo.issues[1].id).toBe(2); + expect(boardsStore.moveIssue).toHaveBeenCalledWith(2, listOne.id, listTwo.id, 1, null); + }); + }); + + it('moves issue in list', () => { + const issue = new ListIssue({ + title: 'Testing', + id: 2, + iid: 2, + confidential: false, + labels: [], + assignees: [], + }); + const list = boardsStore.addList(listObj); + + return axios.waitForAll().then(() => { + list.addIssue(issue); + + expect(list.issues.length).toBe(2); + + boardsStore.moveIssueInList(list, issue, 0, 1, [1, 2]); + + expect(list.issues[0].id).toBe(2); + expect(boardsStore.moveIssue).toHaveBeenCalledWith(2, null, null, 1, null); + }); + }); + }); + + describe('setListDetail', () => { + it('sets the list detail', () => { + boardsStore.detail.list = 'not a list'; + + const dummyValue = 'new list'; + boardsStore.setListDetail(dummyValue); + + expect(boardsStore.detail.list).toEqual(dummyValue); + }); + }); + + describe('clearDetailIssue', () => { + it('resets issue details', () => { + boardsStore.detail.issue = 'something'; + + boardsStore.clearDetailIssue(); + + expect(boardsStore.detail.issue).toEqual({}); + }); + }); + + describe('setIssueDetail', () => { + it('sets issue details', () => { + boardsStore.detail.issue = 'some details'; + + const dummyValue = 'new details'; + boardsStore.setIssueDetail(dummyValue); + + expect(boardsStore.detail.issue).toEqual(dummyValue); + }); + }); + + describe('startMoving', () => { + it('stores list and issue', () => { + const dummyIssue = 'some issue'; + const dummyList = 'some list'; + + boardsStore.startMoving(dummyList, dummyIssue); + + expect(boardsStore.moving.issue).toEqual(dummyIssue); + expect(boardsStore.moving.list).toEqual(dummyList); + }); + }); + + describe('setTimeTrackingLimitToHours', () => { + it('sets the timeTracking.LimitToHours option', () => { + boardsStore.timeTracking.limitToHours = false; + + boardsStore.setTimeTrackingLimitToHours('true'); + + expect(boardsStore.timeTracking.limitToHours).toEqual(true); + }); + }); + + describe('setCurrentBoard', () => { + const dummyBoard = 'hoverboard'; + + it('sets the current board', () => { + const { state } = boardsStore; + state.currentBoard = null; + + boardsStore.setCurrentBoard(dummyBoard); + + expect(state.currentBoard).toEqual(dummyBoard); + }); + }); + + describe('toggleMultiSelect', () => { + let basicIssueObj; + + beforeAll(() => { + basicIssueObj = { id: 987654 }; + }); + + afterEach(() => { + boardsStore.clearMultiSelect(); + }); + + it('adds issue when not present', () => { + boardsStore.toggleMultiSelect(basicIssueObj); + + const selectedIds = boardsStore.multiSelect.list.map(({ id }) => id); + + expect(selectedIds.includes(basicIssueObj.id)).toEqual(true); + }); + + it('removes issue when issue is present', () => { + boardsStore.toggleMultiSelect(basicIssueObj); + let selectedIds = boardsStore.multiSelect.list.map(({ id }) => id); + + expect(selectedIds.includes(basicIssueObj.id)).toEqual(true); + + boardsStore.toggleMultiSelect(basicIssueObj); + selectedIds = boardsStore.multiSelect.list.map(({ id }) => id); + + expect(selectedIds.includes(basicIssueObj.id)).toEqual(false); + }); + }); + + describe('clearMultiSelect', () => { + it('clears all the multi selected issues', () => { + const issue1 = { id: 12345 }; + const issue2 = { id: 12346 }; + + boardsStore.toggleMultiSelect(issue1); + boardsStore.toggleMultiSelect(issue2); + + expect(boardsStore.multiSelect.list.length).toEqual(2); + + boardsStore.clearMultiSelect(); + + expect(boardsStore.multiSelect.list.length).toEqual(0); + }); + }); + + describe('moveMultipleIssuesToList', () => { + it('move issues on the new index', () => { + const listOne = boardsStore.addList(listObj); + const listTwo = boardsStore.addList(listObjDuplicate); + + expect(boardsStore.state.lists.length).toBe(2); + + return axios.waitForAll().then(() => { + expect(listOne.issues.length).toBe(1); + expect(listTwo.issues.length).toBe(1); + + boardsStore.moveMultipleIssuesToList({ + listFrom: listOne, + listTo: listTwo, + issues: listOne.issues, + newIndex: 0, + }); + + expect(listTwo.issues.length).toBe(1); + }); + }); + }); + + describe('moveMultipleIssuesInList', () => { + it('moves multiple issues in list', () => { + const issueObj = { + title: 'Issue #1', + id: 12345, + iid: 2, + confidential: false, + labels: [], + assignees: [], + }; + const issue1 = new ListIssue(issueObj); + const issue2 = new ListIssue({ + ...issueObj, + title: 'Issue #2', + id: 12346, + }); + + const list = boardsStore.addList(listObj); + + return axios.waitForAll().then(() => { + list.addIssue(issue1); + list.addIssue(issue2); + + expect(list.issues.length).toBe(3); + expect(list.issues[0].id).not.toBe(issue2.id); + + boardsStore.moveMultipleIssuesInList({ + list, + issues: [issue1, issue2], + oldIndicies: [0], + newIndex: 1, + idArray: [1, 12345, 12346], + }); + + expect(list.issues[0].id).toBe(issue1.id); + + expect(boardsStore.moveMultipleIssues).toHaveBeenCalledWith({ + ids: [issue1.id, issue2.id], + fromListId: null, + toListId: null, + moveBeforeId: 1, + moveAfterId: null, + }); + }); + }); + }); + }); }); diff --git a/spec/frontend/boards/mock_data.js b/spec/frontend/boards/mock_data.js new file mode 100644 index 00000000000..05f73e0d8dc --- /dev/null +++ b/spec/frontend/boards/mock_data.js @@ -0,0 +1,91 @@ +export const boardObj = { + id: 1, + name: 'test', + milestone_id: null, +}; + +export const listObj = { + id: 300, + position: 0, + title: 'Test', + list_type: 'label', + weight: 3, + label: { + id: 5000, + title: 'Test', + color: 'red', + description: 'testing;', + textColor: 'white', + }, +}; + +export const listObjDuplicate = { + id: listObj.id, + position: 1, + title: 'Test', + list_type: 'label', + weight: 3, + label: { + id: listObj.label.id, + title: 'Test', + color: 'red', + description: 'testing;', + }, +}; + +export const mockAssigneesList = [ + { + id: 2, + name: 'Terrell Graham', + username: 'monserrate.gleichner', + state: 'active', + avatar_url: 'https://www.gravatar.com/avatar/598fd02741ac58b88854a99d16704309?s=80&d=identicon', + web_url: 'http://127.0.0.1:3001/monserrate.gleichner', + path: '/monserrate.gleichner', + }, + { + id: 12, + name: 'Susy Johnson', + username: 'tana_harvey', + state: 'active', + avatar_url: 'https://www.gravatar.com/avatar/e021a7b0f3e4ae53b5068d487e68c031?s=80&d=identicon', + web_url: 'http://127.0.0.1:3001/tana_harvey', + path: '/tana_harvey', + }, + { + id: 20, + name: 'Conchita Eichmann', + username: 'juliana_gulgowski', + state: 'active', + avatar_url: 'https://www.gravatar.com/avatar/c43c506cb6fd7b37017d3b54b94aa937?s=80&d=identicon', + web_url: 'http://127.0.0.1:3001/juliana_gulgowski', + path: '/juliana_gulgowski', + }, + { + id: 6, + name: 'Bryce Turcotte', + username: 'melynda', + state: 'active', + avatar_url: 'https://www.gravatar.com/avatar/cc2518f2c6f19f8fac49e1a5ee092a9b?s=80&d=identicon', + web_url: 'http://127.0.0.1:3001/melynda', + path: '/melynda', + }, + { + id: 1, + name: 'Administrator', + username: 'root', + state: 'active', + avatar_url: 'https://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon', + web_url: 'http://127.0.0.1:3001/root', + path: '/root', + }, +]; + +export const mockMilestone = { + id: 1, + state: 'active', + title: 'Milestone title', + description: 'Harum corporis aut consequatur quae dolorem error sequi quia.', + start_date: '2018-01-01', + due_date: '2019-12-31', +}; diff --git a/spec/javascripts/boards/boards_store_spec.js b/spec/javascripts/boards/boards_store_spec.js deleted file mode 100644 index dfc74faf63c..00000000000 --- a/spec/javascripts/boards/boards_store_spec.js +++ /dev/null @@ -1,509 +0,0 @@ -/* global ListIssue */ - -import MockAdapter from 'axios-mock-adapter'; -import Cookies from 'js-cookie'; -import axios from '~/lib/utils/axios_utils'; - -import '~/boards/models/label'; -import '~/boards/models/assignee'; -import '~/boards/models/issue'; -import '~/boards/models/list'; -import boardsStore from '~/boards/stores/boards_store'; -import eventHub from '~/boards/eventhub'; -import { listObj, listObjDuplicate, boardsMockInterceptor } from './mock_data'; -import waitForPromises from '../../frontend/helpers/wait_for_promises'; - -describe('Store', () => { - let mock; - - beforeEach(() => { - mock = new MockAdapter(axios); - mock.onAny().reply(boardsMockInterceptor); - boardsStore.create(); - - spyOn(boardsStore, 'moveIssue').and.callFake( - () => - new Promise(resolve => { - resolve(); - }), - ); - - spyOn(boardsStore, 'moveMultipleIssues').and.callFake( - () => - new Promise(resolve => { - resolve(); - }), - ); - - Cookies.set('issue_board_welcome_hidden', 'false', { - expires: 365 * 10, - path: '', - }); - }); - - afterEach(() => { - mock.restore(); - }); - - it('starts with a blank state', () => { - expect(boardsStore.state.lists.length).toBe(0); - }); - - describe('addList', () => { - it('sorts by position', () => { - boardsStore.addList({ position: 2 }); - boardsStore.addList({ position: 1 }); - - expect(boardsStore.state.lists[0].position).toBe(1); - }); - }); - - describe('toggleFilter', () => { - const dummyFilter = 'x=42'; - let updateTokensSpy; - - beforeEach(() => { - updateTokensSpy = jasmine.createSpy('updateTokens'); - eventHub.$once('updateTokens', updateTokensSpy); - - // prevent using window.history - spyOn(boardsStore, 'updateFiltersUrl').and.callFake(() => {}); - }); - - it('adds the filter if it is not present', () => { - boardsStore.filter.path = 'something'; - - boardsStore.toggleFilter(dummyFilter); - - expect(boardsStore.filter.path).toEqual(`something&${dummyFilter}`); - expect(updateTokensSpy).toHaveBeenCalled(); - expect(boardsStore.updateFiltersUrl).toHaveBeenCalled(); - }); - - it('removes the filter if it is present', () => { - boardsStore.filter.path = `something&${dummyFilter}`; - - boardsStore.toggleFilter(dummyFilter); - - expect(boardsStore.filter.path).toEqual('something'); - expect(updateTokensSpy).toHaveBeenCalled(); - expect(boardsStore.updateFiltersUrl).toHaveBeenCalled(); - }); - }); - - describe('lists', () => { - it('creates new list without persisting to DB', () => { - boardsStore.addList(listObj); - - expect(boardsStore.state.lists.length).toBe(1); - }); - - it('finds list by ID', () => { - boardsStore.addList(listObj); - const list = boardsStore.findList('id', listObj.id); - - expect(list.id).toBe(listObj.id); - }); - - it('finds list by type', () => { - boardsStore.addList(listObj); - const list = boardsStore.findList('type', 'label'); - - expect(list).toBeDefined(); - }); - - it('finds list by label ID', () => { - boardsStore.addList(listObj); - const list = boardsStore.findListByLabelId(listObj.label.id); - - expect(list.id).toBe(listObj.id); - }); - - it('gets issue when new list added', done => { - boardsStore.addList(listObj); - const list = boardsStore.findList('id', listObj.id); - - expect(boardsStore.state.lists.length).toBe(1); - - setTimeout(() => { - expect(list.issues.length).toBe(1); - expect(list.issues[0].id).toBe(1); - done(); - }, 0); - }); - - it('persists new list', done => { - boardsStore.new({ - title: 'Test', - list_type: 'label', - label: { - id: 1, - title: 'Testing', - color: 'red', - description: 'testing;', - }, - }); - - expect(boardsStore.state.lists.length).toBe(1); - - setTimeout(() => { - const list = boardsStore.findList('id', listObj.id); - - expect(list).toBeDefined(); - expect(list.id).toBe(listObj.id); - expect(list.position).toBe(0); - done(); - }, 0); - }); - - it('check for blank state adding', () => { - expect(boardsStore.shouldAddBlankState()).toBe(true); - }); - - it('check for blank state not adding', () => { - boardsStore.addList(listObj); - - expect(boardsStore.shouldAddBlankState()).toBe(false); - }); - - it('check for blank state adding when closed list exist', () => { - boardsStore.addList({ - list_type: 'closed', - }); - - expect(boardsStore.shouldAddBlankState()).toBe(true); - }); - - it('adds the blank state', () => { - boardsStore.addBlankState(); - - const list = boardsStore.findList('type', 'blank', 'blank'); - - expect(list).toBeDefined(); - }); - - it('removes list from state', () => { - boardsStore.addList(listObj); - - expect(boardsStore.state.lists.length).toBe(1); - - boardsStore.removeList(listObj.id, 'label'); - - expect(boardsStore.state.lists.length).toBe(0); - }); - - it('moves the position of lists', () => { - const listOne = boardsStore.addList(listObj); - boardsStore.addList(listObjDuplicate); - - expect(boardsStore.state.lists.length).toBe(2); - - boardsStore.moveList(listOne, [listObjDuplicate.id, listObj.id]); - - expect(listOne.position).toBe(1); - }); - - it('moves an issue from one list to another', done => { - const listOne = boardsStore.addList(listObj); - const listTwo = boardsStore.addList(listObjDuplicate); - - expect(boardsStore.state.lists.length).toBe(2); - - setTimeout(() => { - expect(listOne.issues.length).toBe(1); - expect(listTwo.issues.length).toBe(1); - - boardsStore.moveIssueToList(listOne, listTwo, listOne.findIssue(1)); - - expect(listOne.issues.length).toBe(0); - expect(listTwo.issues.length).toBe(1); - - done(); - }, 0); - }); - - it('moves an issue from backlog to a list', done => { - const backlog = boardsStore.addList({ - ...listObj, - list_type: 'backlog', - }); - const listTwo = boardsStore.addList(listObjDuplicate); - - expect(boardsStore.state.lists.length).toBe(2); - - setTimeout(() => { - expect(backlog.issues.length).toBe(1); - expect(listTwo.issues.length).toBe(1); - - boardsStore.moveIssueToList(backlog, listTwo, backlog.findIssue(1)); - - expect(backlog.issues.length).toBe(0); - expect(listTwo.issues.length).toBe(1); - - done(); - }, 0); - }); - - it('moves issue to top of another list', done => { - const listOne = boardsStore.addList(listObj); - const listTwo = boardsStore.addList(listObjDuplicate); - - expect(boardsStore.state.lists.length).toBe(2); - - setTimeout(() => { - listOne.issues[0].id = 2; - - expect(listOne.issues.length).toBe(1); - expect(listTwo.issues.length).toBe(1); - - boardsStore.moveIssueToList(listOne, listTwo, listOne.findIssue(2), 0); - - expect(listOne.issues.length).toBe(0); - expect(listTwo.issues.length).toBe(2); - expect(listTwo.issues[0].id).toBe(2); - expect(boardsStore.moveIssue).toHaveBeenCalledWith(2, listOne.id, listTwo.id, null, 1); - - done(); - }, 0); - }); - - it('moves issue to bottom of another list', done => { - const listOne = boardsStore.addList(listObj); - const listTwo = boardsStore.addList(listObjDuplicate); - - expect(boardsStore.state.lists.length).toBe(2); - - setTimeout(() => { - listOne.issues[0].id = 2; - - expect(listOne.issues.length).toBe(1); - expect(listTwo.issues.length).toBe(1); - - boardsStore.moveIssueToList(listOne, listTwo, listOne.findIssue(2), 1); - - expect(listOne.issues.length).toBe(0); - expect(listTwo.issues.length).toBe(2); - expect(listTwo.issues[1].id).toBe(2); - expect(boardsStore.moveIssue).toHaveBeenCalledWith(2, listOne.id, listTwo.id, 1, null); - - done(); - }, 0); - }); - - it('moves issue in list', done => { - const issue = new ListIssue({ - title: 'Testing', - id: 2, - iid: 2, - confidential: false, - labels: [], - assignees: [], - }); - const list = boardsStore.addList(listObj); - - setTimeout(() => { - list.addIssue(issue); - - expect(list.issues.length).toBe(2); - - boardsStore.moveIssueInList(list, issue, 0, 1, [1, 2]); - - expect(list.issues[0].id).toBe(2); - expect(boardsStore.moveIssue).toHaveBeenCalledWith(2, null, null, 1, null); - - done(); - }); - }); - }); - - describe('setListDetail', () => { - it('sets the list detail', () => { - boardsStore.detail.list = 'not a list'; - - const dummyValue = 'new list'; - boardsStore.setListDetail(dummyValue); - - expect(boardsStore.detail.list).toEqual(dummyValue); - }); - }); - - describe('clearDetailIssue', () => { - it('resets issue details', () => { - boardsStore.detail.issue = 'something'; - - boardsStore.clearDetailIssue(); - - expect(boardsStore.detail.issue).toEqual({}); - }); - }); - - describe('setIssueDetail', () => { - it('sets issue details', () => { - boardsStore.detail.issue = 'some details'; - - const dummyValue = 'new details'; - boardsStore.setIssueDetail(dummyValue); - - expect(boardsStore.detail.issue).toEqual(dummyValue); - }); - }); - - describe('startMoving', () => { - it('stores list and issue', () => { - const dummyIssue = 'some issue'; - const dummyList = 'some list'; - - boardsStore.startMoving(dummyList, dummyIssue); - - expect(boardsStore.moving.issue).toEqual(dummyIssue); - expect(boardsStore.moving.list).toEqual(dummyList); - }); - }); - - describe('setTimeTrackingLimitToHours', () => { - it('sets the timeTracking.LimitToHours option', () => { - boardsStore.timeTracking.limitToHours = false; - - boardsStore.setTimeTrackingLimitToHours('true'); - - expect(boardsStore.timeTracking.limitToHours).toEqual(true); - }); - }); - - describe('setCurrentBoard', () => { - const dummyBoard = 'hoverboard'; - - it('sets the current board', () => { - const { state } = boardsStore; - state.currentBoard = null; - - boardsStore.setCurrentBoard(dummyBoard); - - expect(state.currentBoard).toEqual(dummyBoard); - }); - }); - - describe('toggleMultiSelect', () => { - let basicIssueObj; - - beforeAll(() => { - basicIssueObj = { id: 987654 }; - }); - - afterEach(() => { - boardsStore.clearMultiSelect(); - }); - - it('adds issue when not present', () => { - boardsStore.toggleMultiSelect(basicIssueObj); - - const selectedIds = boardsStore.multiSelect.list.map(x => x.id); - - expect(selectedIds.includes(basicIssueObj.id)).toEqual(true); - }); - - it('removes issue when issue is present', () => { - boardsStore.toggleMultiSelect(basicIssueObj); - let selectedIds = boardsStore.multiSelect.list.map(x => x.id); - - expect(selectedIds.includes(basicIssueObj.id)).toEqual(true); - - boardsStore.toggleMultiSelect(basicIssueObj); - selectedIds = boardsStore.multiSelect.list.map(x => x.id); - - expect(selectedIds.includes(basicIssueObj.id)).toEqual(false); - }); - }); - - describe('clearMultiSelect', () => { - it('clears all the multi selected issues', () => { - const issue1 = { id: 12345 }; - const issue2 = { id: 12346 }; - - boardsStore.toggleMultiSelect(issue1); - boardsStore.toggleMultiSelect(issue2); - - expect(boardsStore.multiSelect.list.length).toEqual(2); - - boardsStore.clearMultiSelect(); - - expect(boardsStore.multiSelect.list.length).toEqual(0); - }); - }); - - describe('moveMultipleIssuesToList', () => { - it('move issues on the new index', done => { - const listOne = boardsStore.addList(listObj); - const listTwo = boardsStore.addList(listObjDuplicate); - - expect(boardsStore.state.lists.length).toBe(2); - - setTimeout(() => { - expect(listOne.issues.length).toBe(1); - expect(listTwo.issues.length).toBe(1); - - boardsStore.moveMultipleIssuesToList({ - listFrom: listOne, - listTo: listTwo, - issues: listOne.issues, - newIndex: 0, - }); - - expect(listTwo.issues.length).toBe(1); - - done(); - }, 0); - }); - }); - - describe('moveMultipleIssuesInList', () => { - it('moves multiple issues in list', done => { - const issueObj = { - title: 'Issue #1', - id: 12345, - iid: 2, - confidential: false, - labels: [], - assignees: [], - }; - const issue1 = new ListIssue(issueObj); - const issue2 = new ListIssue({ - ...issueObj, - title: 'Issue #2', - id: 12346, - }); - - const list = boardsStore.addList(listObj); - - waitForPromises() - .then(() => { - list.addIssue(issue1); - list.addIssue(issue2); - - expect(list.issues.length).toBe(3); - expect(list.issues[0].id).not.toBe(issue2.id); - - boardsStore.moveMultipleIssuesInList({ - list, - issues: [issue1, issue2], - oldIndicies: [0], - newIndex: 1, - idArray: [1, 12345, 12346], - }); - - expect(list.issues[0].id).toBe(issue1.id); - - expect(boardsStore.moveMultipleIssues).toHaveBeenCalledWith({ - ids: [issue1.id, issue2.id], - fromListId: null, - toListId: null, - moveBeforeId: 1, - moveAfterId: null, - }); - - done(); - }) - .catch(done.fail); - }); - }); -}); diff --git a/spec/javascripts/boards/mock_data.js b/spec/javascripts/boards/mock_data.js index 91e0c1673c9..fcb5d9cfa08 100644 --- a/spec/javascripts/boards/mock_data.js +++ b/spec/javascripts/boards/mock_data.js @@ -1,53 +1,7 @@ import boardsStore from '~/boards/stores/boards_store'; +import { listObj } from '../../frontend/boards/mock_data'; -export const setMockEndpoints = (opts = {}) => { - const boardsEndpoint = opts.boardsEndpoint || '/test/issue-boards/-/boards.json'; - const listsEndpoint = opts.listsEndpoint || '/test/-/boards/1/lists'; - const bulkUpdatePath = opts.bulkUpdatePath || ''; - const boardId = opts.boardId || '1'; - - boardsStore.setEndpoints({ - boardsEndpoint, - listsEndpoint, - bulkUpdatePath, - boardId, - }); -}; - -export const boardObj = { - id: 1, - name: 'test', - milestone_id: null, -}; - -export const listObj = { - id: 300, - position: 0, - title: 'Test', - list_type: 'label', - weight: 3, - label: { - id: 5000, - title: 'Test', - color: 'red', - description: 'testing;', - textColor: 'white', - }, -}; - -export const listObjDuplicate = { - id: listObj.id, - position: 1, - title: 'Test', - list_type: 'label', - weight: 3, - label: { - id: listObj.label.id, - title: 'Test', - color: 'red', - description: 'testing;', - }, -}; +export * from '../../frontend/boards/mock_data'; export const BoardsMockData = { GET: { @@ -86,59 +40,16 @@ export const boardsMockInterceptor = config => { return [200, body]; }; -export const mockAssigneesList = [ - { - id: 2, - name: 'Terrell Graham', - username: 'monserrate.gleichner', - state: 'active', - avatar_url: 'https://www.gravatar.com/avatar/598fd02741ac58b88854a99d16704309?s=80&d=identicon', - web_url: 'http://127.0.0.1:3001/monserrate.gleichner', - path: '/monserrate.gleichner', - }, - { - id: 12, - name: 'Susy Johnson', - username: 'tana_harvey', - state: 'active', - avatar_url: 'https://www.gravatar.com/avatar/e021a7b0f3e4ae53b5068d487e68c031?s=80&d=identicon', - web_url: 'http://127.0.0.1:3001/tana_harvey', - path: '/tana_harvey', - }, - { - id: 20, - name: 'Conchita Eichmann', - username: 'juliana_gulgowski', - state: 'active', - avatar_url: 'https://www.gravatar.com/avatar/c43c506cb6fd7b37017d3b54b94aa937?s=80&d=identicon', - web_url: 'http://127.0.0.1:3001/juliana_gulgowski', - path: '/juliana_gulgowski', - }, - { - id: 6, - name: 'Bryce Turcotte', - username: 'melynda', - state: 'active', - avatar_url: 'https://www.gravatar.com/avatar/cc2518f2c6f19f8fac49e1a5ee092a9b?s=80&d=identicon', - web_url: 'http://127.0.0.1:3001/melynda', - path: '/melynda', - }, - { - id: 1, - name: 'Administrator', - username: 'root', - state: 'active', - avatar_url: 'https://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon', - web_url: 'http://127.0.0.1:3001/root', - path: '/root', - }, -]; +export const setMockEndpoints = (opts = {}) => { + const boardsEndpoint = opts.boardsEndpoint || '/test/issue-boards/-/boards.json'; + const listsEndpoint = opts.listsEndpoint || '/test/-/boards/1/lists'; + const bulkUpdatePath = opts.bulkUpdatePath || ''; + const boardId = opts.boardId || '1'; -export const mockMilestone = { - id: 1, - state: 'active', - title: 'Milestone title', - description: 'Harum corporis aut consequatur quae dolorem error sequi quia.', - start_date: '2018-01-01', - due_date: '2019-12-31', + boardsStore.setEndpoints({ + boardsEndpoint, + listsEndpoint, + bulkUpdatePath, + boardId, + }); }; |
