From 14819c93b897fb2eb5e11fc4b2f52bd6921b86d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20G=C3=BCnther?= Date: Fri, 5 Feb 2016 13:29:40 +0100 Subject: [PATCH 01/30] Release opf/openproject v5.0.14 --- Gemfile.lock | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index ef8ef7b47e..b4134f56c6 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -45,10 +45,10 @@ GIT GIT remote: https://github.com/opf/openproject-translations.git - revision: 9241f0940eb5ae2d3a38da904a5abb7298d73a47 + revision: 704e7fdc4917bb259dcf9ce1d0b560318a62b58b branch: release/5.0 specs: - openproject-translations (5.0.14) + openproject-translations (5.0.15) crowdin-api (~> 0.4.0) mixlib-shellout (~> 2.1.0) rails (~> 4.2.3) From 02e5a1a588e327830fe882e523b7de1a064b97e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20G=C3=BCnther?= Date: Fri, 5 Feb 2016 13:29:40 +0100 Subject: [PATCH 02/30] Bump VERSION to 5.0.15 --- lib/open_project/version.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/open_project/version.rb b/lib/open_project/version.rb index 9dbf865a5a..d6f11bb0da 100644 --- a/lib/open_project/version.rb +++ b/lib/open_project/version.rb @@ -34,7 +34,7 @@ module OpenProject module VERSION #:nodoc: MAJOR = 5 MINOR = 0 - PATCH = 14 + PATCH = 15 TINY = PATCH # Redmine compat # Used by semver to define the special version (if any). From 291308492858155ad9952cb7d2734cfe6e9726d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20G=C3=BCnther?= Date: Fri, 5 Feb 2016 13:59:08 +0100 Subject: [PATCH 03/30] Remove temporary files before upgrading Related to: https://community.openproject.org/topics/5753?r=5851 https://community.openproject.org/work_packages/22278 [ci skip] --- doc/operation_guides/manual/upgrade-guide.md | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/doc/operation_guides/manual/upgrade-guide.md b/doc/operation_guides/manual/upgrade-guide.md index c77516fe47..f897aa080a 100644 --- a/doc/operation_guides/manual/upgrade-guide.md +++ b/doc/operation_guides/manual/upgrade-guide.md @@ -162,7 +162,17 @@ As a reference, the following Node.js and NPM versions have been installed on ou ## The Upgrade -Now that the sources and dependencies are in place, you can migrate the Database and do the upgrade: +Now that the sources and dependencies are in place, you can migrate the Database and do the upgrade. + +Before actually migrating the database, please remove all temporary files from the previous installation (caches, sessions) by running the following command. + +```bash +[openproject@debian]# cd /home/openproject/openproject +[openproject@debian]# RAILS_ENV="production" bundle exec rake tmp:clear +``` + +If you do not clear the temporary files, you may encounter an error of the form `NoMethodError: undefined method `map' for #` in the `config/initializers/30-patches.rb` files. +The actual upgrade commands are as follows: ```bash [openproject@debian]# cd /home/openproject/openproject From dff1282811d8e4fc9da8d8a0a50befa52804de10 Mon Sep 17 00:00:00 2001 From: Henriette Dinger Date: Fri, 5 Feb 2016 15:39:39 +0100 Subject: [PATCH 04/30] Set required star to the classes suggested by the styleguide --- app/assets/stylesheets/content/_forms.lsg | 4 ++-- app/views/custom_fields/_form.html.erb | 3 ++- app/views/projects/form/attributes/_name.html.erb | 2 +- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/app/assets/stylesheets/content/_forms.lsg b/app/assets/stylesheets/content/_forms.lsg index f994952072..9808fc7857 100644 --- a/app/assets/stylesheets/content/_forms.lsg +++ b/app/assets/stylesheets/content/_forms.lsg @@ -110,8 +110,8 @@ Write anything you like. -
- +
+
diff --git a/app/views/custom_fields/_form.html.erb b/app/views/custom_fields/_form.html.erb index 212f657daf..1f4f33c256 100644 --- a/app/views/custom_fields/_form.html.erb +++ b/app/views/custom_fields/_form.html.erb @@ -31,7 +31,8 @@ See doc/COPYRIGHT.rdoc for more details.
<%= f.text_field :name, - multi_locale: true %> + multi_locale: true, + required: true %>
<%= f.select :field_format, diff --git a/app/views/projects/form/attributes/_name.html.erb b/app/views/projects/form/attributes/_name.html.erb index 35e463f458..fc81757994 100644 --- a/app/views/projects/form/attributes/_name.html.erb +++ b/app/views/projects/form/attributes/_name.html.erb @@ -28,5 +28,5 @@ See doc/COPYRIGHT.rdoc for more details. ++#%>
- <%= form.text_field :name %> + <%= form.text_field :name, required: true %>
From b765c5a1b62c5e1acca3d4dcff18b15cfe5424c7 Mon Sep 17 00:00:00 2001 From: Henriette Dinger Date: Fri, 5 Feb 2016 16:52:41 +0100 Subject: [PATCH 05/30] Change structure of watcher_link so that it is accessible --- app/helpers/watchers_helper.rb | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/app/helpers/watchers_helper.rb b/app/helpers/watchers_helper.rb index 944c05b3b6..821887e9f7 100644 --- a/app/helpers/watchers_helper.rb +++ b/app/helpers/watchers_helper.rb @@ -43,7 +43,7 @@ def watcher_link(object, user, options = { replace: '.watcher_link', class: 'wat path = send(:"#{(watched ? 'unwatch' : 'watch')}_path", object_type: object.class.to_s.underscore.pluralize, object_id: object.id, replace: options.delete('replace')) - html_options[:class] = html_options[:class].to_s + (watched ? ' icon-context icon-watched' : ' icon-context icon-unwatched') + html_options[:class] = html_options[:class].to_s + ' button' method = watched ? :delete : @@ -53,9 +53,11 @@ def watcher_link(object, user, options = { replace: '.watcher_link', class: 'wat l(:button_unwatch) : l(:button_watch) - content_tag :div, class: 'button' do - link_to(label, path, html_options.merge(remote: true, method: method)) - end + link_to(content_tag(:i,'', class: watched ? 'button--icon icon-watched' : ' button--icon icon-unwatched') + + content_tag(:span, label, class: 'button--text'), path, html_options.merge(remote: true, method: method)) + + + end # Returns HTML for a list of users watching the given object From 28249857d5629482aac85fef570eec57d89a0c41 Mon Sep 17 00:00:00 2001 From: CI Date: Sat, 6 Feb 2016 00:37:12 +0000 Subject: [PATCH 06/30] Update reference to OpenProject-Translations --- Gemfile.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gemfile.lock b/Gemfile.lock index b4134f56c6..d02738283a 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -45,7 +45,7 @@ GIT GIT remote: https://github.com/opf/openproject-translations.git - revision: 704e7fdc4917bb259dcf9ce1d0b560318a62b58b + revision: 40350b081b87feccd57b55f4d35eab74ddcb9911 branch: release/5.0 specs: openproject-translations (5.0.15) From a11ff39606ebcf26b99512289643b7d4b778c80b Mon Sep 17 00:00:00 2001 From: CI Date: Sun, 7 Feb 2016 00:37:19 +0000 Subject: [PATCH 07/30] Update reference to OpenProject-Translations --- Gemfile.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gemfile.lock b/Gemfile.lock index d02738283a..2027714d35 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -45,7 +45,7 @@ GIT GIT remote: https://github.com/opf/openproject-translations.git - revision: 40350b081b87feccd57b55f4d35eab74ddcb9911 + revision: 089407397e309588a0a575f175f2665ffa1d2316 branch: release/5.0 specs: openproject-translations (5.0.15) From 5523b2b07b6be2028f1545a67e80640eabbc2918 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20G=C3=BCnther?= Date: Mon, 8 Feb 2016 11:44:28 +0100 Subject: [PATCH 08/30] Preload unicorn for 2+ workers Reduces startup of Unicorn and initial request performance when hitting a freshly started worker for `worker_processes >= 2`. Preloads the application using Copy-on-Write before forking to worker processes. This requires us to explicitly close and re-open the ActiveRecord connection before Unicorn is forking its slaves. We do not need this for Dalli (cf., https://github.com/petergoldstein/dalli#features-and-changes). It may become necessary when using other external services that connect through sockets on startup. Required for some products with 2FA enabled that cause requests back on the unicorn servers, thus recommend a default worker size of 2. Preloading the application will greatly improve the initial request. [ci skip] --- config/unicorn.rb | 36 ++++++++++++++++++++++++++++++++++-- 1 file changed, 34 insertions(+), 2 deletions(-) diff --git a/config/unicorn.rb b/config/unicorn.rb index c19d75f267..ff82e3fb59 100644 --- a/config/unicorn.rb +++ b/config/unicorn.rb @@ -26,6 +26,38 @@ # See doc/COPYRIGHT.rdoc for more details. #++ -worker_processes Integer(ENV['WEB_CONCURRENCY'] || 1) +worker_processes Integer(ENV['WEB_CONCURRENCY'] || 2) timeout Integer(ENV['WEB_TIMEOUT'] || 15) -preload_app false +preload_app true + +# Preloading the unicorn server to have all workers spawn the application +# automatically. +# +# Borrows heavily from https://www.digitalocean.com/community/tutorials/how-to-optimize-unicorn-workers-in-a-ruby-on-rails-app +# +# This method requires ActiveRecord to close and re-establish its connection in the slaves, +# because the connection is not properly shared with them. +# +# If you use any other service, you'll need to add them to these _fork blocks to close +# and reopen sockets when forking. +# (except Dalli/Memcache store, which detects automatically) +before_fork do |_server, _worker| + Signal.trap 'TERM' do + puts 'Unicorn master intercepted SIGTERM. Re-Sending as SIGQUIT for slaves.' + Process.kill 'QUIT', Process.pid + end + + if defined?(ActiveRecord::Base) + ActiveRecord::Base.connection.disconnect! + end +end + +after_fork do |_server, _worker| + Signal.trap 'TERM' do + puts 'Waiting for SIGQUIT from Unicorn master.' + end + + if defined?(ActiveRecord::Base) + ActiveRecord::Base.establish_connection + end +end From d99929bbcb8bdc3e631b5ad87d1f420056bffc0b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20G=C3=BCnther?= Date: Mon, 8 Feb 2016 13:18:24 +0100 Subject: [PATCH 09/30] Mention updated paths --- doc/operation_guides/manual/upgrade-guide.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/doc/operation_guides/manual/upgrade-guide.md b/doc/operation_guides/manual/upgrade-guide.md index f897aa080a..3432e20d8c 100644 --- a/doc/operation_guides/manual/upgrade-guide.md +++ b/doc/operation_guides/manual/upgrade-guide.md @@ -9,6 +9,10 @@ If you haven't configured serving repositories through Apache before, you'll fin For the other steps necessary to upgrade to OpenProject 5.0 please look at the sections below and exchange `v4.1.0` with `v5.0.0`. +## Changed Rails Path + +OpenProject 5.0 employs Rails 4.2.x, which contains a number of changes regarding paths. Foremost, files previously located in the `scripts` directory now reside in `bin` (e.g., `delayed_job`). + ## Upgrading to Managed Repositories You can create repositories explicitly on the filesystem using managed repositories. From 8b0cd01d9c5ec490d5318226a55b00ec87a2ac7d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20G=C3=BCnther?= Date: Fri, 5 Feb 2016 10:33:30 +0100 Subject: [PATCH 10/30] Use back_url for bulk move, copy, and edit This sets the back_url to wherever the bulk request originated from, allowing users to be redirected to their old work package, query, or table. Note that this does not simply work for bulk destroy, since users may use that functionality from split/full view and will in this case be redirected to a non-existing resource. We might pass a query_id in this case (to at least redirect back to queries), however it would be better if the frontend decided what path the user should be redirected to in this case. --- app/controllers/work_packages/moves_controller.rb | 2 +- app/views/work_packages/bulk/edit.html.erb | 1 + app/views/work_packages/moves/new.html.erb | 1 + 3 files changed, 3 insertions(+), 1 deletion(-) diff --git a/app/controllers/work_packages/moves_controller.rb b/app/controllers/work_packages/moves_controller.rb index 460c67a470..89b30aecc4 100644 --- a/app/controllers/work_packages/moves_controller.rb +++ b/app/controllers/work_packages/moves_controller.rb @@ -78,7 +78,7 @@ def create redirect_to project_work_packages_path(@target_project || @project) end else - redirect_to project_work_packages_path(@project) + redirect_back_or_default(project_work_packages_path(@project)) end end diff --git a/app/views/work_packages/bulk/edit.html.erb b/app/views/work_packages/bulk/edit.html.erb index bcb7be413c..02c49982b7 100644 --- a/app/views/work_packages/bulk/edit.html.erb +++ b/app/views/work_packages/bulk/edit.html.erb @@ -31,6 +31,7 @@ See doc/COPYRIGHT.rdoc for more details. <%= styled_form_tag(url_for(controller: '/work_packages/bulk', action: :update), method: :put, class: '-wide-labels') do %> <%= @work_packages.collect {|i| hidden_field_tag('ids[]', i.id)}.join.html_safe %> + <%= back_url_hidden_field_tag %>
<%= l(:label_change_properties) %> diff --git a/app/views/work_packages/moves/new.html.erb b/app/views/work_packages/moves/new.html.erb index a8b0e1b065..73c0320381 100644 --- a/app/views/work_packages/moves/new.html.erb +++ b/app/views/work_packages/moves/new.html.erb @@ -35,6 +35,7 @@ See doc/COPYRIGHT.rdoc for more details. <%= styled_form_tag({action: 'create'}, id: 'move_form', class: '-wide-labels') do %> <%= @work_packages.collect {|i| hidden_field_tag('ids[]', i.id)}.join.html_safe %> + <%= back_url_hidden_field_tag %>
<%= l(:label_change_properties) %> From 07f34004692a8dc55bf0d6ca2ef9eea9dc505a99 Mon Sep 17 00:00:00 2001 From: Henriette Dinger Date: Fri, 8 Jan 2016 11:33:07 +0100 Subject: [PATCH 11/30] Estimate folder action in repo for specialized icons --- app/helpers/repositories_helper.rb | 32 +++++++++++++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/app/helpers/repositories_helper.rb b/app/helpers/repositories_helper.rb index 74528305c6..30d166f983 100644 --- a/app/helpers/repositories_helper.rb +++ b/app/helpers/repositories_helper.rb @@ -91,6 +91,26 @@ def render_changeset_changes render_changes_tree(tree[:s]) end + # This calculates whether a folder was added, deleted or modified. It is based on the assumption that + # a folder was added/deleted when all content was added/deleted since the folder changes were not tracked. + def calculate_folder_action(tree) + folderAction = '' + unified_actions_in_same_iteration = true + tree.keys.sort.each do |changedFile| + if c = tree[changedFile][:c] + if c.action == 'A' && unified_actions_in_same_iteration && folderAction != 'D' + folderAction = 'A' + elsif c.action == 'D' && unified_actions_in_same_iteration && folderAction != 'A' + folderAction = 'D' + else + folderAction = '' + unified_actions_in_same_iteration = false + end + end + end + return folderAction + end + def render_changes_tree(tree) return '' if tree.nil? @@ -108,7 +128,17 @@ def render_changes_tree(tree) project_id: @project, path: path_param, rev: @changeset.identifier) - output << "
  • #{text}
    • " + + folderAction = calculate_folder_action(s) + case folderAction + when 'A' + output << "
  • #{text}
    • " + when 'D' + output << "
  • #{text}
    • " + else + output << "
  • #{text}
    • " + end + output << render_changes_tree(s) elsif c = tree[file][:c] style << " change-#{c.action}" From 8c277bac84f08cc7efaf95f6ac54bd71ae47a63a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20G=C3=BCnther?= Date: Mon, 8 Feb 2016 15:45:20 +0100 Subject: [PATCH 12/30] Use folder mapping and Set for identifying action --- app/helpers/repositories_helper.rb | 38 ++++++++++++------------------ 1 file changed, 15 insertions(+), 23 deletions(-) diff --git a/app/helpers/repositories_helper.rb b/app/helpers/repositories_helper.rb index 30d166f983..e584f8819e 100644 --- a/app/helpers/repositories_helper.rb +++ b/app/helpers/repositories_helper.rb @@ -91,24 +91,25 @@ def render_changeset_changes render_changes_tree(tree[:s]) end + # Mapping from internal action to (folder|file)-icon type + def change_action_mapping + { + 'A' => :add, + 'B' => :remove + } + end + # This calculates whether a folder was added, deleted or modified. It is based on the assumption that # a folder was added/deleted when all content was added/deleted since the folder changes were not tracked. def calculate_folder_action(tree) - folderAction = '' - unified_actions_in_same_iteration = true - tree.keys.sort.each do |changedFile| - if c = tree[changedFile][:c] - if c.action == 'A' && unified_actions_in_same_iteration && folderAction != 'D' - folderAction = 'A' - elsif c.action == 'D' && unified_actions_in_same_iteration && folderAction != 'A' - folderAction = 'D' - else - folderAction = '' - unified_actions_in_same_iteration = false - end + seen = Set.new + tree.each do |_, entry| + if folderStyle = change_action_mapping[entry[:c].try(:action)] + seen << folderStyle end end - return folderAction + + seen.size == 1 ? seen.first : :open end def render_changes_tree(tree) @@ -129,16 +130,7 @@ def render_changes_tree(tree) path: path_param, rev: @changeset.identifier) - folderAction = calculate_folder_action(s) - case folderAction - when 'A' - output << "
  • #{text}
    • " - when 'D' - output << "
  • #{text}
    • " - else - output << "
  • #{text}
    • " - end - + output << "
  • #{text}
    • " output << render_changes_tree(s) elsif c = tree[file][:c] style << " change-#{c.action}" From 57a48963e1068b1f2c19cc4e21d0000995ce3260 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20G=C3=BCnther?= Date: Mon, 8 Feb 2016 15:46:16 +0100 Subject: [PATCH 13/30] Remove unnecessary html_safe's --- lib/redmine/unified_diff.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/redmine/unified_diff.rb b/lib/redmine/unified_diff.rb index 8105b94f40..2430217891 100644 --- a/lib/redmine/unified_diff.rb +++ b/lib/redmine/unified_diff.rb @@ -236,7 +236,7 @@ def line def html_line_left if offsets - line_left.dup.insert(offsets.first, '').insert(offsets.last, '').html_safe + line_left.dup.insert(offsets.first, '').insert(offsets.last, '') else line_left end @@ -244,7 +244,7 @@ def html_line_left def html_line_right if offsets - line_right.dup.insert(offsets.first, '').insert(offsets.last, '').html_safe + line_right.dup.insert(offsets.first, '').insert(offsets.last, '') else line_right end @@ -252,7 +252,7 @@ def html_line_right def html_line if offsets - line.dup.insert(offsets.first, '').insert(offsets.last, '').html_safe + line.dup.insert(offsets.first, '').insert(offsets.last, '') else line end From ac56f8a3fc47d51d058167434553ad013455fe2c Mon Sep 17 00:00:00 2001 From: Henriette Dinger Date: Tue, 9 Feb 2016 08:37:50 +0100 Subject: [PATCH 14/30] Change tabbing order to avoid conflicts with backgorund formular --- app/views/account/_login.html.erb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/views/account/_login.html.erb b/app/views/account/_login.html.erb index 530dabc50d..15453c5db4 100644 --- a/app/views/account/_login.html.erb +++ b/app/views/account/_login.html.erb @@ -51,7 +51,7 @@ See doc/COPYRIGHT.rdoc for more details.
    <%= styled_label_tag 'password-pulldown', User.human_attribute_name(:password) %>
    - <%= styled_password_field_tag 'password', nil, id: 'password-pulldown', tabindex: 2 %> + <%= styled_password_field_tag 'password', nil, id: 'password-pulldown', tabindex: 1 %>
    <% if Setting.lost_password? %> @@ -70,7 +70,7 @@ See doc/COPYRIGHT.rdoc for more details.   + value="<%=l(:button_login)%>" class="button -highlight" tabindex="1" />
    From dcc031dbcf11ee549380aaf27c3dadceb217ccab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20G=C3=BCnther?= Date: Tue, 9 Feb 2016 09:10:52 +0100 Subject: [PATCH 15/30] Pass project to custom field context in bulk mode List custom fields such as `user` uses the project to identify potential options for the custom field selection. In bulk mode, that context was not passed to the cf helper. --- app/helpers/custom_fields_helper.rb | 4 ++-- app/models/custom_field.rb | 22 +++++++++++++------ app/views/work_packages/bulk/edit.html.erb | 2 +- .../work_packages/bulk_controller_spec.rb | 7 +++++- 4 files changed, 24 insertions(+), 11 deletions(-) diff --git a/app/helpers/custom_fields_helper.rb b/app/helpers/custom_fields_helper.rb index 6c0843800b..5372efccbf 100644 --- a/app/helpers/custom_fields_helper.rb +++ b/app/helpers/custom_fields_helper.rb @@ -111,7 +111,7 @@ def custom_field_tag_with_label(name, custom_value) custom_field_label_tag(name, custom_value) + custom_field_tag(name, custom_value) end - def custom_field_tag_for_bulk_edit(name, custom_field) + def custom_field_tag_for_bulk_edit(name, custom_field, project=nil) field_name = "#{name}[custom_field_values][#{custom_field.id}]" field_id = "#{name}_custom_field_values_#{custom_field.id}" field_format = Redmine::CustomFieldFormat.find_by_name(custom_field.field_format) @@ -126,7 +126,7 @@ def custom_field_tag_for_bulk_edit(name, custom_field) [l(:general_text_yes), '1'], [l(:general_text_no), '0']]), id: field_id) when 'list' - styled_select_tag(field_name, options_for_select([[l(:label_no_change_option), '']] + custom_field.possible_values_options), id: field_id) + styled_select_tag(field_name, options_for_select([[l(:label_no_change_option), '']] + custom_field.possible_values_options(project)), id: field_id) else styled_text_field_tag(field_name, '', id: field_id) end diff --git a/app/models/custom_field.rb b/app/models/custom_field.rb index 9004a7b4be..3bbed79526 100644 --- a/app/models/custom_field.rb +++ b/app/models/custom_field.rb @@ -118,13 +118,10 @@ def validate_default_value_in_translations def possible_values_options(obj = nil) case field_format when 'user', 'version' - if obj.respond_to?(:project) && obj.project - case field_format - when 'user' - obj.project.users.sort.map { |u| [u.to_s, u.id.to_s] } - when 'version' - obj.project.versions.sort.map { |u| [u.to_s, u.id.to_s] } - end + if obj.is_a?(Project) + possible_values_options_in_project(obj) + elsif obj.try(:project) + possible_values_options_in_project(obj.project) else [] end @@ -135,6 +132,17 @@ def possible_values_options(obj = nil) end end + def possible_values_options_in_project(project) + case field_format + when 'user' + project.users.sort.map { |u| [u.to_s, u.id.to_s] } + when 'version' + project.versions.sort.map { |u| [u.to_s, u.id.to_s] } + else + [] + end + end + ## # Returns possible values for this custom field. # Options may be a customizable, or options suitable for ActiveRecord#read_attribute. diff --git a/app/views/work_packages/bulk/edit.html.erb b/app/views/work_packages/bulk/edit.html.erb index 02c49982b7..2ad97085fb 100644 --- a/app/views/work_packages/bulk/edit.html.erb +++ b/app/views/work_packages/bulk/edit.html.erb @@ -98,7 +98,7 @@ See doc/COPYRIGHT.rdoc for more details.
    <%= blank_custom_field_label_tag('work_package', custom_field) %>
    - <%= custom_field_tag_for_bulk_edit('work_package', custom_field) %> + <%= custom_field_tag_for_bulk_edit('work_package', custom_field, @project) %>
    <% end %> diff --git a/spec/controllers/work_packages/bulk_controller_spec.rb b/spec/controllers/work_packages/bulk_controller_spec.rb index 1fbde504d6..4d5e1bdbf8 100644 --- a/spec/controllers/work_packages/bulk_controller_spec.rb +++ b/spec/controllers/work_packages/bulk_controller_spec.rb @@ -38,10 +38,11 @@ is_for_all: true) } let(:custom_field_2) { FactoryGirl.create(:work_package_custom_field) } + let(:custom_field_user) { FactoryGirl.create(:user_issue_custom_field) } let(:status) { FactoryGirl.create(:status) } let(:type) { FactoryGirl.create(:type_standard, - custom_fields: [custom_field_1, custom_field_2]) + custom_fields: [custom_field_1, custom_field_2, custom_field_user]) } let(:project_1) { FactoryGirl.create(:project, @@ -146,6 +147,10 @@ describe '#project' do it { assert_select 'select', attributes: { name: "work_package[custom_field_values][#{custom_field_2.id}]" } } end + + describe '#user' do + it { assert_select 'select', attributes: { name: "work_package[custom_field_values][#{custom_field_user.id}]" } } + end end end end From 531e9a20aedf98edbef8357ea5b8d036f9445fa6 Mon Sep 17 00:00:00 2001 From: Henriette Dinger Date: Tue, 9 Feb 2016 09:35:07 +0100 Subject: [PATCH 16/30] Change font color so that the text is readable in all themes --- app/assets/stylesheets/layout/_toolbar.sass | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/assets/stylesheets/layout/_toolbar.sass b/app/assets/stylesheets/layout/_toolbar.sass index 585e79a6a1..4fed1e54ba 100644 --- a/app/assets/stylesheets/layout/_toolbar.sass +++ b/app/assets/stylesheets/layout/_toolbar.sass @@ -151,7 +151,7 @@ float: none span.filter-selection - @include query-select-dropdown-filter-select($main-menu-font-color) + @include query-select-dropdown-filter-select($primary-color) .dropdown-scrollable overflow-y: auto From 9547784bf0b33c8adebcdaf3a6ef05b9816e2017 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20G=C3=BCnther?= Date: Tue, 9 Feb 2016 13:04:24 +0100 Subject: [PATCH 17/30] Fix staticBase of bulk delete path --- .../app/components/common/path-heleper/path-helper.service.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frontend/app/components/common/path-heleper/path-helper.service.js b/frontend/app/components/common/path-heleper/path-helper.service.js index 569f0246c7..5216ddb31d 100644 --- a/frontend/app/components/common/path-heleper/path-helper.service.js +++ b/frontend/app/components/common/path-heleper/path-helper.service.js @@ -120,7 +120,7 @@ function PathHelper() { return '/sub_projects'; }, workPackagesBulkDeletePath: function() { - return PathHelper.workPackagesPath() + '/bulk'; + return PathHelper.staticBase + PathHelper.workPackagesPath() + '/bulk'; }, workPackageJsonAutoCompletePath: function() { return '/work_packages/auto_complete.json'; From 5b32ed59e2f9b0a8cec31d2fd15de5ce78f93473 Mon Sep 17 00:00:00 2001 From: Henriette Dinger Date: Tue, 9 Feb 2016 13:32:35 +0100 Subject: [PATCH 18/30] Change labeling of button --- app/views/my/page_layout.html.erb | 2 +- config/locales/en.yml | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/app/views/my/page_layout.html.erb b/app/views/my/page_layout.html.erb index 823bdd7d3d..7a74d82269 100644 --- a/app/views/my/page_layout.html.erb +++ b/app/views/my/page_layout.html.erb @@ -80,7 +80,7 @@ See doc/COPYRIGHT.rdoc for more details.
  • <%= link_to({action: 'page'}, class: 'button') do %> - <%= l(:button_back) %> + <%= l(:button_save_back) %> <% end %>
    • <% end %> diff --git a/config/locales/en.yml b/config/locales/en.yml index d75cff6471..48b77ddc42 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -552,6 +552,7 @@ en: button_reset: "Reset" button_rollback: "Rollback to this version" button_save: "Save" + button_save_back: "Save and back" button_show: "Show" button_sort: "Sort" button_submit: "Submit" From 122672fc4424ac9170e3ac935f5298a927896e5c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20G=C3=BCnther?= Date: Tue, 9 Feb 2016 13:41:31 +0100 Subject: [PATCH 19/30] Allow queries to be changed by enter While each query item had `ui-sref` set, the keyboard event was handled by `handleSelection` which passed the request on to `WorkPackagesListController#loadQuery`. This fixes the double handling of events by using the same path as `handleSelection`. --- .../controllers/work-packages-list.controller.js | 4 +++- .../menus/query_select_dropdown_menu.html | 4 +--- .../menus/query-select-dropdown-menu-controller.js | 10 +++++----- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/frontend/app/components/routing/controllers/work-packages-list.controller.js b/frontend/app/components/routing/controllers/work-packages-list.controller.js index 3d2c34c98f..7f264a2090 100644 --- a/frontend/app/components/routing/controllers/work-packages-list.controller.js +++ b/frontend/app/components/routing/controllers/work-packages-list.controller.js @@ -216,7 +216,9 @@ function WorkPackagesListController($scope, $rootScope, $state, $stateParams, $l $scope.loadQuery = function(queryId) { // Clear unsaved changes to current query clearUrlQueryParams(); - loadingIndicator.mainPage = $state.go('work-packages.list', { 'query_id': queryId }); + loadingIndicator.mainPage = $state.go('work-packages.list', + { 'query_id': queryId }, + { reload: true }); }; function updateResults() { diff --git a/frontend/app/templates/work_packages/menus/query_select_dropdown_menu.html b/frontend/app/templates/work_packages/menus/query_select_dropdown_menu.html index 9d5645086c..cb7e13f902 100644 --- a/frontend/app/templates/work_packages/menus/query_select_dropdown_menu.html +++ b/frontend/app/templates/work_packages/menus/query_select_dropdown_menu.html @@ -15,10 +15,8 @@
    {{ group.name }}
  • -
    • diff --git a/frontend/app/work_packages/controllers/menus/query-select-dropdown-menu-controller.js b/frontend/app/work_packages/controllers/menus/query-select-dropdown-menu-controller.js index faa8822055..b9b2b22e5a 100644 --- a/frontend/app/work_packages/controllers/menus/query-select-dropdown-menu-controller.js +++ b/frontend/app/work_packages/controllers/menus/query-select-dropdown-menu-controller.js @@ -89,10 +89,6 @@ module.exports = function($scope, $sce, LABEL_MAX_CHARS, KEY_CODES) { }).indexOf(scope.selectedId); } - function performSelect() { - scope.transitionMethod(scope.selectedId); - } - function nextNonEmptyGroup(groups, currentGroupIndex) { currentGroupIndex = (currentGroupIndex === undefined) ? -1 : currentGroupIndex; while (currentGroupIndex < groups.length - 1) { @@ -184,7 +180,7 @@ module.exports = function($scope, $sce, LABEL_MAX_CHARS, KEY_CODES) { scope.handleSelection = function(event) { switch(event.which) { case KEY_CODES.enter: - performSelect(); + scope.switchToSelectedQuery(scope.selectedId); preventDefault(event); break; case KEY_CODES.down: @@ -200,6 +196,10 @@ module.exports = function($scope, $sce, LABEL_MAX_CHARS, KEY_CODES) { } }; + scope.switchToSelectedQuery = function(queryId) { + scope.transitionMethod(queryId); + } + scope.reload = function(modelId, newTitle) { scope.selectedTitle = newTitle; scope.reloadMethod(modelId); From b2e7802088c580e301e87d1a104f0328ac2393aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20G=C3=BCnther?= Date: Tue, 9 Feb 2016 14:45:17 +0100 Subject: [PATCH 20/30] Use generic URI instead of URI::HTTP Otherwise, the passed scheme is ignored. [ci skip] --- lib/tasks/packager.rake | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/tasks/packager.rake b/lib/tasks/packager.rake index 5aa623ab81..5d4518bb3b 100644 --- a/lib/tasks/packager.rake +++ b/lib/tasks/packager.rake @@ -89,7 +89,7 @@ namespace :packager do # SCM configuration may have been skipped if svn_path.present? || git_path.present? - base_url = URI::HTTP.build(protocol: ENV['SERVER_PROTOCOL'], host: ENV['SERVER_HOSTNAME']) + base_url = URI::Generic.build(scheme: ENV['SERVER_PROTOCOL'], host: ENV['SERVER_HOSTNAME']) prefix = ENV['SERVER_PATH_PREFIX'] checkout_data = Setting.repository_checkout_data From 8ae87adafa4aec10a16a9fef05ae23477e950693 Mon Sep 17 00:00:00 2001 From: Henriette Dinger Date: Tue, 9 Feb 2016 16:34:58 +0100 Subject: [PATCH 21/30] Set error message to `position: relative` so that it contributes to the height --- app/assets/stylesheets/content/_in_place_editing.sass | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/app/assets/stylesheets/content/_in_place_editing.sass b/app/assets/stylesheets/content/_in_place_editing.sass index 73b7762d41..ce7be387a9 100644 --- a/app/assets/stylesheets/content/_in_place_editing.sass +++ b/app/assets/stylesheets/content/_in_place_editing.sass @@ -176,6 +176,10 @@ .user-field-user-link display: inline + .macro-unavailable.permanent + position: relative + top: 0 + .inplace-edit--preview border: 1px solid $inplace-edit--border-color padding: 0.375rem From 595e25a3467829e4098cce7259d557155f678610 Mon Sep 17 00:00:00 2001 From: Henriette Dinger Date: Tue, 9 Feb 2016 14:57:16 +0100 Subject: [PATCH 22/30] Add missing flash messages --- app/controllers/boards_controller.rb | 1 + app/controllers/groups_controller.rb | 1 + app/controllers/news_controller.rb | 1 + app/controllers/projects_controller.rb | 5 ++++- app/controllers/roles_controller.rb | 1 + app/controllers/wiki_controller.rb | 14 ++++++++++++-- config/locales/en.yml | 2 ++ 7 files changed, 22 insertions(+), 3 deletions(-) diff --git a/app/controllers/boards_controller.rb b/app/controllers/boards_controller.rb index 2126ca3f84..4bc0432360 100644 --- a/app/controllers/boards_controller.rb +++ b/app/controllers/boards_controller.rb @@ -135,6 +135,7 @@ def move def destroy @board.destroy + flash[:notice] = l(:notice_successful_delete) redirect_to_settings_in_projects end diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb index a50ee5c096..03e3a6ddbf 100644 --- a/app/controllers/groups_controller.rb +++ b/app/controllers/groups_controller.rb @@ -111,6 +111,7 @@ def destroy @group.destroy respond_to do |format| + flash[:notice] = l(:notice_successful_delete) format.html do redirect_to(groups_url) end format.xml do head :ok end end diff --git a/app/controllers/news_controller.rb b/app/controllers/news_controller.rb index 01be79047f..9780a9ed33 100644 --- a/app/controllers/news_controller.rb +++ b/app/controllers/news_controller.rb @@ -95,6 +95,7 @@ def update def destroy @news.destroy + flash[:notice] = l(:notice_successful_delete) redirect_to action: 'index', project_id: @project end diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index 5ebfa97456..8b13423cf2 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -227,7 +227,10 @@ def destroy OpenProject::Notifications.send('project_deletion_imminent', project: @project_to_destroy) @project_to_destroy.destroy respond_to do |format| - format.html do redirect_to controller: '/admin', action: 'projects' end + format.html do + flash[:notice] = l(:notice_successful_delete) + redirect_to controller: '/admin', action: 'projects' + end end hide_project_in_layout diff --git a/app/controllers/roles_controller.rb b/app/controllers/roles_controller.rb index 5063a9d37d..8c776e4e75 100644 --- a/app/controllers/roles_controller.rb +++ b/app/controllers/roles_controller.rb @@ -89,6 +89,7 @@ def update def destroy @role = Role.find(params[:id]) @role.destroy + flash[:notice] = l(:notice_successful_delete) redirect_to action: 'index' notify_changed_roles(:removed, @role) rescue diff --git a/app/controllers/wiki_controller.rb b/app/controllers/wiki_controller.rb index d1138b8e2f..d2429fe609 100644 --- a/app/controllers/wiki_controller.rb +++ b/app/controllers/wiki_controller.rb @@ -182,7 +182,11 @@ def edit # Creates a new page or updates an existing one def update @page = @wiki.find_or_new_page(wiki_page_title) - return render_403 unless editable? + unless editable? + flash[:error] = l(:error_unable_update_wiki) + return render_403 + end + @page.content = WikiContent.new(page: @page) if @page.new_record? @content = @page.content_for_version(params[:version]) @@ -205,6 +209,7 @@ def update attachments = Attachment.attach_files(@page, params[:attachments]) render_attachment_warning_if_needed(@page) call_hook(:controller_wiki_edit_after_save, params: params, page: @page) + flash[:notice] = l(:notice_successful_update) redirect_to_show else render action: 'edit' @@ -278,7 +283,10 @@ def annotate # Removes a wiki page and its history # Children can be either set as root pages, removed or reassigned to another parent page def destroy - return render_403 unless editable? + unless editable? + flash[:error] = l(:error_unable_delete_wiki) + return render_403 + end @descendants_count = @page.descendants.size if @descendants_count > 0 @@ -303,8 +311,10 @@ def destroy @page.destroy if page = @wiki.find_page(@wiki.start_page) || @wiki.pages.first + flash[:notice] = l(:notice_successful_delete) redirect_to action: 'index', project_id: @project, id: page else + flash[:notice] = l(:notice_successful_delete) redirect_to project_path(@project) end end diff --git a/config/locales/en.yml b/config/locales/en.yml index 48b77ddc42..a1cc84d8f5 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -764,6 +764,8 @@ en: error_unable_delete_status: "The work package status cannot be deleted since it is used by at least one work package." error_unable_delete_default_status: "Unable to delete the default work package status. Please select another default work package status before deleting the current one." error_unable_to_connect: "Unable to connect (%{value})" + error_unable_delete_wiki: "Unable to delete the wiki page." + error_unable_update_wiki: "Unable to update the wiki page." error_workflow_copy_source: "Please select a source type or role" error_workflow_copy_target: "Please select target type(s) and role(s)" error_menu_item_not_created: Menu item could not be added From 34893071e1cee7881f99725128188f316aff1a00 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20G=C3=BCnther?= Date: Tue, 9 Feb 2016 15:31:47 +0100 Subject: [PATCH 23/30] Format dates according to the user's timezone By default, the date headlines of activities will be output with UTC+0, which may pose a problem when work in different timezones. --- app/controllers/activities_controller.rb | 4 ++-- app/helpers/application_helper.rb | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/controllers/activities_controller.rb b/app/controllers/activities_controller.rb index 51b772d2a6..5ce3e6b3e8 100644 --- a/app/controllers/activities_controller.rb +++ b/app/controllers/activities_controller.rb @@ -39,7 +39,7 @@ def index begin; @date_to = params[:from].to_date + 1; rescue; end end - @date_to ||= Date.today + 1 + @date_to ||= User.current.today + 1 @date_from = @date_to - @days @with_subprojects = params[:with_subprojects].nil? ? Setting.display_subprojects_work_packages? : (params[:with_subprojects] == '1') @author = (params[:user_id].blank? ? nil : User.active.find(params[:user_id])) @@ -56,7 +56,7 @@ def index if events.empty? || stale?(etag: [@activity.scope, @date_to, @date_from, @with_subprojects, @author, events.first, User.current, current_language]) respond_to do |format| format.html do - @events_by_day = events.group_by { |e| e.event_datetime.to_date } + @events_by_day = events.group_by { |e| e.event_datetime.in_time_zone(User.current.time_zone).to_date } render layout: false if request.xhr? end format.atom do diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index efbef67f80..4141846dbc 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -123,7 +123,7 @@ def format_activity_title(text) end def format_activity_day(date) - date == Date.today ? l(:label_today).titleize : format_date(date) + date == User.current.today ? l(:label_today).titleize : format_date(date) end def format_activity_description(text) From 836224a542b96b00ba432149bc033db3d0976c6c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20G=C3=BCnther?= Date: Wed, 10 Feb 2016 11:20:05 +0100 Subject: [PATCH 24/30] Run plugin specs/cukes in this product --- .travis.yml | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/.travis.yml b/.travis.yml index 62bf826502..740d86ce16 100644 --- a/.travis.yml +++ b/.travis.yml @@ -67,6 +67,13 @@ env: - "TEST_SUITE=rspec DB=mysql GROUP_SIZE=6 GROUP=4" - "TEST_SUITE=rspec DB=mysql GROUP_SIZE=6 GROUP=5" - "TEST_SUITE=rspec DB=mysql GROUP_SIZE=6 GROUP=6" + - "TEST_SUITE=plugins:spec DB=mysql GROUP_SIZE=3 GROUP=1" + - "TEST_SUITE=plugins:spec DB=mysql GROUP_SIZE=3 GROUP=2" + - "TEST_SUITE=plugins:spec DB=mysql GROUP_SIZE=3 GROUP=3" + - "TEST_SUITE=plugins:cucumber DB=mysql GROUP_SIZE=3 GROUP=1" + - "TEST_SUITE=plugins:cucumber DB=mysql GROUP_SIZE=3 GROUP=2" + - "TEST_SUITE=plugins:cucumber DB=mysql GROUP_SIZE=3 GROUP=3" + - "TEST_SUITE=spec_legacy DB=postgres GROUP_SIZE=2 GROUP=1" - "TEST_SUITE=spec_legacy DB=postgres GROUP_SIZE=2 GROUP=2" @@ -79,6 +86,12 @@ env: - "TEST_SUITE=rspec DB=postgres GROUP_SIZE=6 GROUP=4" - "TEST_SUITE=rspec DB=postgres GROUP_SIZE=6 GROUP=5" - "TEST_SUITE=rspec DB=postgres GROUP_SIZE=6 GROUP=6" + - "TEST_SUITE=plugins:spec DB=postgres GROUP_SIZE=3 GROUP=1" + - "TEST_SUITE=plugins:spec DB=postgres GROUP_SIZE=3 GROUP=2" + - "TEST_SUITE=plugins:spec DB=postgres GROUP_SIZE=3 GROUP=3" + - "TEST_SUITE=plugins:cucumber DB=postgres GROUP_SIZE=3 GROUP=1" + - "TEST_SUITE=plugins:cucumber DB=postgres GROUP_SIZE=3 GROUP=2" + - "TEST_SUITE=plugins:cucumber DB=postgres GROUP_SIZE=3 GROUP=3" before_install: - "echo `firefox -v`" From 9c403189526bf77380260a7c4fe45466e89a3187 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20G=C3=BCnther?= Date: Thu, 11 Feb 2016 12:51:37 +0100 Subject: [PATCH 25/30] Remove unnecessary step definition Only used by one reporting feature and that seems to be invalid. --- features/step_definitions/custom_web_steps.rb | 9 --------- 1 file changed, 9 deletions(-) diff --git a/features/step_definitions/custom_web_steps.rb b/features/step_definitions/custom_web_steps.rb index 2e260b4a8f..664581cd31 100644 --- a/features/step_definitions/custom_web_steps.rb +++ b/features/step_definitions/custom_web_steps.rb @@ -52,15 +52,6 @@ find("##{name}").click end -When /^(?:|I )jump to [Pp]roject "([^\"]*)"$/ do |project| - click_link('Projects') - # supports both variants of finding: by class and by id - # id is older and can be dropped later - project_div = find(:css, '.project-search-results', text: project) || find(:css, '#project-search-results', text: project) - - page.execute_script("window.location = jQuery(\"##{project_div[:id]} div[title='#{project}']\").parent().data('select2Data').project.url;") -end - Then /^"([^"]*)" should be selected for "([^"]*)"$/ do |value, select_id| # that makes capybara wait for the ajax request find(:xpath, '//body') From 65e5d8fc5ca5602764c91b1b1efc3d9909a8aebf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20G=C3=BCnther?= Date: Fri, 12 Feb 2016 10:25:00 +0100 Subject: [PATCH 26/30] Remove query feature Is covered by `spec/features/work_packages/select_query_spec.rb` --- features/menu_items/query_menu_items.feature | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/features/menu_items/query_menu_items.feature b/features/menu_items/query_menu_items.feature index f8249e9d86..0ea0645f56 100644 --- a/features/menu_items/query_menu_items.feature +++ b/features/menu_items/query_menu_items.feature @@ -53,17 +53,6 @@ Feature: Query menu items | Features | Feature | And I am already logged in as "bob" - @javascript @selenium - Scenario: Create a query menu item - When I go to the applied query "Bugs" on the work packages index page of the project "Awesome Project" - And the work package table has finished loading - And I click on "Settings" - And I click on "Share ..." - And I check "Show page in menu" - And I click "Save" - And I click "Work packages" within "#main-menu" - Then I should see "Bugs" within "#main-menu" - @javascript @selenium Scenario: Delete a query menu item Given the user "bob" has the following query menu items in the project "Awesome Project": From 4fbbe60e664d37cef319eae742510f2fa362341d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20G=C3=BCnther?= Date: Fri, 12 Feb 2016 10:53:11 +0100 Subject: [PATCH 27/30] Wait for AJAX on membership edit --- features/admin/user.feature | 1 + 1 file changed, 1 insertion(+) diff --git a/features/admin/user.feature b/features/admin/user.feature index a20011ef23..08917d1d22 100644 --- a/features/admin/user.feature +++ b/features/admin/user.feature @@ -104,6 +104,7 @@ Feature: User | alpha | | beta | | gamma | + And I wait for the AJAX requests to finish And I go to the memberships tab of the edit page for the user peter Then I should see membership to the project "project1" with the roles: | alpha | From c7e410ef1179028c30dd9b6d904d1ba04d20e0cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20G=C3=BCnther?= Date: Fri, 12 Feb 2016 19:07:37 +0100 Subject: [PATCH 28/30] Warn users when OP cookie is missing When a user has disabled cookies, trying to log in to the application will result in a 422 error due to CSRF validation trying to use the session in Rails 4+. This commit introduces a separate warning when the OpenProject cookie was not found, which should always be present after the first request to the application. --- app/controllers/application_controller.rb | 11 ++++++++++- config/locales/en.yml | 2 ++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 3b4cff1f18..6b8924a977 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -89,7 +89,16 @@ def handle_unverified_request # is raised here, but is denied by disable_api. # # See http://stackoverflow.com/a/15350123 for more information on login CSRF. - render_error status: 422, message: 'Invalid form authenticity token.' unless api_request? + unless api_request? + + # Check whether user have cookies enabled, otherwise they'll only be + # greeted with the CSRF error upon login. + cookie_missing = request.cookies['_open_project_session'].nil? + + message = I18n.t(:error_token_authenticity) + message << ' ' + I18n.t(:error_cookie_missing) if cookie_missing + render_error status: 422, message: message + end end rescue_from ActionController::ParameterMissing do |exception| diff --git a/config/locales/en.yml b/config/locales/en.yml index a1cc84d8f5..a7e558a84b 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -753,6 +753,8 @@ en: error_can_not_remove_role: "This role is in use and cannot be deleted." error_can_not_reopen_work_package_on_closed_version: "A work package assigned to a closed version cannot be reopened" error_check_user_and_role: "Please choose a user and a role." + error_cookie_missing: 'The OpenProject cookie is missing. Please ensure that cookies are enabled, as this application will not properly function without.' + error_token_authenticity: 'Unable to verify Cross-Site Request Forgery token.' error_work_package_done_ratios_not_updated: "Work package done ratios not updated." error_work_package_not_found_in_project: "The work package was not found or does not belong to this project" error_must_be_project_member: "must be project member" From 82eb512450f0c4836500b216a9873ecce955dcfb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20G=C3=BCnther?= Date: Mon, 15 Feb 2016 09:41:14 +0100 Subject: [PATCH 29/30] Make session name configurable --- app/controllers/application_controller.rb | 11 ++++++++--- config/configuration.yml.example | 3 +++ config/initializers/session_store.rb | 2 +- lib/open_project/configuration.rb | 1 + 4 files changed, 13 insertions(+), 4 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 6b8924a977..ce300b5b85 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -93,10 +93,8 @@ def handle_unverified_request # Check whether user have cookies enabled, otherwise they'll only be # greeted with the CSRF error upon login. - cookie_missing = request.cookies['_open_project_session'].nil? - message = I18n.t(:error_token_authenticity) - message << ' ' + I18n.t(:error_cookie_missing) if cookie_missing + message << ' ' + I18n.t(:error_cookie_missing) if openproject_cookie_missing? render_error status: 422, message: message end end @@ -202,6 +200,13 @@ def check_if_login_required require_login if Setting.login_required? end + # Checks if the session cookie is missing. + # This is useful only on a second request + def openproject_cookie_missing? + request.cookies[OpenProject::Configuration['session_cookie_name']].nil? + end + helper_method :openproject_cookie_missing? + def log_requesting_user return unless Setting.log_requesting_user? login_and_mail = " (#{escape_for_logging(User.current.login)} ID: #{User.current.id} " \ diff --git a/config/configuration.yml.example b/config/configuration.yml.example index cff1a55428..25fa1a6159 100644 --- a/config/configuration.yml.example +++ b/config/configuration.yml.example @@ -194,6 +194,9 @@ default: # autologin_cookie_path: # autologin_cookie_secure: + # Configuration of the session cookie + # session_cookie_name: the name of the OpenProject cookie (default: _open_project_session) + # disable browser cache for security reasons # see: https://websecuritytool.codeplex.com/wikipage?title=Checks#http-cache-control-header-no-store # disable_browser_cache: true diff --git a/config/initializers/session_store.rb b/config/initializers/session_store.rb index adeccd2dbc..05045278cc 100644 --- a/config/initializers/session_store.rb +++ b/config/initializers/session_store.rb @@ -35,7 +35,7 @@ relative_url_root = config['rails_relative_url_root'].presence session_options = { - key: '_open_project_session', + key: config['session_cookie_name'], path: relative_url_root } diff --git a/lib/open_project/configuration.rb b/lib/open_project/configuration.rb index 77c8eb0aed..fff9d1ccc6 100644 --- a/lib/open_project/configuration.rb +++ b/lib/open_project/configuration.rb @@ -54,6 +54,7 @@ module Configuration 'cache_memcache_server' => nil, # where to store session data 'session_store' => :cache_store, + 'session_cookie_name' => '_open_project_session', # url-path prefix 'rails_relative_url_root' => '', 'rails_force_ssl' => false, From eb4d71e31f37613f9118c8521a1a0b54ea982aa0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20G=C3=BCnther?= Date: Mon, 15 Feb 2016 11:36:41 +0100 Subject: [PATCH 30/30] Release opf/openproject v5.0.15 --- Gemfile.lock | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index 2027714d35..c4c8019d97 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -45,8 +45,8 @@ GIT GIT remote: https://github.com/opf/openproject-translations.git - revision: 089407397e309588a0a575f175f2665ffa1d2316 - branch: release/5.0 + revision: c5146cc45b62d11cc83329ce5e9ac6caf98f510e + branch: stable/5 specs: openproject-translations (5.0.15) crowdin-api (~> 0.4.0)