Merge branch 'release/11.1' into dev

app/helpers/versions_helper.rb

@@ -41,6 +41,8 @@ module VersionsHelper
   def link_to_version(version, html_options = {}, options = {})
     return '' unless version&.is_a?(Version)
 
+    html_options = html_options.merge(id: link_to_version_id(version))
+
     link_name = options[:before_text].to_s.html_safe + format_version_name(version, options[:project] || @project)
     link_to_if version.visible?,
                link_name,
@@ -48,6 +50,10 @@ module VersionsHelper
                html_options
   end
 
+  def link_to_version_id(version)
+    ERB::Util.url_encode("version-#{version.name}")
+  end
+
   def format_version_name(version, project = @project)
     h(version.to_s_for_project(project))
   end

app/uploaders/direct_fog_uploader.rb

@@ -3,17 +3,26 @@ require_relative 'fog_file_uploader'
 class DirectFogUploader < FogFileUploader
   include CarrierWaveDirect::Uploader
 
-  def self.for_attachment(attachment)
+  ##
+  # This needs to be true so that the necessary condition is included
+  # in S3 upload policy (only relevant for direct uploads).
+  def will_include_content_type
+    true
+  end
+
+  class << self
+    def for_attachment(attachment)
       for_uploader attachment.file
     end
 
-  def self.for_uploader(fog_file_uploader)
+    def for_uploader(fog_file_uploader)
       raise ArgumentError, "FogFileUploader expected" unless fog_file_uploader.is_a? FogFileUploader
 
       uploader = self.new
 
       uploader.instance_variable_set "@file", fog_file_uploader.file
       uploader.instance_variable_set "@key", fog_file_uploader.path
+      uploader.instance_variable_set "@model", fog_file_uploader.model
 
       uploader
     end
@@ -25,28 +34,48 @@ class DirectFogUploader < FogFileUploader
     # @param success_action_redirect [String] URL to redirect to if successful (none by default, using status).
     # @param success_action_status [String] The HTTP status to return on success (201 by default).
     # @param max_file_size [Integer] The maximum file size to be allowed in bytes.
-  def self.direct_fog_hash(
+    def direct_fog_hash(
       attachment:,
       success_action_redirect: nil,
       success_action_status: "201",
       max_file_size: Setting.attachment_max_size * 1024
     )
-    uploader = for_attachment attachment
+      uploader = direct_fog_hash_uploader attachment, success_action_redirect, success_action_status
+      hash = uploader
+        .direct_fog_hash(enforce_utf8: false, max_file_size: max_file_size)
+        .merge(extra_fog_hash_attributes(uploader: uploader))
 
       if success_action_redirect.present?
-      uploader.success_action_redirect = success_action_redirect
-      uploader.use_action_status = false
+        hash.merge(success_action_redirect: success_action_redirect)
       else
-      uploader.success_action_status = success_action_status
-      uploader.use_action_status = true
+        hash.merge(success_action_status: success_action_status)
       end
+    end
+
+    def extra_fog_hash_attributes(uploader:)
+      return {} unless include_content_type?(uploader)
 
-    hash = uploader.direct_fog_hash(enforce_utf8: false, max_file_size: max_file_size)
+      {
+        "Content-Type": uploader.fog_attributes[:"Content-Type"]
+      }
+    end
 
+    private
+
+    def include_content_type?(uploader)
+      uploader.will_include_content_type && uploader.fog_attributes.include?(:"Content-Type")
+    end
+
+    def direct_fog_hash_uploader(attachment, success_action_redirect, success_action_status)
+      for_attachment(attachment).tap do |uploader|
         if success_action_redirect.present?
-      hash.merge(success_action_redirect: success_action_redirect)
+          uploader.success_action_redirect = success_action_redirect
+          uploader.use_action_status = false
         else
-      hash.merge(success_action_status: success_action_status)
+          uploader.success_action_status = success_action_status
+          uploader.use_action_status = true
+        end
+      end
     end
   end
 end

app/uploaders/fog_file_uploader.rb

@@ -57,6 +57,16 @@ class FogFileUploader < CarrierWave::Uploader::Base
     super
   end
 
+  ##
+  # This is necessary for carrierwave to set the Content-Type in the S3 metadata for instance.
+  def fog_attributes
+    content_type = model.content_type
+
+    return super if content_type.blank?
+
+    super.merge "Content-Type": content_type
+  end
+
   ##
   # Generates a download URL for this file.
   #

app/views/versions/_roadmap_filter.html.erb

@@ -21,7 +21,6 @@
 
     <% if @project.descendants.active.any? %>
       <div class="form--space"></div>
-      <%= hidden_field_tag 'with_subprojects', 0 %>
       <div class="form--field -trailing-label -no-margin">
         <%= styled_label_tag "with-subprojects", t(:label_subproject_plural) %>
 

app/views/versions/_roadmap_version_links.html.erb

@@ -1,5 +1,8 @@
 <h3><%= t(:label_version_plural) %></h3>
 
 <% @versions.each do |version| %>
-  <%= link_to format_version_name(version), "#{project_roadmap_url}##{version.name}" %><br />
+  <%= link_to format_version_name(version),
+              project_roadmap_path({ anchor: link_to_version_id(version) }.merge(params.permit(:completed,
+                                                                                               :with_subprojects,
+                                                                                               type_ids: []).to_h)) %><br />
 <% end %>

app/views/versions/index.html.erb

@@ -45,7 +45,7 @@ See docs/COPYRIGHT.rdoc for more details.
   <div id="roadmap">
     <% @versions.each do |version| %>
       <h3 class="version icon-context icon-modules">
-        <%= link_to_version version, name: h(version.name) %>
+        <%= link_to_version(version, name: h(version.name), id: "version-#{version.name}") %>
       </h3>
       <%= render partial: 'versions/overview', locals: {version: version} %>
       <%= render(partial: "wiki/content", locals: {content: version.wiki_page.content}) if version.wiki_page %>

modules/costs/spec/requests/api/attachments/attachments_by_budget_resource_spec.rb

@@ -68,8 +68,8 @@ describe 'API v3 Attachments by budget resource', type: :request do
     let(:permissions) { %i[view_budgets edit_budgets] }
 
     let(:request_path) { api_v3_paths.attachments_by_budget budget.id }
-    let(:request_parts) { { metadata: metadata, file: file } }
-    let(:metadata) { { fileName: 'cat.png' }.to_json }
+    let(:request_parts) { { metadata: metadata.to_json, file: file } }
+    let(:metadata) { { fileName: 'cat.png' } }
     let(:file) { mock_uploaded_file(name: 'original-filename.txt') }
     let(:max_file_size) { 1 } # given in kiB
 
@@ -99,19 +99,19 @@ describe 'API v3 Attachments by budget resource', type: :request do
     context 'file section is missing' do
       # rack-test won't send a multipart request without a file being present
       # however as long as we depend on correctly named sections this test should do just fine
-      let(:request_parts) { { metadata: metadata, wrongFileSection: file } }
+      let(:request_parts) { { metadata: metadata.to_json, wrongFileSection: file } }
 
       it_behaves_like 'invalid request body', I18n.t('api_v3.errors.multipart_body_error')
     end
 
     context 'metadata section is no valid JSON' do
-      let(:metadata) { '"fileName": "cat.png"' }
+      let(:request_parts) { { metadata: '"fileName": "cat.png"', file: file } }
 
       it_behaves_like 'parse error'
     end
 
     context 'metadata is missing the fileName' do
-      let(:metadata) { Hash.new.to_json }
+      let(:metadata) { Hash.new }
 
       it_behaves_like 'constraint violation' do
         let(:message) { "fileName #{I18n.t('activerecord.errors.messages.blank')}" }

modules/documents/spec/requests/api/v3/attachments/attachments_by_documents_resource_spec.rb

@@ -69,8 +69,8 @@ describe 'API v3 Attachments by document resource', type: :request do
     let(:permissions) { %i[view_documents manage_documents] }
 
     let(:request_path) { api_v3_paths.attachments_by_document document.id }
-    let(:request_parts) { { metadata: metadata, file: file } }
-    let(:metadata) { { fileName: 'cat.png' }.to_json }
+    let(:request_parts) { { metadata: metadata.to_json, file: file } }
+    let(:metadata) { { fileName: 'cat.png' } }
     let(:file) { mock_uploaded_file(name: 'original-filename.txt') }
     let(:max_file_size) { 1 } # given in kiB
 
@@ -100,19 +100,19 @@ describe 'API v3 Attachments by document resource', type: :request do
     context 'file section is missing' do
       # rack-test won't send a multipart request without a file being present
       # however as long as we depend on correctly named sections this test should do just fine
-      let(:request_parts) { { metadata: metadata, wrongFileSection: file } }
+      let(:request_parts) { { metadata: metadata.to_json, wrongFileSection: file } }
 
       it_behaves_like 'invalid request body', I18n.t('api_v3.errors.multipart_body_error')
     end
 
     context 'metadata section is no valid JSON' do
-      let(:metadata) { '"fileName": "cat.png"' }
+      let(:request_parts) { { metadata: '"fileName": "cat.png"', file: file } }
 
       it_behaves_like 'parse error'
     end
 
     context 'metadata is missing the fileName' do
-      let(:metadata) { Hash.new.to_json }
+      let(:metadata) { Hash.new }
 
       it_behaves_like 'constraint violation' do
         let(:message) { "fileName #{I18n.t('activerecord.errors.messages.blank')}" }

spec/helpers/versions_helper_spec.rb

@@ -59,7 +59,8 @@ describe VersionsHelper, type: :helper do
     context 'a version' do
       context 'with being allowed to see the version' do
         it 'does not create a link, without permission' do
-          expect(link_to_version(version)).to eq("#{test_project.name} - #{version.name}")
+          expect(link_to_version(version))
+            .to eq("#{test_project.name} - #{version.name}")
         end
       end
 
@@ -71,21 +72,22 @@ describe VersionsHelper, type: :helper do
         end
 
         it 'generates a link' do
-          expect(link_to_version(version)).to eq("<a href=\"/versions/#{version.id}\">#{test_project.name} - #{version.name}</a>")
+          expect(link_to_version(version))
+            .to be_html_eql("<a href=\"/versions/#{version.id}\" id=\"version-#{ERB::Util.url_encode(version.name)}\">#{test_project.name} - #{version.name}</a>")
         end
 
         it 'generates a link within a project' do
           @project = test_project
-          expect(link_to_version(version)).to eq("<a href=\"/versions/#{version.id}\">#{version.name}</a>")
+          expect(link_to_version(version))
+            .to be_html_eql("<a href=\"/versions/#{version.id}\" id=\"version-#{ERB::Util.url_encode(version.name)}\">#{version.name}</a>")
         end
       end
     end
 
-    describe 'an invalid version' do
-      let(:version) { Object }
-
-      it 'does not generate a link' do
-        expect(link_to_version(Object)).to be_empty
+    describe '#link_to_version_id' do
+      it 'generates an escaped id' do
+        expect(link_to_version_id(version))
+          .to eql("version-#{ERB::Util.url_encode(version.name)}")
       end
     end
   end

spec/requests/api/v3/attachments/attachment_resource_shared_examples.rb

@@ -42,8 +42,8 @@ shared_examples 'it supports direct uploads' do
   end
 
   describe 'POST /prepare', with_settings: { attachment_max_size: 512 } do
-    let(:request_parts) { { metadata: metadata, file: file } }
-    let(:metadata) { { fileName: 'cat.png', fileSize: file.size }.to_json }
+    let(:request_parts) { { metadata: metadata.to_json, file: file } }
+    let(:metadata) { { fileName: 'cat.png', fileSize: file.size, contentType: 'image/png' } }
     let(:file) { mock_uploaded_file(name: 'original-filename.txt') }
 
     def request!
@@ -68,7 +68,7 @@ shared_examples 'it supports direct uploads' do
       end
 
       context 'with no filesize metadata' do
-        let(:metadata) { { fileName: 'cat.png' }.to_json }
+        let(:metadata) { { fileName: 'cat.png' } }
 
         it 'should respond with 422 due to missing file size metadata' do
           expect(subject.status).to eq(422)
@@ -125,8 +125,21 @@ shared_examples 'it supports direct uploads' do
                   "success_action_status"
                 )
 
+                expect(fields["Content-Type"]).to eq metadata[:contentType]
+
                 expect(fields["key"]).to end_with "cat.png"
               end
+
+              it 'should also include the content type and the necessary policy in the form fields' do
+                fields = link["form_fields"]
+
+                expect(fields).to include("policy", "Content-Type")
+                expect(fields["Content-Type"]).to eq metadata[:contentType]
+
+                policy = Base64.decode64 fields["policy"]
+
+                expect(policy).to include '["starts-with","$Content-Type",""]'
+              end
             end
           end
         end
@@ -214,8 +227,8 @@ shared_examples 'an APIv3 attachment resource', type: :request, content_type: :j
     let(:permissions) { Array(update_permission) }
 
     let(:request_path) { api_v3_paths.attachments }
-    let(:request_parts) { { metadata: metadata, file: file } }
-    let(:metadata) { { fileName: 'cat.png' }.to_json }
+    let(:request_parts) { { metadata: metadata.to_json, file: file } }
+    let(:metadata) { { fileName: 'cat.png' } }
     let(:file) { mock_uploaded_file(name: 'original-filename.txt') }
     let(:max_file_size) { 1 } # given in kiB
 
@@ -248,19 +261,19 @@ shared_examples 'an APIv3 attachment resource', type: :request, content_type: :j
     context 'file section is missing' do
       # rack-test won't send a multipart request without a file being present
       # however as long as we depend on correctly named sections this test should do just fine
-      let(:request_parts) { { metadata: metadata, wrongFileSection: file } }
+      let(:request_parts) { { metadata: metadata.to_json, wrongFileSection: file } }
 
       it_behaves_like 'invalid request body', I18n.t('api_v3.errors.multipart_body_error')
     end
 
     context 'metadata section is no valid JSON' do
-      let(:metadata) { '"fileName": "cat.png"' }
+      let(:request_parts) { { metadata: '"fileName": "cat.png"', file: file } }
 
       it_behaves_like 'parse error'
     end
 
     context 'metadata is missing the fileName' do
-      let(:metadata) { Hash.new.to_json }
+      let(:metadata) { Hash.new }
 
       it_behaves_like 'constraint violation' do
         let(:message) { "fileName #{I18n.t('activerecord.errors.messages.blank')}" }
@@ -496,8 +509,8 @@ shared_examples 'an APIv3 attachment resource', type: :request, content_type: :j
 
     describe '#post' do
       let(:request_path) { api_v3_paths.send "attachments_by_#{attachment_type}", container.id }
-      let(:request_parts) { { metadata: metadata, file: file } }
-      let(:metadata) { { fileName: 'cat.png' }.to_json }
+      let(:request_parts) { { metadata: metadata.to_json, file: file } }
+      let(:metadata) { { fileName: 'cat.png' } }
       let(:file) { mock_uploaded_file(name: 'original-filename.txt') }
       let(:max_file_size) { 1 } # given in kiB
 
@@ -527,19 +540,19 @@ shared_examples 'an APIv3 attachment resource', type: :request, content_type: :j
       context 'file section is missing' do
         # rack-test won't send a multipart request without a file being present
         # however as long as we depend on correctly named sections this test should do just fine
-        let(:request_parts) { { metadata: metadata, wrongFileSection: file } }
+        let(:request_parts) { { metadata: metadata.to_json, wrongFileSection: file } }
 
         it_behaves_like 'invalid request body', I18n.t('api_v3.errors.multipart_body_error')
       end
 
       context 'metadata section is no valid JSON' do
-        let(:metadata) { '"fileName": "cat.png"' }
+        let(:request_parts) { { metadata: '"fileName": "cat.png"', file: file } }
 
         it_behaves_like 'parse error'
       end
 
       context 'metadata is missing the fileName' do
-        let(:metadata) { Hash.new.to_json }
+        let(:metadata) { Hash.new }
 
         it_behaves_like 'constraint violation' do
           let(:message) { "fileName #{I18n.t('activerecord.errors.messages.blank')}" }

spec/requests/api/v3/attachments_spec.rb

@@ -50,8 +50,8 @@ describe API::V3::Attachments::AttachmentsAPI, type: :request do
     let(:permissions) { [] }
 
     let(:request_path) { api_v3_paths.prepare_new_attachment_upload }
-    let(:request_parts) { { metadata: metadata, file: file } }
-    let(:metadata) { { fileName: 'cat.png' }.to_json }
+    let(:request_parts) { { metadata: metadata.to_json, file: file } }
+    let(:metadata) { { fileName: 'cat.png' } }
     let(:file) { mock_uploaded_file(name: 'original-filename.txt') }
 
     before do