Merge branch 'release/7.4' into dev

app/contracts/work_packages/base_contract.rb

@@ -269,10 +269,19 @@ module WorkPackages
     end
 
     def invalid_relations_with_new_hierarchy
-      Relation
-        .from_parent_to_self_and_descendants(model)
-        .or(Relation.from_self_and_descendants_to_ancestors(model))
-        .direct
+      query = Relation.from_parent_to_self_and_descendants(model)
+                      .or(Relation.from_self_and_descendants_to_ancestors(model))
+                      .direct
+
+      # Ignore the immediate relation from the old parent to the model
+      # since that will still exist before saving.
+      old_parent_id = model.parent_id_was
+
+      if old_parent_id.present?
+        query.where.not(hierarchy: 1, from_id: old_parent_id, to_id: model.id)
+      else
+        query
+      end
     end
   end
 end

app/helpers/accounts_helper.rb

@@ -30,8 +30,22 @@ module AccountsHelper
   end
 
   def registration_footer
-    footer = Setting.registration_footer[I18n.locale.to_s].presence
+    footer = registration_footer_for lang: I18n.locale.to_s
 
     Footer.new(footer).to_html if footer
   end
+
+  ##
+  # Gets the registration footer in the given language.
+  # If registration footers are defined via the OpenProject configuration
+  # then any footers defined via settings will be ignored.
+  #
+  # @param lang [String] ISO 639-1 language code (e.g. 'en', 'de')
+  def registration_footer_for(lang:)
+    if footer = OpenProject::Configuration.registration_footer.presence
+      footer[lang.to_s].presence
+    else
+      Setting.registration_footer[lang.to_s].presence
+    end
+  end
 end

app/helpers/repositories_helper.rb

@@ -40,6 +40,12 @@ module RepositoriesHelper
     end
   end
 
+  ##
+  # Format revision commits with plain formatter
+  def format_revision_text(commit_message)
+    format_text(commit_message, format: 'plain')
+  end
+
   def truncate_at_line_break(text, length = 255)
     if text
       text.gsub(%r{^(.{#{length}}[^\n]*)\n.+$}m, '\\1...')

app/models/permitted_params.rb

@@ -27,6 +27,8 @@
 # See docs/COPYRIGHT.rdoc for more details.
 #++
 
+require 'permitted_params/allowed_settings'
+
 class PermittedParams
   # This class intends to provide a method for all params hashes coming from the
   # client and that are used for mass assignment.
@@ -180,18 +182,7 @@ class PermittedParams
 
   def settings
     permitted_params = params.require(:settings).permit
-
-    all_setting_keys = Setting.available_settings.keys
-    all_valid_keys = if OpenProject::Configuration.disable_password_login?
-                       all_setting_keys - %w(password_min_length
-                                             password_active_rules
-                                             password_min_adhered_rules
-                                             password_days_valid
-                                             password_count_former_banned
-                                             lost_password)
-                     else
-                       all_setting_keys
-                     end
+    all_valid_keys = AllowedSettings.all
 
     permitted_params.merge(params[:settings].to_unsafe_hash.slice(*all_valid_keys))
   end

app/models/permitted_params/allowed_settings.rb

@@ -0,0 +1,65 @@
+class PermittedParams
+  module AllowedSettings
+    class Restriction
+      attr_reader :restricted_keys, :condition
+
+      def initialize(restricted_keys, condition)
+        @restricted_keys = restricted_keys
+        @condition = condition
+      end
+
+      def applicable?
+        if condition.respond_to? :call
+          condition.call
+        else
+          condition
+        end
+      end
+    end
+
+    module_function
+
+    def all
+      keys = Setting.available_settings.keys
+
+      restrictions.select(&:applicable?).each do |restriction|
+        restricted_keys = restriction.restricted_keys
+
+        keys.delete_if { |key| restricted_keys.include? key }
+      end
+
+      keys
+    end
+
+    def add_restriction!(keys:, condition:)
+      restrictions << Restriction.new(keys, condition)
+    end
+
+    def restrictions
+      @restrictions ||= []
+    end
+
+    def init!
+      password_keys = %w(
+        password_min_length
+        password_active_rules
+        password_min_adhered_rules
+        password_days_valid
+        password_count_former_banned
+        lost_password
+      )
+
+      add_restriction!(
+        keys: password_keys,
+        condition: -> { OpenProject::Configuration.disable_password_login? }
+      )
+
+      add_restriction!(
+        keys: %w(registration_footer),
+        condition: -> { OpenProject::Configuration.registration_footer.present? }
+      )
+    end
+
+    init!
+  end
+end

app/models/work_package/scheduling_rules.rb

@@ -57,9 +57,15 @@ module WorkPackage::SchedulingRules
   #   B is 2017/07/25
   #   A is 2017/07/25
   def soonest_start
+    # Using a hand crafted union here instead of the alternative
+    # Relation.from_work_package_or_ancestors(self).follows
+    # as the performance of the above would be several orders of magnitude worse on MySql
+    sql = Relation.connection.unprepared_statement do
+      "((#{ancestors_follows_relations.to_sql}) UNION (#{own_follows_relations.to_sql})) AS relations"
+    end
+
     @soonest_start ||=
-      Relation.from_work_package_or_ancestors(self)
-              .follows
+      Relation.from(sql)
               .map(&:successor_soonest_start)
               .compact
               .max
@@ -79,4 +85,14 @@ module WorkPackage::SchedulingRules
       1
     end
   end
+
+  private
+
+  def ancestors_follows_relations
+    Relation.where(from_id: self.ancestors_relations.select(:from_id)).follows
+  end
+
+  def own_follows_relations
+    Relation.where(from_id: self.id).follows
+  end
 end

app/views/repositories/_revisions.html.erb

@@ -142,7 +142,7 @@ See docs/COPYRIGHT.rdoc for more details.
                 <%=h changeset.author %>
               </td>
               <td class="comments">
-                <%= format_text(truncate_at_line_break(Changeset.to_utf8(changeset.comments, changeset.repository.repo_log_encoding))) %>
+                <%= format_revision_text(truncate_at_line_break(Changeset.to_utf8(changeset.comments, changeset.repository.repo_log_encoding))) %>
               </td>
             </tr>
             <% line_num += 1 %>

app/views/repositories/revision.html.erb

@@ -51,7 +51,7 @@ See docs/COPYRIGHT.rdoc for more details.
 <p><% if @changeset.scmid %>ID: <%= h(@changeset.scmid) %><br />
   <% end %>
   <span class="author"><%= authoring(@changeset.committed_on, @changeset.author) %></span></p>
-<%= format_text @changeset.comments %>
+<%= format_revision_text @changeset.comments %>
 <% if @changeset.work_packages.visible.any? %>
   <h3><%= l(:label_related_work_packages) %></h3>
   <ul>

app/views/settings/_authentication.html.erb

@@ -46,14 +46,16 @@ See docs/COPYRIGHT.rdoc for more details.
       </div>
     </fieldset>
 
-    <%= cell Settings::NumericSettingCell, "invitation_expiration_days", unit: "days" %>
+    <% if OpenProject::Configuration.registration_footer.blank? %>
+      <%= cell Settings::NumericSettingCell, "invitation_expiration_days", unit: "days" %>
 
-    <fieldset class="form--fieldset">
-      <fieldset id="registration_footer" class="form--fieldset">
-        <legend class="form--fieldset-legend"><%= I18n.t(:setting_registration_footer) %></legend>
-        <%= cell Settings::TextSettingCell, I18n.locale, name: "registration_footer" %>
+      <fieldset class="form--fieldset">
+        <fieldset id="registration_footer" class="form--fieldset">
+          <legend class="form--fieldset-legend"><%= I18n.t(:setting_registration_footer) %></legend>
+          <%= cell Settings::TextSettingCell, I18n.locale, name: "registration_footer" %>
+        </fieldset>
       </fieldset>
-    </fieldset>
+    <% end %>
 
     <fieldset class="form--fieldset">
       <legend class="form--fieldset-legend"><%= I18n.t(:passwords, scope: [:settings]) %></legend>

app/views/users/_mail_notifications.html.erb

@@ -41,7 +41,7 @@ See docs/COPYRIGHT.rdoc for more details.
   </div>
 </div>
 
-<%= content_tag 'div', id: 'notified-projects', :'ng-show' => "mail_notifications === 'selected'" do %>
+<%= content_tag 'div', id: 'notified-projects', class: "ng-cloak", :'ng-if' => "mail_notifications === 'selected'" do %>
   <div class="form--field -no-label">
     <div class="form--field-container -vertical">
       <% @user.projects.each do |project| %>

lib/api/v3/relations/relation_collection_representer.rb

@@ -32,6 +32,7 @@ module API
     module Relations
       class RelationCollectionRepresenter < ::API::Decorators::UnpaginatedCollection
         element_decorator ::API::V3::Relations::RelationRepresenter
+        self.to_eager_load = ::API::V3::Relations::RelationRepresenter.to_eager_load
       end
     end
   end

lib/api/v3/work_packages/work_package_relations_api.rb

@@ -43,6 +43,7 @@ module API
                         .where(:involved, '=', @work_package.id)
                         .results
                         .non_hierarchy
+                        .includes(::API::V3::Relations::RelationCollectionRepresenter.to_eager_load)
 
             ::API::V3::Relations::RelationCollectionRepresenter.new(
               relations,

lib/api/v3/work_packages/work_package_representer.rb

@@ -563,7 +563,10 @@ module API
 
         def relations
           self_path = api_v3_paths.work_package_relations(represented.id)
-          visible_relations = represented.visible_relations(current_user).non_hierarchy
+          visible_relations = represented
+                              .visible_relations(current_user)
+                              .non_hierarchy
+                              .includes(::API::V3::Relations::RelationCollectionRepresenter.to_eager_load)
 
           ::API::V3::Relations::RelationCollectionRepresenter.new(visible_relations,
                                                                   self_path,

lib/open_project/configuration.rb

@@ -33,11 +33,11 @@ module OpenProject
   module Configuration
     extend Helpers
 
-    ENV_PREFIX = 'OPENPROJECT_'
+    ENV_PREFIX = 'OPENPROJECT_'.freeze
 
     # Configuration default values
     @defaults = {
-      'attachments_storage'     => 'file',
+      'attachments_storage' => 'file',
       'attachments_storage_path' => nil,
       'autologin_cookie_name'   => 'autologin',
       'autologin_cookie_path'   => '/',
@@ -74,12 +74,12 @@ module OpenProject
       'email_delivery_method' => nil,
       'smtp_address' => nil,
       'smtp_port' => nil,
-      'smtp_domain' => nil,  # HELO domain
+      'smtp_domain' => nil, # HELO domain
       'smtp_authentication' => nil,
       'smtp_user_name' => nil,
       'smtp_password' => nil,
       'smtp_enable_starttls_auto' => nil,
-      'smtp_openssl_verify_mode' => nil,  # 'none', 'peer', 'client_once' or 'fail_if_no_peer_cert'
+      'smtp_openssl_verify_mode' => nil, # 'none', 'peer', 'client_once' or 'fail_if_no_peer_cert'
       'sendmail_location' => '/usr/sbin/sendmail',
       'sendmail_arguments' => '-i',
 
@@ -118,7 +118,9 @@ module OpenProject
       'main_content_language' => 'english',
 
       # Allow in-context translations to be loaded with CSP
-      'crowdin_in_context_translations' => true
+      'crowdin_in_context_translations' => true,
+
+      'registration_footer' => {}
     }
 
     @config = nil
@@ -149,8 +151,8 @@ module OpenProject
       # exists
       def override_config!(config, source = default_override_source)
         config.keys
-          .select { |key| source.include? key.upcase }
-          .each   do |key| config[key] = extract_value key, source[key.upcase] end
+              .select { |key| source.include? key.upcase }
+              .each   { |key| config[key] = extract_value key, source[key.upcase] }
 
         config.deep_merge! merge_config(config, source)
       end
@@ -350,7 +352,6 @@ module OpenProject
         ActionMailer::Base.smtp_settings[:enable_starttls_auto] = Setting.smtp_enable_starttls_auto?
       end
 
-
       ##
       # The default source for overriding configuration values
       # is ENV, but may be changed for testing purposes
@@ -367,13 +368,12 @@ module OpenProject
       # @return A ruby object (e.g. Integer, Float, String, Hash, Boolean, etc.)
       # @raise [ArgumentError] If the string could not be parsed.
       def extract_value(key, value)
-
         # YAML parses '' as false, but empty ENV variables will be passed as that.
         # To specify specific values, one can use !!str (-> '') or !!null (-> nil)
         return value if value == ''
 
         YAML.load(value)
-      rescue => e
+      rescue StandardError => e
         raise ArgumentError, "Configuration value for '#{key}' is invalid: #{e.message}"
       end
 
@@ -410,9 +410,9 @@ module OpenProject
         if config['email_delivery']
           unless options[:disable_deprecation_message]
             ActiveSupport::Deprecation.warn 'Deprecated mail delivery settings used. Please ' +
-              'update them in config/configuration.yml or use ' +
-              'environment variables. See doc/CONFIGURATION.md for ' +
-              'more information.'
+                                            'update them in config/configuration.yml or use ' +
+                                            'environment variables. See doc/CONFIGURATION.md for ' +
+                                            'more information.'
           end
 
           config['email_delivery_method'] = config['email_delivery']['delivery_method'] || :smtp

lib/redmine/i18n.rb

@@ -41,6 +41,7 @@ module Redmine
         Dir.glob(Rails.root.join('config/locales/**/*.yml'))
           .map { |f| File.basename(f).split('.').first }
           .reject! { |l| /\Ajs-/.match(l.to_s) }
+          .uniq
           .map(&:to_sym)
       end
     end

spec/lib/api/v3/relations/relation_representer_spec.rb

@@ -72,7 +72,7 @@ describe ::API::V3::Relations::RelationRepresenter do
         "to" => {
           "href" => "/api/v3/work_packages/#{to.id}",
           "title" => to.subject
-        },
+        }
       },
       "id" => relation.id,
       "name" => "follows",

spec/lib/api/v3/work_packages/work_package_representer_spec.rb

@@ -958,7 +958,7 @@ describe ::API::V3::WorkPackages::WorkPackageRepresenter do
 
         before do
           allow(work_package)
-            .to receive_message_chain(:visible_relations, :non_hierarchy)
+            .to receive_message_chain(:visible_relations, :non_hierarchy, :includes)
             .and_return([relation])
         end
 

spec/lib/redmine/i18n_spec.rb

@@ -84,13 +84,9 @@ module OpenProject
       end
 
       # it is OK if more languages exist
-      it 'has a language for every language file' do
-        lang_files_count = Dir.glob(Rails.root.join('config/locales/**/*.yml'))
-                           .map { |f| File.basename(f) }
-                           .reject { |b| b.starts_with? 'js' }
-                           .size
-
-        expect(all_languages.size).to eql lang_files_count
+      it 'has multiple languages' do
+        expect(all_languages).to include :en, :de, :fr, :es
+        expect(all_languages.size).to be >= 25
       end
     end
 

spec/models/permitted_params_spec.rb

@@ -863,6 +863,48 @@ describe PermittedParams, type: :model do
 
       it { expect(subject).to eq(permitted_hash) }
     end
+
+    describe 'with no registration footer configured' do
+      before do
+        allow(OpenProject::Configuration)
+          .to receive(:registration_footer)
+          .and_return({})
+      end
+
+      let(:hash) do
+        {
+          'registration_footer' => {
+            'en' => 'some footer'
+          }
+        }
+      end
+
+      it_behaves_like 'allows params'
+    end
+
+    describe 'with a registration footer configured' do
+      include_context 'prepare params comparison'
+
+      before do
+        allow(OpenProject::Configuration)
+          .to receive(:registration_footer)
+          .and_return("en" => "configured footer")
+      end
+
+      let(:hash) do
+        {
+          'registration_footer' => {
+            'en' => 'some footer'
+          }
+        }
+      end
+
+      let(:permitted_hash) do
+        {}
+      end
+
+      it { expect(subject).to eq(permitted_hash) }
+    end
   end
 
   describe '#enumerations' do

spec/services/work_packages/update_service_integration_spec.rb

@@ -1119,4 +1119,27 @@ describe WorkPackages::UpdateService, 'integration tests', type: :model do
       end
     end
   end
+
+  ##
+  # Regression test for #27746
+  # - Parent: A
+  # - Child1: B
+  # - Child2: C
+  #
+  # Trying to set parent of C to B failed because parent relation is requested before change is saved.
+  describe 'Changing parent to a new one that has the same parent as the current element (Regression #27746)' do
+    let(:project) { FactoryGirl.create :project }
+    let!(:wp_a) { FactoryGirl.create :work_package }
+    let!(:wp_b) { FactoryGirl.create :work_package, parent: wp_a }
+    let!(:wp_c) { FactoryGirl.create :work_package, parent: wp_a }
+
+    let(:user) { FactoryGirl.create :admin }
+    let(:work_package) { wp_c }
+
+    let(:attributes) { { parent: wp_b } }
+
+    it 'allows changing the parent' do
+      expect(subject).to be_success
+    end
+  end
 end

spec/views/account/register.html.erb_spec.rb

@@ -94,11 +94,24 @@ describe 'account/register', type: :view do
       allow(Setting).to receive(:registration_footer).and_return("en" => footer)
 
       assign(:user, user)
-      render
     end
 
-    it 'should render the emai footer' do
+    it 'should render the registration footer from the settings' do
+      render
+
       expect(rendered).to include(footer)
     end
+
+    context 'with a registration footer in the OpenProject configuration' do
+      before do
+        allow(OpenProject::Configuration).to receive(:registration_footer).and_return("en" => footer.reverse)
+      end
+
+      it 'should render the registration footer from the configuration, overriding the settings' do
+        render
+
+        expect(rendered).to include(footer.reverse)
+      end
+    end
   end
 end

spec/views/settings/_authentication.html.erb_spec.rb

@@ -58,4 +58,29 @@ describe 'settings/_authentication', type: :view do
       expect(rendered).not_to have_text I18n.t(:brute_force_prevention, scope: [:settings])
     end
   end
+
+  context 'with no registration_footer configured' do
+    before do
+      allow(OpenProject::Configuration).to receive(:registration_footer).and_return({})
+      render
+    end
+
+    it 'shows the registration footer textfield' do
+      expect(rendered).to have_text I18n.t(:setting_registration_footer)
+    end
+  end
+
+  context 'with registration_footer configured' do
+    before do
+      allow(OpenProject::Configuration)
+        .to receive(:registration_footer)
+        .and_return("en" => "You approve.")
+
+      render
+    end
+
+    it 'does not show the registration footer textfield' do
+      expect(rendered).not_to have_text I18n.t(:setting_registration_footer)
+    end
+  end
 end