From 00c6fd2692780b32a0c071ceb550eb2acbf2948a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20G=C3=BCnther?= Date: Wed, 23 Mar 2022 09:49:41 +0100 Subject: [PATCH 01/12] Add new query attribute include_subprojects --- .../work_packages/filter/project_filter.rb | 15 ++++++++++++++- app/models/query.rb | 1 + ...0323083000_add_include_subprojects_to_query.rb | 14 ++++++++++++++ lib/api/v3/queries/query_representer.rb | 2 ++ 4 files changed, 31 insertions(+), 1 deletion(-) create mode 100644 db/migrate/20220323083000_add_include_subprojects_to_query.rb diff --git a/app/models/queries/work_packages/filter/project_filter.rb b/app/models/queries/work_packages/filter/project_filter.rb index 9f81d54458..11866a7abc 100644 --- a/app/models/queries/work_packages/filter/project_filter.rb +++ b/app/models/queries/work_packages/filter/project_filter.rb @@ -59,7 +59,7 @@ class Queries::WorkPackages::Filter::ProjectFilter < Queries::WorkPackages::Filt available_projects = visible_projects.index_by(&:id) values - .map { |project_id| available_projects[project_id.to_i] } + .flat_map { |project_id| expanded_subprojects(available_projects[project_id.to_i]) } .compact end @@ -68,4 +68,17 @@ class Queries::WorkPackages::Filter::ProjectFilter < Queries::WorkPackages::Filt def visible_projects @visible_projects ||= Project.visible.active end + + ## + # Depending on whether subprojects are included in the query, + # expand selected projects with its descendants + def expanded_subprojects(selected_project) + return if selected_project.nil? + + if context.include_subprojects? + [selected_project] + else + [selected_project].concat(selected_project.descendants.visible) + end + end end diff --git a/app/models/query.rb b/app/models/query.rb index e14a6f5a5a..9fc48f8938 100644 --- a/app/models/query.rb +++ b/app/models/query.rb @@ -62,6 +62,7 @@ class Query < ApplicationRecord query.add_default_filter query.set_default_sort query.show_hierarchies = true + query.include_subprojects = Setting.display_subprojects_work_packages? end end diff --git a/db/migrate/20220323083000_add_include_subprojects_to_query.rb b/db/migrate/20220323083000_add_include_subprojects_to_query.rb new file mode 100644 index 0000000000..eac71f74d7 --- /dev/null +++ b/db/migrate/20220323083000_add_include_subprojects_to_query.rb @@ -0,0 +1,14 @@ +class AddIncludeSubprojectsToQuery < ActiveRecord::Migration[6.1] + def change + add_column :queries, + :include_subprojects, + :boolean, + null: false, + default: Setting.display_subprojects_work_packages? + + # Remove the default now + reversible do |dir| + dir.up { change_column_default :queries, :include_subprojects, nil } + end + end +end diff --git a/lib/api/v3/queries/query_representer.rb b/lib/api/v3/queries/query_representer.rb index 2543999998..95def2a591 100644 --- a/lib/api/v3/queries/query_representer.rb +++ b/lib/api/v3/queries/query_representer.rb @@ -288,6 +288,8 @@ module API property :filters, exec_context: :decorator + property :include_subprojects + property :display_sums, as: :sums property :public From 7673d3951514bc0d2db4d10ce462c20aed1812c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20G=C3=BCnther?= Date: Mon, 28 Mar 2022 08:56:54 +0200 Subject: [PATCH 02/12] Set include_subprojects in factories --- spec/factories/query_factory.rb | 1 + spec_legacy/fixtures/queries.yml | 9 +++++++++ 2 files changed, 10 insertions(+) diff --git a/spec/factories/query_factory.rb b/spec/factories/query_factory.rb index 35b81959f1..e2ad644639 100644 --- a/spec/factories/query_factory.rb +++ b/spec/factories/query_factory.rb @@ -30,6 +30,7 @@ FactoryBot.define do factory :query do project user factory: :user + include_subprojects { true } sequence(:name) { |n| "Query #{n}" } factory :public_query do diff --git a/spec_legacy/fixtures/queries.yml b/spec_legacy/fixtures/queries.yml index 465e506de7..8abf181191 100644 --- a/spec_legacy/fixtures/queries.yml +++ b/spec_legacy/fixtures/queries.yml @@ -31,6 +31,7 @@ queries_001: id: 1 project_id: 1 public: true + include_subprojects: true name: Multiple custom fields query filters: | --- @@ -53,6 +54,7 @@ queries_002: id: 2 project_id: 1 public: false + include_subprojects: true name: Private query for cookbook filters: | --- @@ -71,6 +73,7 @@ queries_003: id: 3 project_id: public: false + include_subprojects: true name: Private query for all projects filters: | --- @@ -85,6 +88,7 @@ queries_004: id: 4 project_id: public: true + include_subprojects: true name: Public query for all projects filters: | --- @@ -99,6 +103,7 @@ queries_005: id: 5 project_id: public: true + include_subprojects: true name: Open issues by priority and type filters: | --- @@ -119,6 +124,7 @@ queries_006: id: 6 project_id: public: true + include_subprojects: true name: Open issues grouped by type filters: | --- @@ -138,6 +144,7 @@ queries_007: id: 7 project_id: 2 public: true + include_subprojects: true name: Public query for project 2 filters: | --- @@ -152,6 +159,7 @@ queries_008: id: 8 project_id: 2 public: false + include_subprojects: true name: Private query for project 2 filters: | --- @@ -166,6 +174,7 @@ queries_009: id: 9 project_id: public: true + include_subprojects: true name: Open issues grouped by list custom field filters: | --- From 92430943475b775f80d69d805ee1116e1a0dd0c6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20G=C3=BCnther?= Date: Mon, 28 Mar 2022 08:57:04 +0200 Subject: [PATCH 03/12] Replace create_query helper with default endpoint --- app/contracts/queries/base_contract.rb | 1 + app/services/base_services/set_attributes.rb | 2 ++ app/services/queries/create_service.rb | 23 ++++--------------- .../queries/set_attributes_service.rb | 16 ++++++++++++- lib/api/v3/queries/queries_api.rb | 6 ++--- lib/api/v3/queries/query_helper.rb | 11 --------- 6 files changed, 25 insertions(+), 34 deletions(-) diff --git a/app/contracts/queries/base_contract.rb b/app/contracts/queries/base_contract.rb index e2ed078289..813a40c2d9 100644 --- a/app/contracts/queries/base_contract.rb +++ b/app/contracts/queries/base_contract.rb @@ -43,6 +43,7 @@ module Queries attribute :highlighted_attributes attribute :show_hierarchies attribute :display_representation + attribute :include_subprojects attribute :column_names # => columns attribute :filters diff --git a/app/services/base_services/set_attributes.rb b/app/services/base_services/set_attributes.rb index 46109999d2..7fb2618cc0 100644 --- a/app/services/base_services/set_attributes.rb +++ b/app/services/base_services/set_attributes.rb @@ -31,6 +31,8 @@ module BaseServices include Contracted def initialize(user:, model:, contract_class:, contract_options: {}) + super() + self.user = user self.model = prepare_model(model) diff --git a/app/services/queries/create_service.rb b/app/services/queries/create_service.rb index 3ae93b737b..f5dbc9c9a9 100644 --- a/app/services/queries/create_service.rb +++ b/app/services/queries/create_service.rb @@ -26,20 +26,9 @@ # See COPYRIGHT and LICENSE files for more details. #++ -class Queries::CreateService < Queries::BaseService - def initialize(**args) - super(**args) - self.contract_class = Queries::CreateContract - end - - def call(query) - remove_invalid_order(query) - super - end - - private - - def remove_invalid_order(query) +class Queries::CreateService < ::BaseServices::Create + def after_validate(params, call) + query = call.result # Check which of the work package IDs exist ids = query.ordered_work_packages.map(&:work_package_id) existent_wps = WorkPackage.where(id: ids).pluck(:id).to_set @@ -47,11 +36,7 @@ class Queries::CreateService < Queries::BaseService query.ordered_work_packages = query.ordered_work_packages.select do |order_item| existent_wps.include?(order_item.work_package_id) end - end - - def service_result(result, errors, query) - query.update user: user - super + call end end diff --git a/app/services/queries/set_attributes_service.rb b/app/services/queries/set_attributes_service.rb index b3ae12f922..6488d5aed9 100644 --- a/app/services/queries/set_attributes_service.rb +++ b/app/services/queries/set_attributes_service.rb @@ -26,4 +26,18 @@ # See COPYRIGHT and LICENSE files for more details. #++ -class Queries::SetAttributesService < ::BaseServices::SetAttributes; end +class Queries::SetAttributesService < ::BaseServices::SetAttributes + def set_default_attributes(_params) + if model.include_subprojects.nil? + model.include_subprojects = Setting.display_subprojects_work_packages? + end + + set_default_user + end + + def set_default_user + model.change_by_system do + model.user = user + end + end +end diff --git a/lib/api/v3/queries/queries_api.rb b/lib/api/v3/queries/queries_api.rb index 7076eac455..f9be692b42 100644 --- a/lib/api/v3/queries/queries_api.rb +++ b/lib/api/v3/queries/queries_api.rb @@ -96,9 +96,9 @@ module API end end - post do - create_query request_body, current_user - end + post &::API::V3::Utilities::Endpoints::Create + .new(model: Query) + .mount route_param :id, type: Integer, desc: 'Query ID' do after_validation do diff --git a/lib/api/v3/queries/query_helper.rb b/lib/api/v3/queries/query_helper.rb index 9d511eff04..f9cc83c776 100644 --- a/lib/api/v3/queries/query_helper.rb +++ b/lib/api/v3/queries/query_helper.rb @@ -63,17 +63,6 @@ module API end end - def create_query(request_body, current_user) - rep = representer.new Query.new, current_user: current_user - query = rep.from_hash request_body - call = ::Queries::CreateService.new(user: current_user).call query - if call.success? - representer.new call.result, current_user: current_user, embed_links: true - else - fail ::API::Errors::ErrorBase.create_and_merge_errors(call.errors) - end - end - def update_query(query, request_body, current_user) rep = representer.new query, current_user: current_user query = rep.from_hash request_body From ad9e4546b52509975862f6663ad37a98730123a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20G=C3=BCnther?= Date: Mon, 28 Mar 2022 09:16:21 +0200 Subject: [PATCH 04/12] Adapt spec to new query create service --- app/services/queries/create_service.rb | 2 +- spec/services/queries/create_service_spec.rb | 15 +++++++++------ .../work_packages/table_configuration_modal.rb | 2 +- 3 files changed, 11 insertions(+), 8 deletions(-) diff --git a/app/services/queries/create_service.rb b/app/services/queries/create_service.rb index f5dbc9c9a9..3f0ffd5215 100644 --- a/app/services/queries/create_service.rb +++ b/app/services/queries/create_service.rb @@ -27,7 +27,7 @@ #++ class Queries::CreateService < ::BaseServices::Create - def after_validate(params, call) + def after_validate(_params, call) query = call.result # Check which of the work package IDs exist ids = query.ordered_work_packages.map(&:work_package_id) diff --git a/spec/services/queries/create_service_spec.rb b/spec/services/queries/create_service_spec.rb index 328b0c3e8b..58370d5033 100644 --- a/spec/services/queries/create_service_spec.rb +++ b/spec/services/queries/create_service_spec.rb @@ -30,17 +30,20 @@ require 'spec_helper' describe Queries::CreateService do let(:user) { build_stubbed(:admin) } - let(:query) { build(:query, user: user) } let(:instance) { described_class.new(user: user) } - subject { instance.call(query).result } + subject { instance.call(params).result } describe 'ordered work packages' do let!(:work_package) { create :work_package } - - before do - query.ordered_work_packages.build(work_package_id: work_package.id, position: 0) - query.ordered_work_packages.build(work_package_id: 99999, position: 1) + let(:params) do + { + name: 'My query', + ordered_work_packages: { + work_package.id => 0, + 9999 => 1 + } + } end it 'removes items for which work packages do not exist' do diff --git a/spec/support/components/work_packages/table_configuration_modal.rb b/spec/support/components/work_packages/table_configuration_modal.rb index 01880f9c75..a449160fed 100644 --- a/spec/support/components/work_packages/table_configuration_modal.rb +++ b/spec/support/components/work_packages/table_configuration_modal.rb @@ -89,7 +89,7 @@ module Components end def expect_disabled_tab(name) - expect(page).to have_selector("#{selector} [data-qa-tab-disabled]", text: name.upcase) + expect(page).to have_selector("#{selector} [data-qa-tab-disabled]", text: name.upcase, wait: 10) end def selected_tab(name) From e5b05b9fa0cfaeb41da6d4495f3dfea9dfdb5a39 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20G=C3=BCnther?= Date: Mon, 28 Mar 2022 09:45:55 +0200 Subject: [PATCH 05/12] Add spec for changed subproject behavior --- app/models/query.rb | 2 +- spec/factories/query_factory.rb | 2 +- ...ults_subproject_filter_integration_spec.rb | 106 +++++++++++++++--- 3 files changed, 94 insertions(+), 16 deletions(-) diff --git a/app/models/query.rb b/app/models/query.rb index 9fc48f8938..82e91ccb6d 100644 --- a/app/models/query.rb +++ b/app/models/query.rb @@ -369,7 +369,7 @@ class Query < ApplicationRecord subproject_filter = Queries::WorkPackages::Filter::SubprojectFilter.create! subproject_filter.context = self - subproject_filter.operator = if Setting.display_subprojects_work_packages? + subproject_filter.operator = if include_subprojects? '*' else '!*' diff --git a/spec/factories/query_factory.rb b/spec/factories/query_factory.rb index e2ad644639..6d44755050 100644 --- a/spec/factories/query_factory.rb +++ b/spec/factories/query_factory.rb @@ -30,7 +30,7 @@ FactoryBot.define do factory :query do project user factory: :user - include_subprojects { true } + include_subprojects { Setting.display_subprojects_work_packages? } sequence(:name) { |n| "Query #{n}" } factory :public_query do diff --git a/spec/models/query/results_subproject_filter_integration_spec.rb b/spec/models/query/results_subproject_filter_integration_spec.rb index 52adc6bb80..5b9330e052 100644 --- a/spec/models/query/results_subproject_filter_integration_spec.rb +++ b/spec/models/query/results_subproject_filter_integration_spec.rb @@ -58,34 +58,112 @@ describe ::Query::Results, 'Subproject filter integration', type: :model, with_m login_as user end - context 'when subprojects included', with_settings: { display_subprojects_work_packages: true } do - it 'shows the sub work packages' do - expect(query_results.work_packages).to match_array [parent_wp, child_wp] + describe 'new default query' do + context 'when subprojects included', with_settings: { display_subprojects_work_packages: true } do + it 'shows the sub work packages' do + expect(query_results.work_packages).to match_array [parent_wp, child_wp] + end + end + + context 'when subprojects not included', with_settings: { display_subprojects_work_packages: false } do + it 'does not show the sub work packages' do + expect(query_results.work_packages).to match_array [parent_wp] + end + + context 'when subproject filter added manually' do + before do + query.add_filter('subproject_id', '=', [child_project.id]) + end + + it 'shows the sub work packages' do + expect(query_results.work_packages).to match_array [parent_wp, child_wp] + end + end + + context 'when only subproject filter added manually' do + before do + query.add_filter('only_subproject_id', '=', [child_project.id]) + end + + it 'shows only the sub work packages' do + expect(query_results.work_packages).to match_array [child_wp] + end + end end end - context 'when subprojects not included', with_settings: { display_subprojects_work_packages: false } do - it 'does not show the sub work packages' do - expect(query_results.work_packages).to match_array [parent_wp] + describe 'query with overriden include_subprojects = true' do + before do + query.include_subprojects = true end - context 'when subproject filter added manually' do - before do - query.add_filter('subproject_id', '=', [child_project.id]) + context 'when subprojects included', with_settings: { display_subprojects_work_packages: true } do + it 'shows the sub work packages' do + expect(query_results.work_packages).to match_array [parent_wp, child_wp] end + end + context 'when subprojects not included', with_settings: { display_subprojects_work_packages: false } do it 'shows the sub work packages' do expect(query_results.work_packages).to match_array [parent_wp, child_wp] end + + context 'when subproject filter added manually' do + before do + query.add_filter('subproject_id', '=', [child_project.id]) + end + + it 'shows the sub work packages' do + expect(query_results.work_packages).to match_array [parent_wp, child_wp] + end + end + + context 'when only subproject filter added manually' do + before do + query.add_filter('only_subproject_id', '=', [child_project.id]) + end + + it 'shows only the sub work packages' do + expect(query_results.work_packages).to match_array [child_wp] + end + end + end + end + + describe 'query with overriden include_subprojects = false' do + before do + query.include_subprojects = false end - context 'when only subproject filter added manually' do - before do - query.add_filter('only_subproject_id', '=', [child_project.id]) + context 'when subprojects included', with_settings: { display_subprojects_work_packages: true } do + it 'does not show the sub work packages' do + expect(query_results.work_packages).to match_array [parent_wp] end + end + + context 'when subprojects not included', with_settings: { display_subprojects_work_packages: false } do + it 'does not show the sub work packages' do + expect(query_results.work_packages).to match_array [parent_wp] + end + + context 'when subproject filter added manually' do + before do + query.add_filter('subproject_id', '=', [child_project.id]) + end + + it 'shows the sub work packages' do + expect(query_results.work_packages).to match_array [parent_wp, child_wp] + end + end + + context 'when only subproject filter added manually' do + before do + query.add_filter('only_subproject_id', '=', [child_project.id]) + end - it 'shows only the sub work packages' do - expect(query_results.work_packages).to match_array [child_wp] + it 'shows only the sub work packages' do + expect(query_results.work_packages).to match_array [child_wp] + end end end end From df220ed594d713eaec09b1de9fe823e6efd1c5b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20G=C3=BCnther?= Date: Mon, 28 Mar 2022 10:21:10 +0200 Subject: [PATCH 06/12] Extend project filter with replaced values and add spec --- .../work_packages/filter/project_filter.rb | 19 ++- app/seeders/demo_data/query_builder.rb | 3 +- .../demo_data/work_package_board_seeder.rb | 1 + lib/api/v3/queries/query_representer.rb | 6 +- .../features/calendar_project_include_spec.rb | 8 +- .../team_planner_project_include_spec.rb | 8 +- ...ork_packages_table_project_include_spec.rb | 14 +- .../timeline/timeline_labels_spec.rb | 6 +- .../queries/query_representer_parsing_spec.rb | 15 +- .../filter/project_filter_spec.rb | 20 ++- ...results_project_filter_integration_spec.rb | 150 ++++++++++++++++++ ...eferences_service_call_integration_spec.rb | 8 +- 12 files changed, 217 insertions(+), 41 deletions(-) create mode 100644 spec/models/query/results_project_filter_integration_spec.rb diff --git a/app/models/queries/work_packages/filter/project_filter.rb b/app/models/queries/work_packages/filter/project_filter.rb index 11866a7abc..dd7e88d9ee 100644 --- a/app/models/queries/work_packages/filter/project_filter.rb +++ b/app/models/queries/work_packages/filter/project_filter.rb @@ -59,8 +59,13 @@ class Queries::WorkPackages::Filter::ProjectFilter < Queries::WorkPackages::Filt available_projects = visible_projects.index_by(&:id) values - .flat_map { |project_id| expanded_subprojects(available_projects[project_id.to_i]) } + .flat_map { |project_id| available_projects[project_id.to_i] } .compact + .uniq + end + + def where + operator_strategy.sql_for_field(projects_and_descendants, self.class.model.table_name, :project_id) end private @@ -72,13 +77,17 @@ class Queries::WorkPackages::Filter::ProjectFilter < Queries::WorkPackages::Filt ## # Depending on whether subprojects are included in the query, # expand selected projects with its descendants - def expanded_subprojects(selected_project) - return if selected_project.nil? + def projects_and_descendants + value_objects + .inject(Set.new) { |project_set, project| project_set + expand_subprojects(project) } + .map(&:id) + end + def expand_subprojects(selected_project) if context.include_subprojects? - [selected_project] - else [selected_project].concat(selected_project.descendants.visible) + else + [selected_project] end end end diff --git a/app/seeders/demo_data/query_builder.rb b/app/seeders/demo_data/query_builder.rb index 17ba39c7f0..d8832d8bbc 100644 --- a/app/seeders/demo_data/query_builder.rb +++ b/app/seeders/demo_data/query_builder.rb @@ -52,7 +52,8 @@ module DemoData public: config.fetch(:public, true), starred: config.fetch(:starred, false), show_hierarchies: config.fetch(:hierarchy, false), - timeline_visible: config.fetch(:timeline, false) + timeline_visible: config.fetch(:timeline, false), + include_subprojects: true } end diff --git a/app/seeders/demo_data/work_package_board_seeder.rb b/app/seeders/demo_data/work_package_board_seeder.rb index 418f8f2533..d950663713 100644 --- a/app/seeders/demo_data/work_package_board_seeder.rb +++ b/app/seeders/demo_data/work_package_board_seeder.rb @@ -160,6 +160,7 @@ module DemoData Query.new(project: project, user: admin).tap do |query| # Make it public so that new members can see it too query.public = true + query.include_subprojects = true query.name = list[:name] diff --git a/lib/api/v3/queries/query_representer.rb b/lib/api/v3/queries/query_representer.rb index 95def2a591..af6f87be52 100644 --- a/lib/api/v3/queries/query_representer.rb +++ b/lib/api/v3/queries/query_representer.rb @@ -259,11 +259,9 @@ module API exec_context: :decorator, getter: nil, setter: ->(fragment:, **) { - next unless represented.new_record? + next if represented.persisted? - Hash(fragment).each do |wp_id, position| - represented.ordered_work_packages.build(work_package_id: wp_id, position: position) - end + represented.ordered_work_packages = fragment } property :starred, diff --git a/modules/calendar/spec/features/calendar_project_include_spec.rb b/modules/calendar/spec/features/calendar_project_include_spec.rb index 5919e63273..c1c36dd0db 100644 --- a/modules/calendar/spec/features/calendar_project_include_spec.rb +++ b/modules/calendar/spec/features/calendar_project_include_spec.rb @@ -50,8 +50,8 @@ describe 'Calendar project include', type: :feature, js: true do dropdown.expect_closed work_package_view.expect_event task - work_package_view.expect_event sub_bug, present: false - work_package_view.expect_event sub_sub_bug, present: false + work_package_view.expect_event sub_bug, present: true + work_package_view.expect_event sub_sub_bug, present: true work_package_view.expect_event other_task work_package_view.expect_event other_other_task, present: false @@ -60,7 +60,7 @@ describe 'Calendar project include', type: :feature, js: true do dropdown.click_button 'Apply' dropdown.expect_count 2 - work_package_view.expect_event sub_bug, present: false + work_package_view.expect_event sub_bug, present: true work_package_view.expect_event sub_sub_bug dropdown.toggle! @@ -74,7 +74,7 @@ describe 'Calendar project include', type: :feature, js: true do page.refresh work_package_view.expect_event task - work_package_view.expect_event sub_bug, present: false + work_package_view.expect_event sub_bug, present: true work_package_view.expect_event sub_sub_bug work_package_view.expect_event other_task work_package_view.expect_event other_other_task diff --git a/modules/team_planner/spec/features/team_planner_project_include_spec.rb b/modules/team_planner/spec/features/team_planner_project_include_spec.rb index 01ac4467cb..107c704de5 100644 --- a/modules/team_planner/spec/features/team_planner_project_include_spec.rb +++ b/modules/team_planner/spec/features/team_planner_project_include_spec.rb @@ -76,8 +76,8 @@ describe 'Team planner project include', type: :feature, js: true do work_package_view.within_lane(user) do work_package_view.expect_event task - work_package_view.expect_event sub_bug, present: false - work_package_view.expect_event sub_sub_bug, present: false + work_package_view.expect_event sub_bug, present: true + work_package_view.expect_event sub_sub_bug, present: true end work_package_view.within_lane(other_user) do @@ -92,7 +92,7 @@ describe 'Team planner project include', type: :feature, js: true do work_package_view.within_lane(user) do work_package_view.expect_event task - work_package_view.expect_event sub_bug, present: false + work_package_view.expect_event sub_bug, present: true work_package_view.expect_event sub_sub_bug end @@ -115,7 +115,7 @@ describe 'Team planner project include', type: :feature, js: true do work_package_view.within_lane(user) do work_package_view.expect_event task - work_package_view.expect_event sub_bug, present: false + work_package_view.expect_event sub_bug, present: true work_package_view.expect_event sub_sub_bug end end diff --git a/spec/features/work_packages/table/work_packages_table_project_include_spec.rb b/spec/features/work_packages/table/work_packages_table_project_include_spec.rb index 867a8c6159..e77c4a4fee 100644 --- a/spec/features/work_packages/table/work_packages_table_project_include_spec.rb +++ b/spec/features/work_packages/table/work_packages_table_project_include_spec.rb @@ -45,29 +45,27 @@ describe 'Calendar project include', type: :feature, js: true do dropdown.click_button 'Apply' dropdown.expect_closed - work_package_view.expect_work_package_listed(task, other_task) - work_package_view.ensure_work_package_not_listed!(sub_bug, sub_sub_bug, other_other_task) + work_package_view.expect_work_package_listed(task, other_task, sub_bug, sub_sub_bug) + work_package_view.ensure_work_package_not_listed!(other_other_task) dropdown.toggle! dropdown.toggle_checkbox(sub_sub_project.id) dropdown.click_button 'Apply' dropdown.expect_count 2 - work_package_view.expect_work_package_listed(task, other_task, sub_sub_bug) - work_package_view.ensure_work_package_not_listed!(sub_bug, other_other_task) + work_package_view.expect_work_package_listed(task, other_task, sub_sub_bug, sub_bug) + work_package_view.ensure_work_package_not_listed!(other_other_task) dropdown.toggle! dropdown.toggle_checkbox(other_project.id) dropdown.click_button 'Apply' dropdown.expect_count 3 - work_package_view.expect_work_package_listed(task, other_task, sub_sub_bug, other_other_task) - work_package_view.ensure_work_package_not_listed!(sub_bug) + work_package_view.expect_work_package_listed(task, other_task, sub_sub_bug, sub_bug, other_other_task) page.refresh - work_package_view.expect_work_package_listed(task, other_task, sub_sub_bug, other_other_task) - work_package_view.ensure_work_package_not_listed!(sub_bug) + work_package_view.expect_work_package_listed(task, other_task, sub_bug, sub_sub_bug, other_other_task) end end end diff --git a/spec/features/work_packages/timeline/timeline_labels_spec.rb b/spec/features/work_packages/timeline/timeline_labels_spec.rb index dd06bc21d5..8dc02e4487 100644 --- a/spec/features/work_packages/timeline/timeline_labels_spec.rb +++ b/spec/features/work_packages/timeline/timeline_labels_spec.rb @@ -135,9 +135,9 @@ RSpec.feature 'Work package timeline labels', # Check the query query = Query.last - expect(query.timeline_labels).to eq 'left' => 'assignee', - 'right' => 'type', - 'farRight' => 'status' + expect(query.timeline_labels).to eq left: 'assignee', + right: 'type', + farRight: 'status' # Revisit page wp_timeline.visit_query query diff --git a/spec/lib/api/v3/queries/query_representer_parsing_spec.rb b/spec/lib/api/v3/queries/query_representer_parsing_spec.rb index 724c0c74cf..160cc520fa 100644 --- a/spec/lib/api/v3/queries/query_representer_parsing_spec.rb +++ b/spec/lib/api/v3/queries/query_representer_parsing_spec.rb @@ -31,7 +31,7 @@ require 'spec_helper' describe ::API::V3::Queries::QueryRepresenter, 'parsing' do include ::API::V3::Utilities::PathHelper - let(:query) { build_stubbed(:query, project: project) } + let(:query) { ::API::ParserStruct.new } let(:project) { build_stubbed(:project) } let(:user) { build_stubbed(:user) } let(:representer) do @@ -82,10 +82,7 @@ describe ::API::V3::Queries::QueryRepresenter, 'parsing' do end it 'unsets group_by' do - expect(query).to be_grouped expect(query.group_by).to eq('project') - - expect(subject).not_to be_grouped end end @@ -115,20 +112,20 @@ describe ::API::V3::Queries::QueryRepresenter, 'parsing' do end before do - allow(query).to receive(:new_record?).and_return(new_record) + allow(query).to receive(:persisted?).and_return(persisted) end context 'if query is new' do - let(:new_record) { true } + let(:persisted) { nil } it 'sets ordered_work_packages' do - order = subject.ordered_work_packages.map { |el| [el.work_package_id, el.position] } - expect(order).to match_array [[50, 0], [38, 1234], [102, 81234123]] + order = subject.ordered_work_packages + expect(order).to eq({ '50' => 0, '38' => 1234, '102' => 81234123 }) end end context 'if query is not new' do - let(:new_record) { false } + let(:persisted) { true } it 'sets ordered_work_packages' do allow(query) diff --git a/spec/models/queries/work_packages/filter/project_filter_spec.rb b/spec/models/queries/work_packages/filter/project_filter_spec.rb index d1f77b4995..d6c57c737e 100644 --- a/spec/models/queries/work_packages/filter/project_filter_spec.rb +++ b/spec/models/queries/work_packages/filter/project_filter_spec.rb @@ -106,13 +106,29 @@ describe Queries::WorkPackages::Filter::ProjectFilter, type: :model do end describe '#value_objects' do + let(:selected) { visible_projects.first } + let(:visible_descendants) { [] } + let(:descendants) { double('Project', visible: visible_descendants) } # rubocop:disable RSpec/VerifiedDoubles + before do - instance.values = [visible_projects.first.id.to_s] + allow(selected).to receive(:descendants).and_return(descendants) + + instance.values = [selected.id.to_s] end it 'returns an array of projects' do expect(instance.value_objects) - .to match_array([visible_projects.first]) + .to match_array([selected]) + end + + context 'with a visible child' do + let(:child) { build_stubbed(:project, parent: selected, id: 2134) } + let(:visible_descendants) { [child] } + + it 'still only returns the parent object' do + expect(instance.value_objects) + .to match_array([selected]) + end end end end diff --git a/spec/models/query/results_project_filter_integration_spec.rb b/spec/models/query/results_project_filter_integration_spec.rb new file mode 100644 index 0000000000..7ff0f3e918 --- /dev/null +++ b/spec/models/query/results_project_filter_integration_spec.rb @@ -0,0 +1,150 @@ +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) 2012-2022 the OpenProject GmbH +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See COPYRIGHT and LICENSE files for more details. +#++ + +require 'spec_helper' + +describe ::Query::Results, 'Project filter integration', type: :model, with_mail: false do + let(:query) do + build(:query, + user: user, + project: parent_project).tap do |q| + q.filters.clear + end + end + let(:query_results) do + described_class.new query + end + + shared_let(:parent_project) { create :project } + shared_let(:child_project) { create :project, parent: parent_project } + + shared_let(:second_parent_project) { create :project } + shared_let(:second_child_project) { create :project, parent: second_parent_project } + + shared_let(:user) do + create(:user, + firstname: 'user', + lastname: '1', + member_in_projects: [parent_project, child_project, second_parent_project, second_child_project], + member_with_permissions: [:view_work_packages]) + end + + shared_let(:parent_wp) { create :work_package, project: parent_project } + shared_let(:child_wp) { create :work_package, project: child_project } + + shared_let(:second_parent_wp) { create :work_package, project: second_parent_project } + shared_let(:second_child_wp) { create :work_package, project: second_child_project } + + before do + login_as user + end + + describe 'both parent projects selected' do + before do + query.add_filter 'project_id', '=', [parent_project.id, second_parent_project.id] + end + + context 'when subprojects included', with_settings: { display_subprojects_work_packages: true } do + it 'shows the sub work packages' do + expect(query_results.work_packages).to match_array [parent_wp, child_wp, second_parent_wp, second_child_wp] + end + end + + context 'when subprojects not included', with_settings: { display_subprojects_work_packages: false } do + it 'does not show the sub work packages' do + expect(query_results.work_packages).to match_array [parent_wp, second_parent_wp] + end + end + + context 'when subprojects explicitly disabled' do + before do + query.include_subprojects = false + end + + it 'does not show the sub work packages' do + expect(query_results.work_packages).to match_array [parent_wp, second_parent_wp] + end + end + end + + describe 'one parent projects selected' do + before do + query.add_filter 'project_id', '=', [second_parent_project.id] + end + + context 'when subprojects included', with_settings: { display_subprojects_work_packages: true } do + it 'shows the sub work packages' do + expect(query_results.work_packages).to match_array [second_parent_wp, second_child_wp] + end + end + + context 'when subprojects not included', with_settings: { display_subprojects_work_packages: false } do + it 'does not show the sub work packages' do + expect(query_results.work_packages).to match_array [second_parent_wp] + end + end + + context 'when subprojects explicitly disabled' do + before do + query.include_subprojects = false + end + + it 'does not show the sub work packages' do + expect(query_results.work_packages).to match_array [second_parent_wp] + end + end + end + + describe 'one parent and one other child selected' do + before do + query.add_filter 'project_id', '=', [child_project.id, second_parent_project.id] + end + + context 'when subprojects included', with_settings: { display_subprojects_work_packages: true } do + it 'shows the sub work packages' do + expect(query_results.work_packages).to match_array [child_wp, second_parent_wp, second_child_wp] + end + end + + context 'when subprojects not included', with_settings: { display_subprojects_work_packages: false } do + it 'does not show the sub work packages' do + expect(query_results.work_packages).to match_array [child_wp, second_parent_wp] + end + end + + context 'when subprojects explicitly disabled' do + before do + query.include_subprojects = false + end + + it 'does not show the sub work packages' do + expect(query_results.work_packages).to match_array [child_wp, second_parent_wp] + end + end + end +end diff --git a/spec/services/principals/replace_references_service_call_integration_spec.rb b/spec/services/principals/replace_references_service_call_integration_spec.rb index 45817a214d..59f666cf0d 100644 --- a/spec/services/principals/replace_references_service_call_integration_spec.rb +++ b/spec/services/principals/replace_references_service_call_integration_spec.rb @@ -380,7 +380,13 @@ describe Principals::ReplaceReferencesService, '#call', type: :model do context 'with Query' do it_behaves_like 'rewritten record', :query, - :user_id + :user_id do + let(:attributes) do + { + include_subprojects: true + } + end + end end context 'with CostQuery' do From bd67f1e9bcea7927d57647186a9aaa059ca58795 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20G=C3=BCnther?= Date: Mon, 28 Mar 2022 14:58:13 +0200 Subject: [PATCH 07/12] Add a ParserStruct overriding Enumerable#group_by --- app/services/api/parser_struct.rb | 42 +++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) create mode 100644 app/services/api/parser_struct.rb diff --git a/app/services/api/parser_struct.rb b/app/services/api/parser_struct.rb new file mode 100644 index 0000000000..407c78b727 --- /dev/null +++ b/app/services/api/parser_struct.rb @@ -0,0 +1,42 @@ +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) 2012-2022 the OpenProject GmbH +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See COPYRIGHT and LICENSE files for more details. +#++ + +module API + class ParserStruct < ::Hashie::Mash + ## + # TODO: Hashie::Mash extends from Hash and + # does not allow overriding any enumerable methods. + # + # This clashed with moving the queries services to BaseContracted, + # as we now use a +group_by+ attribute clashing with +Enumerable#group_by#. + # This redefines the method to ensure it works with queries, but does not solve the underlying issue. + def group_by + self[:group_by] + end + end +end From bcd8da890234eb3130e76377db298c0adf8e8fdd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20G=C3=BCnther?= Date: Mon, 28 Mar 2022 11:08:36 +0200 Subject: [PATCH 08/12] Fix ordered_work_packages now that we're base service compatible --- app/services/queries/create_service.rb | 15 +-------------- .../queries/set_attributes_service.rb | 19 +++++++++++++++++++ 2 files changed, 20 insertions(+), 14 deletions(-) diff --git a/app/services/queries/create_service.rb b/app/services/queries/create_service.rb index 3f0ffd5215..0d75b853ff 100644 --- a/app/services/queries/create_service.rb +++ b/app/services/queries/create_service.rb @@ -26,17 +26,4 @@ # See COPYRIGHT and LICENSE files for more details. #++ -class Queries::CreateService < ::BaseServices::Create - def after_validate(_params, call) - query = call.result - # Check which of the work package IDs exist - ids = query.ordered_work_packages.map(&:work_package_id) - existent_wps = WorkPackage.where(id: ids).pluck(:id).to_set - - query.ordered_work_packages = query.ordered_work_packages.select do |order_item| - existent_wps.include?(order_item.work_package_id) - end - - call - end -end +class Queries::CreateService < ::BaseServices::Create; end diff --git a/app/services/queries/set_attributes_service.rb b/app/services/queries/set_attributes_service.rb index 6488d5aed9..934e12bf53 100644 --- a/app/services/queries/set_attributes_service.rb +++ b/app/services/queries/set_attributes_service.rb @@ -27,6 +27,11 @@ #++ class Queries::SetAttributesService < ::BaseServices::SetAttributes + def set_attributes(params) + set_ordered_work_packages params.delete(:ordered_work_packages) + super + end + def set_default_attributes(_params) if model.include_subprojects.nil? model.include_subprojects = Setting.display_subprojects_work_packages? @@ -40,4 +45,18 @@ class Queries::SetAttributesService < ::BaseServices::SetAttributes model.user = user end end + + def set_ordered_work_packages(ordered_hash) + return if ordered_hash.nil? + + available = WorkPackage.where(id: ordered_hash.keys.map(&:to_s)).pluck(:id).to_set + + ordered_hash.each do |key, position| + # input keys are symbols due to hashie::mash, and AR doesn't like that + wp_id = key.to_s.to_i + next unless available.include?(wp_id.to_s.to_i) + + model.ordered_work_packages.build(work_package_id: wp_id, position: position) + end + end end From 8aefbc4a5d205dc62bac83f4ea3e914175608a44 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20G=C3=BCnther?= Date: Mon, 4 Apr 2022 09:00:36 +0200 Subject: [PATCH 09/12] Actually use the ParserStruct --- app/services/api/parse_resource_params_service.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/services/api/parse_resource_params_service.rb b/app/services/api/parse_resource_params_service.rb index d2b06598eb..685e29e420 100644 --- a/app/services/api/parse_resource_params_service.rb +++ b/app/services/api/parse_resource_params_service.rb @@ -76,7 +76,7 @@ module API end def struct - Hashie::Mash.new + ParserStruct.new end def deep_to_h(value) From cb8fac1f14ede18c94735511cea7702c0af1ca2b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20G=C3=BCnther?= Date: Mon, 4 Apr 2022 09:05:46 +0200 Subject: [PATCH 10/12] Add validation and test for include_subprojects --- app/models/query.rb | 3 +++ spec/models/query_spec.rb | 22 +++++++++++++++++++ .../exporter/csv_integration_spec.rb | 2 +- 3 files changed, 26 insertions(+), 1 deletion(-) diff --git a/app/models/query.rb b/app/models/query.rb index 82e91ccb6d..d298dc512a 100644 --- a/app/models/query.rb +++ b/app/models/query.rb @@ -45,6 +45,9 @@ class Query < ApplicationRecord presence: true, length: { maximum: 255 } + validates :include_subprojects, + inclusion: [true, false] + validate :validate_work_package_filters validate :validate_columns validate :validate_sort_criteria diff --git a/spec/models/query_spec.rb b/spec/models/query_spec.rb index 8690bb1a92..15604a5a2e 100644 --- a/spec/models/query_spec.rb +++ b/spec/models/query_spec.rb @@ -60,6 +60,28 @@ describe Query, type: :model do expect(query.sort_criteria) .to match_array([['id', 'asc']]) end + + context 'with global subprojects include', with_settings: { display_subprojects_work_packages: true } do + it 'sets the include subprojects' do + expect(query.include_subprojects).to eq true + end + end + + context 'with global subprojects include', with_settings: { display_subprojects_work_packages: false } do + it 'sets the include subprojects' do + expect(query.include_subprojects).to eq false + end + end + end + + describe 'include_subprojects' do + let(:query) { Query.new name: 'foo' } + + it 'is required' do + expect(query).not_to be_valid + + expect(query.errors[:include_subprojects]).to include 'is not set to one of the allowed values.' + end end describe 'hidden' do diff --git a/spec/models/work_package/exporter/csv_integration_spec.rb b/spec/models/work_package/exporter/csv_integration_spec.rb index 4055bc28eb..b121be559c 100644 --- a/spec/models/work_package/exporter/csv_integration_spec.rb +++ b/spec/models/work_package/exporter/csv_integration_spec.rb @@ -41,7 +41,7 @@ describe WorkPackage::Exports::CSV, 'integration', type: :model do member_with_permissions: %i(view_work_packages)) end let(:query) do - Query.new(name: '_').tap do |query| + Query.new_default(name: '_').tap do |query| query.column_names = %i(subject assigned_to updated_at estimated_hours) end end From dc978f9bedfbf40a3ab073df0e00bbdd35b0a02f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20G=C3=BCnther?= Date: Mon, 4 Apr 2022 09:18:47 +0200 Subject: [PATCH 11/12] Use scope.where in project filter --- .../queries/work_packages/filter/project_filter.rb | 9 ++------- .../queries/work_packages/filter/project_filter_spec.rb | 6 ++++++ 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/app/models/queries/work_packages/filter/project_filter.rb b/app/models/queries/work_packages/filter/project_filter.rb index dd7e88d9ee..3a4030a9c9 100644 --- a/app/models/queries/work_packages/filter/project_filter.rb +++ b/app/models/queries/work_packages/filter/project_filter.rb @@ -56,12 +56,7 @@ class Queries::WorkPackages::Filter::ProjectFilter < Queries::WorkPackages::Filt end def value_objects - available_projects = visible_projects.index_by(&:id) - - values - .flat_map { |project_id| available_projects[project_id.to_i] } - .compact - .uniq + visible_projects.where(id: values.map(&:to_i)) end def where @@ -71,7 +66,7 @@ class Queries::WorkPackages::Filter::ProjectFilter < Queries::WorkPackages::Filt private def visible_projects - @visible_projects ||= Project.visible.active + Project.visible.active end ## diff --git a/spec/models/queries/work_packages/filter/project_filter_spec.rb b/spec/models/queries/work_packages/filter/project_filter_spec.rb index d6c57c737e..b847bea81c 100644 --- a/spec/models/queries/work_packages/filter/project_filter_spec.rb +++ b/spec/models/queries/work_packages/filter/project_filter_spec.rb @@ -48,6 +48,12 @@ describe Queries::WorkPackages::Filter::ProjectFilter, type: :model do allow(visible_projects) .to receive(:exists?) .and_return(visible_projects.any?) + + allow(visible_projects) + .to receive(:where) do |args| + ids = args[:id] + visible_projects.select { |p| ids.include?(p.id) } + end end describe '#available?' do From 58b64686d835e7c073cf4b1396afbf1377d0853b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20G=C3=BCnther?= Date: Mon, 4 Apr 2022 09:30:21 +0200 Subject: [PATCH 12/12] Add create contract spec for queries --- .../contracts/queries/create_contract_spec.rb | 62 +++++++++++++++++++ .../queries/shared_contract_examples.rb | 52 ++++++++++++++++ .../contracts/queries/update_contract_spec.rb | 32 +++------- spec/models/query_spec.rb | 6 +- 4 files changed, 124 insertions(+), 28 deletions(-) create mode 100644 spec/contracts/queries/create_contract_spec.rb create mode 100644 spec/contracts/queries/shared_contract_examples.rb diff --git a/spec/contracts/queries/create_contract_spec.rb b/spec/contracts/queries/create_contract_spec.rb new file mode 100644 index 0000000000..df49504a04 --- /dev/null +++ b/spec/contracts/queries/create_contract_spec.rb @@ -0,0 +1,62 @@ +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) 2012-2022 the OpenProject GmbH +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See COPYRIGHT and LICENSE files for more details. +#++ + +require 'spec_helper' +require 'contracts/shared/model_contract_shared_context' +require_relative 'shared_contract_examples' + +describe Queries::CreateContract do + include_context 'ModelContract shared context' + include_context 'with queries contract' + + describe 'include subprojects' do + let(:query) do + Query.new name: 'foo', + include_subprojects: include_subprojects, + project: project + end + + context 'when true' do + let(:include_subprojects) { true } + + it_behaves_like 'contract is valid' + end + + context 'when falsea' do + let(:include_subprojects) { false } + + it_behaves_like 'contract is valid' + end + + context 'when nil' do + let(:include_subprojects) { nil } + + it_behaves_like 'contract is invalid', include_subprojects: %i[inclusion] + end + end +end diff --git a/spec/contracts/queries/shared_contract_examples.rb b/spec/contracts/queries/shared_contract_examples.rb new file mode 100644 index 0000000000..90d0d20938 --- /dev/null +++ b/spec/contracts/queries/shared_contract_examples.rb @@ -0,0 +1,52 @@ +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) 2012-2022 the OpenProject GmbH +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See COPYRIGHT and LICENSE files for more details. +#++ + +require 'spec_helper' +require 'contracts/shared/model_contract_shared_context' + +shared_context 'with queries contract' do + let(:project) { build_stubbed :project } + let(:query) do + build_stubbed(:query, project: project, public: public, user: user) + end + + let(:current_user) do + build_stubbed(:user) do |user| + allow(user) + .to receive(:allowed_to?) do |permission, permission_project| + permissions.include?(permission) && project == permission_project + end + end + end + let(:contract) { described_class.new(query, current_user) } + + before do + # Assume project is always visible + allow(contract).to receive(:project_visible?).and_return true + end +end diff --git a/spec/contracts/queries/update_contract_spec.rb b/spec/contracts/queries/update_contract_spec.rb index 37c2a8a34e..30735721b6 100644 --- a/spec/contracts/queries/update_contract_spec.rb +++ b/spec/contracts/queries/update_contract_spec.rb @@ -28,29 +28,11 @@ require 'spec_helper' require 'contracts/shared/model_contract_shared_context' +require_relative 'shared_contract_examples' describe Queries::UpdateContract do include_context 'ModelContract shared context' - - let(:project) { build_stubbed :project } - let(:query) do - build_stubbed(:query, project: project, public: public, user: user) - end - - let(:current_user) do - build_stubbed(:user) do |user| - allow(user) - .to receive(:allowed_to?) do |permission, permission_project| - permissions.include?(permission) && project == permission_project - end - end - end - let(:contract) { described_class.new(query, current_user) } - - before do - # Assume project is always visible - allow(contract).to receive(:project_visible?).and_return true - end + include_context 'with queries contract' describe 'private query' do let(:public) { false } @@ -58,13 +40,13 @@ describe Queries::UpdateContract do context 'when user is author' do let(:user) { current_user } - context 'user has no permission to save' do + context 'when user has no permission to save' do let(:permissions) { %i(edit_work_packages) } it_behaves_like 'contract user is unauthorized' end - context 'user has permission to save' do + context 'when user has permission to save' do let(:permissions) { %i(save_queries) } it_behaves_like 'contract is valid' @@ -83,19 +65,19 @@ describe Queries::UpdateContract do let(:public) { true } let(:user) { nil } - context 'user has no permission to save' do + context 'when user has no permission to save' do let(:permissions) { %i(invalid_permission) } it_behaves_like 'contract user is unauthorized' end - context 'user has no permission to manage public' do + context 'when user has no permission to manage public' do let(:permissions) { %i(manage_public_queries) } it_behaves_like 'contract is valid' end - context 'user has permission to save only own' do + context 'when user has permission to save only own' do let(:permissions) { %i(save_queries) } it_behaves_like 'contract user is unauthorized' diff --git a/spec/models/query_spec.rb b/spec/models/query_spec.rb index 15604a5a2e..c90fb1583a 100644 --- a/spec/models/query_spec.rb +++ b/spec/models/query_spec.rb @@ -63,19 +63,19 @@ describe Query, type: :model do context 'with global subprojects include', with_settings: { display_subprojects_work_packages: true } do it 'sets the include subprojects' do - expect(query.include_subprojects).to eq true + expect(query.include_subprojects).to be true end end context 'with global subprojects include', with_settings: { display_subprojects_work_packages: false } do it 'sets the include subprojects' do - expect(query.include_subprojects).to eq false + expect(query.include_subprojects).to be false end end end describe 'include_subprojects' do - let(:query) { Query.new name: 'foo' } + let(:query) { described_class.new name: 'foo' } it 'is required' do expect(query).not_to be_valid