From 3ae7090878d49af1c07315e708f31e769193fc9a Mon Sep 17 00:00:00 2001 From: Wieland Lindenthal Date: Tue, 21 Feb 2017 14:33:09 +0100 Subject: [PATCH 01/10] Devide the long form for custom fields into tabs. --- app/controllers/types_controller.rb | 17 +++-- app/views/types/edit.html.erb | 31 +++++++++- .../_form_configuration.html.erb} | 46 -------------- app/views/types/form/_projects.html.erb | 62 +++++++++++++++++++ app/views/types/form/_settings.html.erb | 57 +++++++++++++++++ app/views/types/new.html.erb | 6 +- config/routes.rb | 14 ++--- 7 files changed, 171 insertions(+), 62 deletions(-) rename app/views/types/{_form.html.erb => form/_form_configuration.html.erb} (67%) create mode 100644 app/views/types/form/_projects.html.erb create mode 100644 app/views/types/form/_settings.html.erb diff --git a/app/controllers/types_controller.rb b/app/controllers/types_controller.rb index 038dc019f1..e5f5e50f3b 100644 --- a/app/controllers/types_controller.rb +++ b/app/controllers/types_controller.rb @@ -69,13 +69,19 @@ class TypesController < ApplicationController end def edit - @projects = Project.all - @type = ::Type.includes(:projects, - :custom_fields) - .find(params[:id]) + if params[:tab].blank? + redirect_to tab: :settings + else + @tab = params[:tab] + @projects = Project.all + @type = ::Type.includes(:projects, + :custom_fields) + .find(params[:id]) + end end def update + @tab = params["tab"] || "settings" @type = ::Type.find(params[:id]) # forbid renaming if it is a standard type @@ -85,7 +91,8 @@ class TypesController < ApplicationController result = service.call(attributes: permitted_params.type) if result.success? - redirect_to edit_type_path(id: @type.id), notice: t(:notice_successful_update) + redirect_to(edit_type_tab_path(id: @type.id, tab: @tab), + notice: t(:notice_successful_update)) else @projects = Project.all render action: 'edit' diff --git a/app/views/types/edit.html.erb b/app/views/types/edit.html.erb index e43e1339fd..a896001bbf 100644 --- a/app/views/types/edit.html.erb +++ b/app/views/types/edit.html.erb @@ -36,6 +36,33 @@ See doc/COPYRIGHT.rdoc for more details. <%= breadcrumb_toolbar @type.name %> -<%= form_for @type, builder: TabularFormBuilder, lang: current_language do |f| %> - <%= render partial: 'form', locals: { f: f } %> +
+ +
+ +<%= javascript_include_tag "type_form.js" %> + +<%= error_messages_for 'type' %> + +<%= form_for @type, + url: update_type_tab_path( id: @type.id, tab: @tab ), + builder: TabularFormBuilder, + lang: current_language do |f| %> + <%= render partial: "types/form/#{@tab}", locals: { f: f } %> <% end %> diff --git a/app/views/types/_form.html.erb b/app/views/types/form/_form_configuration.html.erb similarity index 67% rename from app/views/types/_form.html.erb rename to app/views/types/form/_form_configuration.html.erb index b0bfac2c4f..a02f8d3509 100644 --- a/app/views/types/_form.html.erb +++ b/app/views/types/form/_form_configuration.html.erb @@ -26,31 +26,7 @@ Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. See doc/COPYRIGHT.rdoc for more details. ++#%> -<%= javascript_include_tag "type_form.js" %> -<%= error_messages_for 'type' %> - -
-
-
- -
<%= f.text_field :name, required: true, disabled: @type.is_standard? %>
-
<%= f.check_box :is_in_roadmap %>
-
<%= f.select :color_id, options_for_colors(@type) %>
-
<%= f.check_box :in_aggregation %>
-
<%= f.check_box :is_default, label: t(:label_type_default_new_projects) %>
-
<%= f.check_box :is_milestone %>
- - <% if @type.new_record? && @types.any? %> -
- <%= styled_label_tag 'copy_workflow_from', t(:label_copy_workflow_from) %> - <%= styled_select_tag(:copy_workflow_from, content_tag("option") + options_from_collection_for_select(@types, :id, :name)) %> -
- <% end %> - -
-
-
@@ -113,28 +89,6 @@ See doc/COPYRIGHT.rdoc for more details.
-
- <% if @projects.any? %> -
- - <%= t(:label_project_plural) %> - -
-
- - (<%= check_all_links 'type_project_ids' %>) - -
- <%= project_nested_ul(@projects) do |p| - content_tag('label', - check_box_tag('type[project_ids][]', p.id, @type.projects.include?(p), id: nil) + - ' ' + h(p), class: 'form--label-with-check-box') - end %> - <%= hidden_field_tag('type[project_ids][]', '', id: nil) %> -
-
- <% end %> -
diff --git a/app/views/types/form/_projects.html.erb b/app/views/types/form/_projects.html.erb new file mode 100644 index 0000000000..e6504e7566 --- /dev/null +++ b/app/views/types/form/_projects.html.erb @@ -0,0 +1,62 @@ +<%#-- copyright +OpenProject is a project management system. +Copyright (C) 2012-2017 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 doc/COPYRIGHT.rdoc for more details. + +++#%> + +
+
+
+ <% if @projects.any? %> +
+ + <%= t(:label_project_plural) %> + +
+
+ + (<%= check_all_links 'type_project_ids' %>) + +
+ <%= project_nested_ul(@projects) do |p| + content_tag('label', + check_box_tag('type[project_ids][]', p.id, @type.projects.include?(p), id: nil) + + ' ' + h(p), class: 'form--label-with-check-box') + end %> + <%= hidden_field_tag('type[project_ids][]', '', id: nil) %> +
+
+ <% end %> +
+
+
+ +
+
+ <%= styled_button_tag t(@type.new_record? ? :button_create : :button_save), + class: '-highlight -with-icon icon-checkmark' %> +
+
diff --git a/app/views/types/form/_settings.html.erb b/app/views/types/form/_settings.html.erb new file mode 100644 index 0000000000..7b8e2a383e --- /dev/null +++ b/app/views/types/form/_settings.html.erb @@ -0,0 +1,57 @@ +<%#-- copyright +OpenProject is a project management system. +Copyright (C) 2012-2017 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 doc/COPYRIGHT.rdoc for more details. + +++#%> + +
+
+
+ +
<%= f.text_field :name, required: true, disabled: @type.is_standard? %>
+
<%= f.check_box :is_in_roadmap %>
+
<%= f.select :color_id, options_for_colors(@type) %>
+
<%= f.check_box :in_aggregation %>
+
<%= f.check_box :is_default, label: t(:label_type_default_new_projects) %>
+
<%= f.check_box :is_milestone %>
+ + <% if @type.new_record? && @types.any? %> +
+ <%= styled_label_tag 'copy_workflow_from', t(:label_copy_workflow_from) %> + <%= styled_select_tag(:copy_workflow_from, content_tag("option") + options_from_collection_for_select(@types, :id, :name)) %> +
+ <% end %> + +
+
+
+ +
+
+ <%= styled_button_tag t(@type.new_record? ? :button_create : :button_save), + class: '-highlight -with-icon icon-checkmark' %> +
+
diff --git a/app/views/types/new.html.erb b/app/views/types/new.html.erb index 86da9bd804..e6947f5e11 100644 --- a/app/views/types/new.html.erb +++ b/app/views/types/new.html.erb @@ -33,6 +33,10 @@ See doc/COPYRIGHT.rdoc for more details. <% local_assigns[:additional_breadcrumb] = t(:label_type_new) %> <%= breadcrumb_toolbar t(:label_type_new) %> +<%= javascript_include_tag "type_form.js" %> + +<%= error_messages_for 'type' %> + <%= form_for controller.type, builder: TabularFormBuilder, lang: current_language do |f| %> - <%= render partial: 'form', locals: { f: f } %> + <%= render partial: 'types/form/settings', locals: { f: f } %> <% end %> diff --git a/config/routes.rb b/config/routes.rb index 31ff6b0acf..d7d9d769b4 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -149,6 +149,11 @@ OpenProject::Application.routes.draw do get '/roles/workflow/:id/:role_id/:type_id' => 'roles#workflow' get '/help/:ctrl/:page' => 'help#index' + get '/types/:id/edit/:tab' => "types#edit", + as: "edit_type_tab" + match '/types/:id/update/:tab' => "types#update", + as: "update_type_tab", + via: [:post, :patch] resources :types do post 'move/:id', action: 'move', on: :collection end @@ -163,14 +168,7 @@ OpenProject::Application.routes.draw do as: 'custom_style_logo', constraints: { filename: /[^\/]*/ } - resources :custom_fields, except: :show do - member do - match "options/:option_id", - to: "custom_fields#delete_option", - via: :delete, - as: :delete_option_of - end - end + resources :custom_fields, except: :show get '(projects/:project_id)/search' => 'search#index', as: 'search' # only providing routes for journals when there are multiple subclasses of journals From 4875141b386280776fe2a7a4038b83571c0600e2 Mon Sep 17 00:00:00 2001 From: Wieland Lindenthal Date: Tue, 21 Feb 2017 14:42:38 +0100 Subject: [PATCH 02/10] i18n for tab labels --- app/views/types/edit.html.erb | 6 +++--- config/locales/en.yml | 4 ++++ 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/app/views/types/edit.html.erb b/app/views/types/edit.html.erb index a896001bbf..aa690c24cd 100644 --- a/app/views/types/edit.html.erb +++ b/app/views/types/edit.html.erb @@ -39,17 +39,17 @@ See doc/COPYRIGHT.rdoc for more details.
  • - <%= link_to "Settings", + <%= link_to t("types.edit.settings"), edit_type_tab_path( id: @type.id, tab: :settings ), class: "#{'selected' if @tab == "settings" }" %>
  • - <%= link_to "Form configuration", + <%= link_to t("types.edit.form_configuration"), edit_type_tab_path( id: @type.id, tab: :form_configuration ), class: "#{'selected' if @tab == 'form_configuration' }" %>
  • - <%= link_to "Projects", + <%= link_to t("types.edit.projects"), edit_type_tab_path( id: @type.id, tab: :projects ), class: "#{'selected' if @tab == 'projects' }" %>
    • diff --git a/config/locales/en.yml b/config/locales/en.yml index e2f98695d8..4cc6e13e3b 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -156,6 +156,10 @@ en: index: no_results_title_text: There are currently no types. no_results_content_text: Create a new type + edit: + settings: "Settings" + form_configuration: "Form configuration" + projects: "Projects" versions: overview: From 3e346f3e7ef0fddff33f8459015ecc5d873b5fa4 Mon Sep 17 00:00:00 2001 From: Wieland Lindenthal Date: Tue, 21 Feb 2017 15:25:31 +0100 Subject: [PATCH 03/10] improve codeclimate first --- spec/controllers/types_controller_spec.rb | 82 +++++++++++++---------- 1 file changed, 47 insertions(+), 35 deletions(-) diff --git a/spec/controllers/types_controller_spec.rb b/spec/controllers/types_controller_spec.rb index bed8a492a1..ab4a91806a 100644 --- a/spec/controllers/types_controller_spec.rb +++ b/spec/controllers/types_controller_spec.rb @@ -29,15 +29,15 @@ require 'spec_helper' describe TypesController, type: :controller do - let(:project) { + let(:project) do FactoryGirl.create(:project, work_package_custom_fields: [custom_field_2]) - } - let(:custom_field_1) { + end + let(:custom_field_1) do FactoryGirl.create(:work_package_custom_field, field_format: 'string', is_for_all: true) - } + end let(:custom_field_2) { FactoryGirl.create(:work_package_custom_field) } let(:status_0) { FactoryGirl.create(:status) } let(:status_1) { FactoryGirl.create(:status) } @@ -51,7 +51,7 @@ describe TypesController, type: :controller do describe 'GET index' do describe 'the access should be restricted' do - before do get 'index' end + before { get 'index' } it { expect(response.status).to eq(403) } end @@ -59,7 +59,7 @@ describe TypesController, type: :controller do describe 'GET new' do describe 'the access should be restricted' do - before do get 'new' end + before { get 'new' } it { expect(response.status).to eq(403) } end @@ -67,7 +67,7 @@ describe TypesController, type: :controller do describe 'GET edit' do describe 'the access should be restricted' do - before do get 'edit' end + before { get 'edit' } it { expect(response.status).to eq(403) } end @@ -75,7 +75,7 @@ describe TypesController, type: :controller do describe 'POST create' do describe 'the access should be restricted' do - before do post 'create' end + before { post 'create' } it { expect(response.status).to eq(403) } end @@ -83,7 +83,7 @@ describe TypesController, type: :controller do describe 'DELETE destroy' do describe 'the access should be restricted' do - before do delete 'destroy' end + before { delete 'destroy' } it { expect(response.status).to eq(403) } end @@ -91,7 +91,7 @@ describe TypesController, type: :controller do describe 'POST update' do describe 'the access should be restricted' do - before do post 'update' end + before { post 'update' } it { expect(response.status).to eq(403) } end @@ -99,7 +99,7 @@ describe TypesController, type: :controller do describe 'POST move' do describe 'the access should be restricted' do - before do post 'move' end + before { post 'move' } it { expect(response.status).to eq(403) } end @@ -114,24 +114,25 @@ describe TypesController, type: :controller do end describe 'GET index' do - before do get 'index' end + before { get 'index' } it { expect(response).to be_success } it { expect(response).to render_template 'index' } end describe 'GET new' do - before do get 'new' end + before { get 'new' } it { expect(response).to be_success } it { expect(response).to render_template 'new' } end describe 'POST create' do describe 'WITH valid params' do - let(:params) { + let(:params) do { 'type' => { name: 'New type', project_ids: { '1' => project.id }, - custom_field_ids: { '1' => custom_field_1.id, '2' => custom_field_2.id } - } } } + custom_field_ids: { '1' => custom_field_1.id, + '2' => custom_field_2.id } } } + end before do post :create, params: params @@ -143,11 +144,12 @@ describe TypesController, type: :controller do describe 'WITH an empty name' do render_views - let(:params) { + let(:params) do { 'type' => { name: '', project_ids: { '1' => project.id }, - custom_field_ids: { '1' => custom_field_1.id, '2' => custom_field_2.id } - } } } + custom_field_ids: { '1' => custom_field_1.id, + '2' => custom_field_2.id } } } + end before do post :create, params: params @@ -161,18 +163,19 @@ describe TypesController, type: :controller do describe 'WITH workflow copy' do let!(:existing_type) { FactoryGirl.create(:type, name: 'Existing type') } - let!(:workflow) { + let!(:workflow) do FactoryGirl.create(:workflow, old_status: status_0, new_status: status_1, type_id: existing_type.id) - } - let(:params) { + end + + let(:params) do { 'type' => { name: 'New type', project_ids: { '1' => project.id }, custom_field_ids: { '1' => custom_field_1.id, '2' => custom_field_2.id } }, - 'copy_workflow_from' => existing_type.id - } } + 'copy_workflow_from' => existing_type.id } + end before do post :create, params: params @@ -181,14 +184,19 @@ describe TypesController, type: :controller do it { expect(response).to be_redirect } it { expect(response).to redirect_to(types_path) } it 'should have the copied workflows' do - expect(::Type.find_by(name: 'New type').workflows.count).to eq(existing_type.workflows.count) + expect(::Type.find_by(name: 'New type') + .workflows.count).to eq(existing_type.workflows.count) end end end describe 'GET edit' do render_views - let(:type) { FactoryGirl.create(:type, name: 'My type', is_milestone: true, projects: [project]) } + let(:type) do + FactoryGirl.create(:type, name: 'My type', + is_milestone: true, + projects: [project]) + end before do get 'edit', params: { id: type.id } @@ -203,10 +211,16 @@ describe TypesController, type: :controller do describe 'POST update' do let(:project2) { FactoryGirl.create(:project) } - let(:type) { FactoryGirl.create(:type, name: 'My type', is_milestone: true, projects: [project, project2]) } + let(:type) do + FactoryGirl.create(:type, name: 'My type', + is_milestone: true, + projects: [project, project2]) + end describe 'WITH type rename' do - let(:params) { { 'id' => type.id, 'type' => { name: 'My type renamed' } } } + let(:params) do + { 'id' => type.id, 'type' => { name: 'My type renamed' } } + end before do put :update, params: params @@ -273,17 +287,17 @@ describe TypesController, type: :controller do end describe 'detroy type in use should fail' do - let(:project2) { + let(:project2) do FactoryGirl.create(:project, work_package_custom_fields: [custom_field_2], types: [type2]) - } - let!(:work_package) { + end + let!(:work_package) do FactoryGirl.create(:work_package, author: current_user, type: type2, project: project2) - } + end let(:params) { { 'id' => type2.id } } before do @@ -303,9 +317,7 @@ describe TypesController, type: :controller do describe 'destroy standard type should fail' do let(:params) { { 'id' => type3.id } } - before do - delete :destroy, params: params - end + before { delete :destroy, params: params } it { expect(response).to be_redirect } it { expect(response).to redirect_to(types_path) } From e21dd87e298e7b3d6e4f51223b1e3845ce76c890 Mon Sep 17 00:00:00 2001 From: Wieland Lindenthal Date: Tue, 21 Feb 2017 15:45:07 +0100 Subject: [PATCH 04/10] Fix type_controller_spec.rb --- spec/controllers/types_controller_spec.rb | 46 +++++++++++++++++++---- 1 file changed, 39 insertions(+), 7 deletions(-) diff --git a/spec/controllers/types_controller_spec.rb b/spec/controllers/types_controller_spec.rb index ab4a91806a..46820d6998 100644 --- a/spec/controllers/types_controller_spec.rb +++ b/spec/controllers/types_controller_spec.rb @@ -190,7 +190,7 @@ describe TypesController, type: :controller do end end - describe 'GET edit' do + describe 'GET edit settings' do render_views let(:type) do FactoryGirl.create(:type, name: 'My type', @@ -199,16 +199,34 @@ describe TypesController, type: :controller do end before do - get 'edit', params: { id: type.id } + get 'edit', params: { id: type.id, tab: :settings } end it { expect(response).to be_success } it { expect(response).to render_template 'edit' } + it { expect(response).to render_template 'types/form/_settings' } it { expect(response.body).to have_selector "input[@name='type[name]'][@value='My type']" } - it { expect(response.body).to have_selector "input[@name='type[project_ids][]'][@value='#{project.id}'][@checked='checked']" } it { expect(response.body).to have_selector "input[@name='type[is_milestone]'][@value='1'][@checked='checked']" } end + describe 'GET edit projects' do + render_views + let(:type) do + FactoryGirl.create(:type, name: 'My type', + is_milestone: true, + projects: [project]) + end + + before do + get 'edit', params: { id: type.id, tab: :projects } + end + + it { expect(response).to be_success } + it { expect(response).to render_template 'edit' } + it { expect(response).to render_template 'types/form/_projects' } + it { expect(response.body).to have_selector "input[@name='type[project_ids][]'][@value='#{project.id}'][@checked='checked']" } + end + describe 'POST update' do let(:project2) { FactoryGirl.create(:project) } let(:type) do @@ -219,7 +237,9 @@ describe TypesController, type: :controller do describe 'WITH type rename' do let(:params) do - { 'id' => type.id, 'type' => { name: 'My type renamed' } } + { 'id' => type.id, + 'type' => { name: 'My type renamed' }, + 'tab' => "settings" } end before do @@ -227,21 +247,33 @@ describe TypesController, type: :controller do end it { expect(response).to be_redirect } - it { expect(response).to redirect_to(edit_type_path(id: type.id)) } + it do + expect(response).to( + redirect_to(edit_type_tab_path(id: type.id, tab: "settings")) + ) + end it 'should be renamed' do expect(::Type.find_by(name: 'My type renamed').id).to eq(type.id) end end describe 'WITH projects removed' do - let(:params) { { 'id' => type.id, 'type' => { project_ids: [''] } } } + let(:params) do + { 'id' => type.id, + 'type' => { project_ids: [''] }, + 'tab' => "projects" } + end before do put :update, params: params end it { expect(response).to be_redirect } - it { expect(response).to redirect_to(edit_type_path(id: type.id)) } + it do + expect(response).to( + redirect_to(edit_type_tab_path(id: type.id, tab: :projects)) + ) + end it 'should have no projects assigned' do expect(::Type.find_by(name: 'My type').projects.count).to eq(0) end From 4a805fbef8f70fd0631f29a10241c616b5b88c3b Mon Sep 17 00:00:00 2001 From: Wieland Lindenthal Date: Tue, 21 Feb 2017 15:59:54 +0100 Subject: [PATCH 05/10] Update path; i18n l to t, code climate --- app/views/types/index.html.erb | 54 +++++++++++++++++++++++++--------- 1 file changed, 40 insertions(+), 14 deletions(-) diff --git a/app/views/types/index.html.erb b/app/views/types/index.html.erb index ea6b2d54ef..1965c1b944 100644 --- a/app/views/types/index.html.erb +++ b/app/views/types/index.html.erb @@ -27,15 +27,17 @@ See doc/COPYRIGHT.rdoc for more details. ++#%> -<% html_title l(:label_administration), l("label_type_plural") %> -<%= toolbar title: l(:label_type_plural) do %> +<% html_title t(:label_administration), t("label_type_plural") %> +<%= toolbar title: t(:label_type_plural) do %>
  • <%= link_to new_type_path, { class: 'button -alt-highlight', - aria: {label: t(:label_type_new)}, + aria: { label: t(:label_type_new) }, title: t(:label_type_new)} do %> - <%= t('activerecord.attributes.workpackage.type') %> + + <%= t('activerecord.attributes.workpackage.type') %> + <% end %>
    • <% end %> @@ -85,7 +87,8 @@ See doc/COPYRIGHT.rdoc for more details.
- +
@@ -107,7 +110,7 @@ See doc/COPYRIGHT.rdoc for more details.
- <%=l(:button_sort)%> + <%=t(:button_sort)%>
@@ -118,16 +121,39 @@ See doc/COPYRIGHT.rdoc for more details. <% for type in @types %> - <%= link_to type.name, edit_type_path(type) %> - <% if type.workflows.empty? %><%= l(:text_type_no_workflow) %> (<%= link_to l(:button_edit), {controller: '/workflows', action: 'edit', type_id: type} %>)<% end %> - <%= icon_for_type type %> - <%= checked_image(type.is_default) %> - <%= checked_image(type.in_aggregation) %> - <%= checked_image(type.is_milestone) %> - <%= reorder_links('type', {action: 'move', id: type}) %> + + <%= link_to type.name, edit_type_tab_path(type, tab: "settings") %> + + + <% if type.workflows.empty? %> + + <%= t(:text_type_no_workflow) %> + (<%= link_to t(:button_edit), { controller: '/workflows', + action: 'edit', + type_id: type} %>) + + <% end %> + + + <%= icon_for_type type %> + + + <%= checked_image(type.is_default) %> + + + <%= checked_image(type.in_aggregation) %> + + + <%= checked_image(type.is_milestone) %> + + + <%= reorder_links('type', {action: 'move', id: type}) %> + <% if !type.is_standard? %> - <%= link_to type, method: :delete, data: { confirm: t(:text_are_you_sure) }, class: 'icon icon-delete' do %> + <%= link_to type, method: :delete, + data: { confirm: t(:text_are_you_sure) }, + class: 'icon icon-delete' do %> <%= t(:button_delete) %> <%=h type.name %> <% end %> From 0acba977665a067878e019c685b9bc2b451e9783 Mon Sep 17 00:00:00 2001 From: Wieland Lindenthal Date: Tue, 21 Feb 2017 18:21:02 +0100 Subject: [PATCH 06/10] smaller stuff: labels, layout and code beauty --- app/views/types/edit.html.erb | 10 +- .../types/form/_form_configuration.html.erb | 107 +++++++++--------- app/views/types/form/_projects.html.erb | 29 ++--- app/views/types/form/_settings.html.erb | 33 +++--- app/views/types/index.html.erb | 2 +- 5 files changed, 85 insertions(+), 96 deletions(-) diff --git a/app/views/types/edit.html.erb b/app/views/types/edit.html.erb index aa690c24cd..7f353f30d3 100644 --- a/app/views/types/edit.html.erb +++ b/app/views/types/edit.html.erb @@ -27,7 +27,7 @@ See doc/COPYRIGHT.rdoc for more details. ++#%> -<% html_title l(:label_administration), "#{l(:label_edit)} #{Type.model_name.human} #{h @type.name}" %> +<% html_title t(:label_administration), "#{t(:label_edit)} #{t(:label_work_package_types)} #{h @type.name}" %> <% local_assigns[:additional_breadcrumb] = @type.name %> @@ -40,17 +40,17 @@ See doc/COPYRIGHT.rdoc for more details.
  • <%= link_to t("types.edit.settings"), - edit_type_tab_path( id: @type.id, tab: :settings ), + edit_type_tab_path(id: @type.id, tab: :settings), class: "#{'selected' if @tab == "settings" }" %>
  • <%= link_to t("types.edit.form_configuration"), - edit_type_tab_path( id: @type.id, tab: :form_configuration ), + edit_type_tab_path(id: @type.id, tab: :form_configuration), class: "#{'selected' if @tab == 'form_configuration' }" %>
  • <%= link_to t("types.edit.projects"), - edit_type_tab_path( id: @type.id, tab: :projects ), + edit_type_tab_path(id: @type.id, tab: :projects), class: "#{'selected' if @tab == 'projects' }" %>
@@ -61,7 +61,7 @@ See doc/COPYRIGHT.rdoc for more details. <%= error_messages_for 'type' %> <%= form_for @type, - url: update_type_tab_path( id: @type.id, tab: @tab ), + url: update_type_tab_path(id: @type.id, tab: @tab), builder: TabularFormBuilder, lang: current_language do |f| %> <%= render partial: "types/form/#{@tab}", locals: { f: f } %> diff --git a/app/views/types/form/_form_configuration.html.erb b/app/views/types/form/_form_configuration.html.erb index a02f8d3509..1fcd805bd2 100644 --- a/app/views/types/form/_form_configuration.html.erb +++ b/app/views/types/form/_form_configuration.html.erb @@ -30,64 +30,59 @@ See doc/COPYRIGHT.rdoc for more details.
-
- - <%= I18n.t('label_form_configuration')%> - -
-

<%= I18n.t('text_form_configuration') %>

- - +
+

<%= I18n.t('text_form_configuration') %>

+
+ + + + + + + + + <% + attributes = ::TypesHelper + .work_package_form_attributes(merge_date: true) + .reject { |name, attr| + # display all custom fields don't display required fields without a default + not name =~ /custom_field_/ and (attr[:required] and not attr[:has_default]) + } + keys = attributes.keys.sort_by do |name| + translated_attribute_name(name, attributes[name]) + end + %> + <% keys.each do |name| %> + <% attr = attributes[name] %> - - - + + + - - - <% - attributes = ::TypesHelper - .work_package_form_attributes(merge_date: true) - .reject { |name, attr| - # display all custom fields don't display required fields without a default - not name =~ /custom_field_/ and (attr[:required] and not attr[:has_default]) - } - keys = attributes.keys.sort_by do |name| - translated_attribute_name(name, attributes[name]) - end - %> - <% keys.each do |name| %> - <% attr = attributes[name] %> - - - - - - <% end %> - -
<%= I18n.t('label_attribute') %><%= I18n.t('label_active') %><%= I18n.t('label_always_visible') %>
<%= I18n.t('label_attribute') %><%= I18n.t('label_active') %><%= I18n.t('label_always_visible') %> + <%= label_tag "type_attribute_visibility_#{name}", + translated_attribute_name(name, attr), + value: "type_attribute_visibility[#{name}]", + class: 'ellipsis' %> + + " type="hidden" value="hidden" /> + <% active_checked = [nil, 'default', 'visible'].include?(attr_visibility(name, @type)) %> + <%= check_box_tag "type[attribute_visibility][#{name}]", + 'default', + active_checked, + id: "type_attribute_visibility_default_#{name}", + title: I18n.t('tooltip.attribute_visibility.default') %> + + <%= check_box_tag "type[attribute_visibility][#{name}]", + 'visible', + attr_visibility(name, @type) == 'visible', + id: "type_attribute_visibility_visible_#{name}", + title: I18n.t('tooltip.attribute_visibility.visible'), + disabled: !active_checked %> +
- <%= label_tag "type_attribute_visibility_#{name}", - translated_attribute_name(name, attr), - value: "type_attribute_visibility[#{name}]", - class: 'ellipsis' %> - - " type="hidden" value="hidden" /> - <% active_checked = [nil, 'default', 'visible'].include?(attr_visibility(name, @type)) %> - <%= check_box_tag "type[attribute_visibility][#{name}]", - 'default', - active_checked, - id: "type_attribute_visibility_default_#{name}", - title: I18n.t('tooltip.attribute_visibility.default') %> - - <%= check_box_tag "type[attribute_visibility][#{name}]", - 'visible', - attr_visibility(name, @type) == 'visible', - id: "type_attribute_visibility_visible_#{name}", - title: I18n.t('tooltip.attribute_visibility.visible'), - disabled: !active_checked %> -
-
-
+ <% end %> + + +
diff --git a/app/views/types/form/_projects.html.erb b/app/views/types/form/_projects.html.erb index e6504e7566..7ad714ff79 100644 --- a/app/views/types/form/_projects.html.erb +++ b/app/views/types/form/_projects.html.erb @@ -31,24 +31,19 @@ See doc/COPYRIGHT.rdoc for more details.
<% if @projects.any? %> -
- - <%= t(:label_project_plural) %> - -
-
- - (<%= check_all_links 'type_project_ids' %>) - -
- <%= project_nested_ul(@projects) do |p| - content_tag('label', - check_box_tag('type[project_ids][]', p.id, @type.projects.include?(p), id: nil) + - ' ' + h(p), class: 'form--label-with-check-box') - end %> - <%= hidden_field_tag('type[project_ids][]', '', id: nil) %> +
+
+ + (<%= check_all_links 'type_project_ids' %>) +
-
+ <%= project_nested_ul(@projects) do |p| + content_tag('label', + check_box_tag('type[project_ids][]', p.id, @type.projects.include?(p), id: nil) + + ' ' + h(p), class: 'form--label-with-check-box') + end %> + <%= hidden_field_tag('type[project_ids][]', '', id: nil) %> +
<% end %>
diff --git a/app/views/types/form/_settings.html.erb b/app/views/types/form/_settings.html.erb index 7b8e2a383e..372445b43e 100644 --- a/app/views/types/form/_settings.html.erb +++ b/app/views/types/form/_settings.html.erb @@ -29,23 +29,22 @@ See doc/COPYRIGHT.rdoc for more details.
-
- -
<%= f.text_field :name, required: true, disabled: @type.is_standard? %>
-
<%= f.check_box :is_in_roadmap %>
-
<%= f.select :color_id, options_for_colors(@type) %>
-
<%= f.check_box :in_aggregation %>
-
<%= f.check_box :is_default, label: t(:label_type_default_new_projects) %>
-
<%= f.check_box :is_milestone %>
- - <% if @type.new_record? && @types.any? %> -
- <%= styled_label_tag 'copy_workflow_from', t(:label_copy_workflow_from) %> - <%= styled_select_tag(:copy_workflow_from, content_tag("option") + options_from_collection_for_select(@types, :id, :name)) %> -
- <% end %> - -
+ +
<%= f.text_field :name, required: true, disabled: @type.is_standard? %>
+
<%= f.check_box :is_in_roadmap %>
+
<%= f.select :color_id, options_for_colors(@type) %>
+
<%= f.check_box :in_aggregation %>
+
<%= f.check_box :is_default, label: t(:label_type_default_new_projects) %>
+
<%= f.check_box :is_milestone %>
+ + <% if @type.new_record? && @types.any? %> +
+ <%= styled_label_tag 'copy_workflow_from', t(:label_copy_workflow_from) %> + <%= styled_select_tag(:copy_workflow_from, content_tag("option") + options_from_collection_for_select(@types, :id, :name)) %> +
+ <% end %> + +
diff --git a/app/views/types/index.html.erb b/app/views/types/index.html.erb index 1965c1b944..3f11895771 100644 --- a/app/views/types/index.html.erb +++ b/app/views/types/index.html.erb @@ -28,7 +28,7 @@ See doc/COPYRIGHT.rdoc for more details. ++#%> <% html_title t(:label_administration), t("label_type_plural") %> -<%= toolbar title: t(:label_type_plural) do %> +<%= toolbar title: t(:label_work_package_types) do %>
  • <%= link_to new_type_path, { class: 'button -alt-highlight', From 48a126bf5cc1ef67129168d40b61660d87cb1d16 Mon Sep 17 00:00:00 2001 From: Wieland Lindenthal Date: Wed, 22 Feb 2017 11:00:28 +0100 Subject: [PATCH 07/10] Fix: I somehow deleted a few routes by accident --- config/routes.rb | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/config/routes.rb b/config/routes.rb index d7d9d769b4..8d41c93514 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -168,7 +168,15 @@ OpenProject::Application.routes.draw do as: 'custom_style_logo', constraints: { filename: /[^\/]*/ } - resources :custom_fields, except: :show + resources :custom_fields, except: :show do + member do + match "options/:option_id", + to: "custom_fields#delete_option", + via: :delete, + as: :delete_option_of + end + end + get '(projects/:project_id)/search' => 'search#index', as: 'search' # only providing routes for journals when there are multiple subclasses of journals From a333d6799d66926f7996df6f59096988d9d258dd Mon Sep 17 00:00:00 2001 From: Wieland Lindenthal Date: Wed, 22 Feb 2017 14:28:50 +0100 Subject: [PATCH 08/10] Fix legecy spec --- .../functional/types_controller_spec.rb | 31 +++++++++++-------- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/spec_legacy/functional/types_controller_spec.rb b/spec_legacy/functional/types_controller_spec.rb index 3995ce581d..649f23f9ad 100644 --- a/spec_legacy/functional/types_controller_spec.rb +++ b/spec_legacy/functional/types_controller_spec.rb @@ -26,7 +26,7 @@ # # See doc/COPYRIGHT.rdoc for more details. #++ -require 'legacy_spec_helper' +require_relative '../legacy_spec_helper' require 'types_controller' describe TypesController, type: :controller do @@ -78,33 +78,38 @@ describe TypesController, type: :controller do it 'should get edit' do ::Type.find(1).project_ids = [1, 3] - get :edit, id: 1 + get :edit, id: 1, tab: 'settings' assert_response :success assert_template 'edit' + assert_template 'types/form/_settings' assert_select 'input', attributes: { name: 'type[project_ids][]', - value: '1', - checked: 'checked' } + value: '1', + checked: 'checked' } assert_select 'input', attributes: { name: 'type[project_ids][]', - value: '2', - checked: nil } + value: '2', + checked: nil } assert_select 'input', attributes: { name: 'type[project_ids][]', - value: '', - type: 'hidden' } + value: '', + type: 'hidden' } end - it 'should post update' do - post :update, id: 1, type: { name: 'Renamed', - project_ids: ['1', '2', ''] } + it 'should post update name' do + post :update, id: 1, tab: "settings", type: { name: 'Renamed' } + assert_equal "Renamed", ::Type.find(1).name + assert_redirected_to action: 'edit' + end + + it 'should post update projects' do + post :update, id: 1, tab: "projects", type: { project_ids: ['1', '2', ''] } assert_redirected_to action: 'edit' assert_equal [1, 2], ::Type.find(1).project_ids.sort end it 'should post update without projects' do - post :update, id: 1, type: { name: 'Renamed', - project_ids: [''] } + post :update, id: 1, tab: "projects", type: { project_ids: [''] } assert_redirected_to action: 'edit' assert ::Type.find(1).project_ids.empty? end From 2ed63ff446cf17a1c7b2247723b58e95531b9b21 Mon Sep 17 00:00:00 2001 From: Wieland Lindenthal Date: Wed, 22 Feb 2017 15:39:54 +0100 Subject: [PATCH 09/10] readd "check all / uncheck all" --- app/views/types/form/_projects.html.erb | 4 ++++ config/locales/en.yml | 1 + 2 files changed, 5 insertions(+) diff --git a/app/views/types/form/_projects.html.erb b/app/views/types/form/_projects.html.erb index 7ad714ff79..d7906a8cd0 100644 --- a/app/views/types/form/_projects.html.erb +++ b/app/views/types/form/_projects.html.erb @@ -31,6 +31,10 @@ See doc/COPYRIGHT.rdoc for more details.
    <% if @projects.any? %> +
    + + <%= t('types.edit.enabled_projects') %> +
    diff --git a/config/locales/en.yml b/config/locales/en.yml index 4cc6e13e3b..ccdf23cc37 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -160,6 +160,7 @@ en: settings: "Settings" form_configuration: "Form configuration" projects: "Projects" + enabled_projects: "Enabled projects" versions: overview: From ebea10a4cec2ba17224436a20b650363fc7dcbc5 Mon Sep 17 00:00:00 2001 From: Wieland Lindenthal Date: Wed, 22 Feb 2017 16:59:16 +0100 Subject: [PATCH 10/10] desparate try to fix this spec --- spec/features/types/form_configuration_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/features/types/form_configuration_spec.rb b/spec/features/types/form_configuration_spec.rb index 9d53193cb8..b7cadb25b9 100644 --- a/spec/features/types/form_configuration_spec.rb +++ b/spec/features/types/form_configuration_spec.rb @@ -49,7 +49,7 @@ describe 'form configuration', type: :feature, js: true do before do allow(User).to receive(:current).and_return current_user - visit edit_type_path(id: type.id) + visit edit_type_tab_path(id: type.id, tab: "form_configuration") end shared_examples 'attribute visibility' do