From 79f7d4caec16d1e646998407e0fbf09c92f11227 Mon Sep 17 00:00:00 2001 From: Jens Ulferts Date: Tue, 29 May 2018 09:48:04 +0200 Subject: [PATCH 1/5] replace FactoryGirl by FactoryBot added by merging --- spec/features/auth/consent_auth_stage_spec.rb | 11 +++++----- .../v3/work_packages/dependent_errors_spec.rb | 20 +++++++++---------- 2 files changed, 16 insertions(+), 15 deletions(-) diff --git a/spec/features/auth/consent_auth_stage_spec.rb b/spec/features/auth/consent_auth_stage_spec.rb index 7f99e4d441..1ae424f12a 100644 --- a/spec/features/auth/consent_auth_stage_spec.rb +++ b/spec/features/auth/consent_auth_stage_spec.rb @@ -32,7 +32,7 @@ describe 'Authentication Stages', type: :feature, js: true do let(:language) { 'en' } let(:user_password) { 'bob' * 4 } let(:user) do - FactoryGirl.create( + FactoryBot.create( :user, admin: true, force_password_change: false, @@ -48,7 +48,9 @@ describe 'Authentication Stages', type: :feature, js: true do end before do - Setting.consent_required = consent_required + allow(Setting) + .to receive(:consent_required?) + .and_return(consent_required) end def expect_logged_in @@ -61,7 +63,6 @@ describe 'Authentication Stages', type: :feature, js: true do expect(page).to have_no_selector('.form--field-container', text: user.login) end - context 'when disabled' do let(:consent_required) { false } it 'should not show consent' do @@ -84,7 +85,7 @@ describe 'Authentication Stages', type: :feature, js: true do end end context 'when enabled, localized consent exists', - with_settings: { consent_info: { de: 'h1. Einwilligung', en: 'h1. Consent header!'} } do + with_settings: { consent_info: { de: 'h1. Einwilligung', en: 'h1. Consent header!' } } do let(:consent_required) { true } let(:language) { 'de' } @@ -96,7 +97,7 @@ describe 'Authentication Stages', type: :feature, js: true do end end - context 'when enabled, but consent exists', with_settings: { consent_info: { en: 'h1. Consent header!'} } do + context 'when enabled, but consent exists', with_settings: { consent_info: { en: 'h1. Consent header!' } } do let(:consent_required) { true } it 'should show consent' do expect(Setting.consent_time).to be_blank diff --git a/spec/requests/api/v3/work_packages/dependent_errors_spec.rb b/spec/requests/api/v3/work_packages/dependent_errors_spec.rb index 6c63f52efc..1a8a6cb6ec 100644 --- a/spec/requests/api/v3/work_packages/dependent_errors_spec.rb +++ b/spec/requests/api/v3/work_packages/dependent_errors_spec.rb @@ -35,7 +35,7 @@ describe 'API v3 Work package resource', type: :request, content_type: :json do include API::V3::Utilities::PathHelper let(:work_package) do - FactoryGirl.create( + FactoryBot.create( :work_package, project_id: project.id, parent: parent, @@ -44,7 +44,7 @@ describe 'API v3 Work package resource', type: :request, content_type: :json do end let!(:parent) do - FactoryGirl.create(:work_package, project_id: project.id, type: type, subject: "Invalid Dependent WorkPackage").tap do |parent| + FactoryBot.create(:work_package, project_id: project.id, type: type, subject: "Invalid Dependent WorkPackage").tap do |parent| parent.custom_values.create custom_field: custom_field, value: custom_field.possible_values.first.id cv = parent.custom_values.last @@ -53,21 +53,21 @@ describe 'API v3 Work package resource', type: :request, content_type: :json do end let(:project) do - FactoryGirl.create(:project, identifier: 'deperr', is_public: false).tap do |project| + FactoryBot.create(:project, identifier: 'deperr', is_public: false).tap do |project| project.types << type end end let(:type) do - FactoryGirl.create(:type).tap do |type| + FactoryBot.create(:type).tap do |type| type.custom_fields << custom_field end end - let(:status) { FactoryGirl.create :status } + let(:status) { FactoryBot.create :status } let(:custom_field) do - FactoryGirl.create( + FactoryBot.create( :list_wp_custom_field, name: "Gate", possible_values: %w(A B C), @@ -75,13 +75,13 @@ describe 'API v3 Work package resource', type: :request, content_type: :json do ) end - let(:role) { FactoryGirl.create(:role, permissions: permissions) } + let(:role) { FactoryBot.create(:role, permissions: permissions) } let(:permissions) { [:view_work_packages, :edit_work_packages, :create_work_packages] } let(:current_user) do - user = FactoryGirl.create(:user, member_in_project: project, member_through_role: role) + user = FactoryBot.create(:user, member_in_project: project, member_through_role: role) - FactoryGirl.create(:user_preference, user: user, others: { no_self_notified: false }) + FactoryBot.create(:user_preference, user: user, others: { no_self_notified: false }) user end @@ -148,7 +148,7 @@ describe 'API v3 Work package resource', type: :request, content_type: :json do end describe '#post' do - let(:current_user) { FactoryGirl.create :admin } + let(:current_user) { FactoryBot.create :admin } let(:path) { api_v3_paths.work_packages } let(:valid_params) do From b9394aca2fe1f0fa44415b0cc7b27f1caeb08b49 Mon Sep 17 00:00:00 2001 From: Jens Ulferts Date: Tue, 29 May 2018 09:52:22 +0200 Subject: [PATCH 2/5] retry notoriously flickering spec more often --- spec/features/work_packages/custom_actions_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/features/work_packages/custom_actions_spec.rb b/spec/features/work_packages/custom_actions_spec.rb index 120682d056..4baac1668f 100644 --- a/spec/features/work_packages/custom_actions_spec.rb +++ b/spec/features/work_packages/custom_actions_spec.rb @@ -138,7 +138,7 @@ describe 'Custom actions', type: :feature, js: true do login_as(admin) end - scenario 'viewing workflow buttons' do + scenario 'viewing workflow buttons', retry: 3 do # create custom action 'Unassign' index_ca_page.visit! From 3b8a520ff26937e9ce2e454416cfa3b06e84848d Mon Sep 17 00:00:00 2001 From: Jens Ulferts Date: Tue, 29 May 2018 10:07:29 +0200 Subject: [PATCH 3/5] avoid wrapping on wp payload Wrapping can interfere with a couple of method e.g. setting the errors --- .../work_package_payload_representer.rb | 10 ++++------ .../work_packages/work_package_representer.rb | 20 +++++++++++++------ 2 files changed, 18 insertions(+), 12 deletions(-) diff --git a/lib/api/v3/work_packages/work_package_payload_representer.rb b/lib/api/v3/work_packages/work_package_payload_representer.rb index 58be121d01..a970a65738 100644 --- a/lib/api/v3/work_packages/work_package_payload_representer.rb +++ b/lib/api/v3/work_packages/work_package_payload_representer.rb @@ -36,15 +36,13 @@ module API cached_representer disabled: true - def initialize(model, current_user:, embed_links: false) - model = ::API::V3::WorkPackages::EagerLoading::NeutralWrapper.wrap_one(model, current_user) - - super - end - def writeable_attributes super + ["date"] end + + def load_complete_model(model) + model + end end end end diff --git a/lib/api/v3/work_packages/work_package_representer.rb b/lib/api/v3/work_packages/work_package_representer.rb index a560639c25..399a559ebe 100644 --- a/lib/api/v3/work_packages/work_package_representer.rb +++ b/lib/api/v3/work_packages/work_package_representer.rb @@ -40,12 +40,7 @@ module API disabled: false def initialize(model, current_user:, embed_links: false) - # Define all accessors on the customizable as they - # will be used afterwards anyway. Otherwise, we will have to - # go through method_missing which will take more time. - model.define_all_custom_field_accessors - - model = ::API::V3::WorkPackages::WorkPackageEagerLoadingWrapper.wrap_one(model, current_user) + model = load_complete_model(model) super end @@ -532,6 +527,15 @@ module API 'WorkPackage' end + def to_hash(*args) + # Define all accessors on the customizable as they + # will be used afterwards anyway. Otherwise, we will have to + # go through method_missing which will take more time. + represented.define_all_custom_field_accessors + + super + end + def watchers # TODO/LEGACY: why do we need to ensure a specific order here? watchers = represented.watcher_users.order(User::USER_FORMATS_STRUCTURE[Setting.user_format]) @@ -639,6 +643,10 @@ module API def view_time_entries_allowed? current_user_allowed_to(:view_time_entries, context: represented.project) end + + def load_complete_model(model) + ::API::V3::WorkPackages::WorkPackageEagerLoadingWrapper.wrap_one(model, current_user) + end end end end From 9659285be35ab8ee903c376752c8a04c9fa524fe Mon Sep 17 00:00:00 2001 From: Inga Mai <39332861+ingamai@users.noreply.github.com> Date: Tue, 29 May 2018 11:03:55 +0200 Subject: [PATCH 4/5] AngularJS in 'mail-notifications.html.erb' replaced by Angular component (#6338) * AngularJS in 'mail-notifications.html.erb' replaced by new Angular component 'show-section-dropdown.component.ts' --- app/controllers/my_controller.rb | 2 + app/controllers/users_controller.rb | 2 + app/views/users/_mail_notifications.html.erb | 43 ++++++++----- frontend/app/angular4-modules.ts | 3 + .../show-section-dropdown.component.ts | 63 +++++++++++++++++++ spec/views/users/edit.html.erb_spec.rb | 1 + 6 files changed, 97 insertions(+), 17 deletions(-) create mode 100644 frontend/app/components/common/hide-section/show-section-dropdown.component.ts diff --git a/app/controllers/my_controller.rb b/app/controllers/my_controller.rb index a9c1e47fe7..57099628e4 100644 --- a/app/controllers/my_controller.rb +++ b/app/controllers/my_controller.rb @@ -32,6 +32,8 @@ class MyController < ApplicationController include Concerns::PasswordConfirmation layout 'my' + helper_method :gon + before_action :require_login before_action :check_password_confirmation, only: [:account], diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 5d0b002b9f..b9ef12b082 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -30,6 +30,8 @@ class UsersController < ApplicationController layout 'admin' + helper_method :gon + before_action :disable_api before_action :require_admin, except: [:show, :deletion_info, :destroy] before_action :find_user, only: [:show, diff --git a/app/views/users/_mail_notifications.html.erb b/app/views/users/_mail_notifications.html.erb index fcb33f32de..34c4b7ba69 100644 --- a/app/views/users/_mail_notifications.html.erb +++ b/app/views/users/_mail_notifications.html.erb @@ -27,35 +27,44 @@ See docs/COPYRIGHT.rdoc for more details. ++#%> +<% active_sections = if @user.mail_notification == 'selected' + [{key: 'notified_projects'}] + else + [] + end %> + +<%= initialize_hide_sections_with [{key: 'notified_projects'}], active_sections %> + +
<%= styled_label_tag "user_mail_notification", t(:'user.settings.mail_notifications') %>
- <%= styled_select_tag 'user[mail_notification]', + + <%= styled_select_tag 'user[mail_notification]', options_for_select(user_mail_notification_options(@user), @user.mail_notification), - :'ng-model' => 'mail_notifications', - :'ng-init' => "mail_notifications = '#{@user.mail_notification}'", container_class: '-wide' %> +
-<%= content_tag 'div', id: 'notified-projects', class: "ng-cloak", :'ng-if' => "mail_notifications === 'selected'" do %> -
-
- <% @user.projects.each do |project| %> - - <% end %> -
-
- <%= t(:'user.settings.mail_project_explanaition') %> -
+ +
+
+ <% @user.projects.each do |project| %> + + <% end %> +
+
+ <%= t(:'user.settings.mail_project_explanaition') %> +
-<% end %> +
<%= styled_label_tag 'self_notified', t(:'user.settings.mail_self_notified') %> diff --git a/frontend/app/angular4-modules.ts b/frontend/app/angular4-modules.ts index 168cba5f30..ec6d5833ac 100644 --- a/frontend/app/angular4-modules.ts +++ b/frontend/app/angular4-modules.ts @@ -87,6 +87,7 @@ import {HideSectionComponent} from 'core-components/common/hide-section/hide-sec import {HideSectionService} from 'core-components/common/hide-section/hide-section.service'; import {AddSectionDropdownComponent} from 'core-components/common/hide-section/add-section-dropdown/add-section-dropdown.component'; import {HideSectionLinkComponent} from 'core-components/common/hide-section/hide-section-link/hide-section-link.component'; +import {ShowSectionDropdownComponent} from 'core-components/common/hide-section/show-section-dropdown.component'; import {GonRef} from 'core-components/common/gon-ref/gon-ref'; import {AuthorisationService} from 'core-components/common/model-auth/model-auth.service'; import {WorkPackageTableFiltersService} from 'core-components/wp-fast-table/state/wp-table-filters.service'; @@ -368,6 +369,7 @@ import {WorkPackageRelationsCreateComponent} from "core-components/wp-relations/ HideSectionComponent, HideSectionLinkComponent, AddSectionDropdownComponent, + ShowSectionDropdownComponent, AutocompleteSelectDecorationComponent, // Split view @@ -489,6 +491,7 @@ import {WorkPackageRelationsCreateComponent} from "core-components/wp-relations/ HideSectionComponent, HideSectionLinkComponent, AddSectionDropdownComponent, + ShowSectionDropdownComponent, AutocompleteSelectDecorationComponent, // Split view diff --git a/frontend/app/components/common/hide-section/show-section-dropdown.component.ts b/frontend/app/components/common/hide-section/show-section-dropdown.component.ts new file mode 100644 index 0000000000..60688660c4 --- /dev/null +++ b/frontend/app/components/common/hide-section/show-section-dropdown.component.ts @@ -0,0 +1,63 @@ +//-- copyright +// OpenProject is a project management system. +// Copyright (C) 2012-2018 the OpenProject Foundation (OPF) +// +// 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-2017 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 docs/COPYRIGHT.rdoc for more details. +//++ + +import {Component, OnInit, ElementRef, Input} from '@angular/core'; +import {HideSectionService} from 'core-components/common/hide-section/hide-section.service'; +import {opUiComponentsModule} from "core-app/angular-modules"; +import {downgradeComponent} from '@angular/upgrade/static'; + +@Component({ + selector: 'show-section-dropdown', + template: '' +}) + +export class ShowSectionDropdownComponent implements OnInit { + @Input() optValue:string; // value of option for which hide-section should be visible + @Input() hideSecWithName:string; // section-name of hide-section + + constructor(protected hideSections:HideSectionService, + private elementRef:ElementRef) { + } + + ngOnInit() { + jQuery(this.elementRef.nativeElement).change(event => { + let selectedOption = jQuery("option:selected", event.target); + + if (selectedOption.val() !== this.optValue) { + this.hideSections.hide(this.hideSecWithName); + } + else { + this.hideSections.show({key: this.hideSecWithName, label: ""}); + } + }); + } +} + +opUiComponentsModule.directive('showSectionDropdown', + downgradeComponent({component: ShowSectionDropdownComponent}) +); diff --git a/spec/views/users/edit.html.erb_spec.rb b/spec/views/users/edit.html.erb_spec.rb index 1c91f45613..3c8af87110 100644 --- a/spec/views/users/edit.html.erb_spec.rb +++ b/spec/views/users/edit.html.erb_spec.rb @@ -35,6 +35,7 @@ describe 'users/edit', type: :view do # The url_for is missing the users id that is usually taken # from request parameters controller.request.path_parameters[:id] = user.id + view.extend(Gon::ControllerHelpers) end context 'authentication provider' do From a56019930a4e5fc8ece1d3d1dc015cb1d472e281 Mon Sep 17 00:00:00 2001 From: Wieland Lindenthal Date: Tue, 29 May 2018 17:57:39 +0200 Subject: [PATCH 5/5] fix eager loading for wiki pages tree in main menu --- app/views/wiki/_menu_pages_tree.html.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/wiki/_menu_pages_tree.html.erb b/app/views/wiki/_menu_pages_tree.html.erb index 0e511d75c7..5b599e2e78 100644 --- a/app/views/wiki/_menu_pages_tree.html.erb +++ b/app/views/wiki/_menu_pages_tree.html.erb @@ -1,7 +1,7 @@
<% cache @project.wiki do %>
<%= t('label_table_of_contents') %>
- <% pages = @project.wiki.pages.order('title').includes(wiki: :project) + <% pages = @project.wiki.pages.order('title').includes(:project) pages_by_parent_id = pages.group_by(&:parent_id) %> <%= render_page_hierarchy(pages_by_parent_id, nil, timestamp: false) %> <% end %>