Fix local storage protocol options for uploaders (decidim#9285)

* Fix local protocol options for the application uploader

* Add specs to test the storage protocol options for the uploaders

* Fix issues with the spec ports related to uploaders

* Avoid messing with the secrets hash

* Fix generator specs after application uploader refactoring

decidim-assemblies/spec/presenters/decidim/assemblies/assembly_presenter_spec.rb

@@ -26,11 +26,11 @@ module Decidim
 
     describe "when images are attached" do
       it "resolves hero_image_url" do
-        expect(subject.hero_image_url).to eq("http://#{organization_host}#{assembly.attached_uploader(:hero_image).path}")
+        expect(subject.hero_image_url).to eq("http://#{organization_host}:3000#{assembly.attached_uploader(:hero_image).path}")
       end
 
       it "resolves banner_image_url" do
-        expect(subject.banner_image_url).to eq("http://#{organization_host}#{assembly.attached_uploader(:banner_image).path}")
+        expect(subject.banner_image_url).to eq("http://#{organization_host}:3000#{assembly.attached_uploader(:banner_image).path}")
       end
     end
   end

decidim-core/app/uploaders/decidim/application_uploader.rb

@@ -48,9 +48,9 @@ def variant_url(key, options = {})
 
       representable = variant(key)
       if representable.is_a? ActiveStorage::Attached
-        Rails.application.routes.url_helpers.rails_blob_url(representable.blob, **protocol_option.merge(options))
+        Rails.application.routes.url_helpers.rails_blob_url(representable.blob, **protocol_options.merge(options))
       else
-        Rails.application.routes.url_helpers.rails_representation_url(representable, **protocol_option.merge(options))
+        Rails.application.routes.url_helpers.rails_representation_url(representable, **protocol_options.merge(options))
       end
     end
 
@@ -74,11 +74,27 @@ def remote_url=(url)
       model.errors.add(mounted_as, :invalid)
     end
 
-    def protocol_option
+    def protocol_options
       return { host: cdn_host } if cdn_host
-      return {} unless Rails.application.config.force_ssl
 
-      { protocol: "https" }
+      local_protocol_options
+    end
+
+    def local_protocol_options
+      @local_protocol_options ||= {}.tap do |options|
+        default_port =
+          if Rails.env.development? || Rails.env.test?
+            3000
+          elsif Rails.application.config.force_ssl
+            443
+          else
+            80
+          end
+        port = ENV.fetch("PORT", default_port)
+
+        options[:port] = port unless [443, 80].include?(port)
+        options[:protocol] = "https" if Rails.application.config.force_ssl || port == 443
+      end
     end
 
     def cdn_host

decidim-core/lib/decidim/core/test/shared_examples/logo_email.rb

@@ -15,7 +15,7 @@
 
     it "includes organization logo with full link" do
       expect(mail.body).to include("alt=\"#{organization.name}\"")
-      expect(mail.body).to match(%r{https{0,1}://#{organization.host}#{logo_path}})
+      expect(mail.body).to match(%r{https{0,1}://#{organization.host}:3000#{logo_path}})
     end
   end
 end

decidim-core/spec/uploaders/application_uploader_spec.rb

@@ -0,0 +1,143 @@
+# frozen_string_literal: true
+
+require "spec_helper"
+
+module Decidim
+  describe ApplicationUploader do
+    subject { described_class.new(model, mounted_as) }
+
+    let(:model) { create(:organization) }
+    let(:mounted_as) { :open_data_file }
+
+    describe "#protocol_options" do
+      shared_context "with force_ssl enabled" do
+        before do
+          allow(Rails.application.config).to receive(:force_ssl).and_return(true)
+        end
+      end
+
+      shared_context "with PORT environment variable" do
+        before do
+          allow(ENV).to receive(:fetch).with("PORT", instance_of(Integer)).and_return(local_port)
+        end
+      end
+
+      shared_examples "development local storage protocol options" do
+        it "returns a hash containing the port only" do
+          expect(subject.protocol_options).to eq({ port: 3000 })
+        end
+
+        context "when the PORT environment variable is defined as 80" do
+          include_context "with PORT environment variable"
+
+          let(:local_port) { 80 }
+
+          it "returns an empty hash" do
+            expect(subject.protocol_options).to eq({})
+          end
+        end
+
+        context "when force_ssl is enabled" do
+          include_context "with force_ssl enabled"
+
+          it "returns a hash containing the port and protocol" do
+            expect(subject.protocol_options).to eq({ port: 3000, protocol: "https" })
+          end
+
+          context "and the PORT environment variable is defined as 3001" do
+            include_context "with PORT environment variable"
+
+            let(:local_port) { 3001 }
+
+            it "returns a hash containing the port and protocol" do
+              expect(subject.protocol_options).to eq({ port: 3001, protocol: "https" })
+            end
+          end
+
+          context "and the PORT environment variable is defined as 443" do
+            include_context "with PORT environment variable"
+
+            let(:local_port) { 443 }
+
+            it "returns a hash containing the protocol only" do
+              expect(subject.protocol_options).to eq({ protocol: "https" })
+            end
+          end
+        end
+      end
+
+      context "with test environment" do
+        it_behaves_like "development local storage protocol options"
+      end
+
+      context "with development environment" do
+        before do
+          allow(Rails.env).to receive(:development?).and_return(true)
+        end
+
+        it_behaves_like "development local storage protocol options"
+      end
+
+      context "with production environment" do
+        before do
+          allow(Rails.env).to receive(:development?).and_return(false)
+          allow(Rails.env).to receive(:test?).and_return(false)
+        end
+
+        it "returns an empty hash by default" do
+          expect(subject.protocol_options).to eq({})
+        end
+
+        context "and the PORT environment variable is defined as 443" do
+          include_context "with PORT environment variable"
+
+          let(:local_port) { 443 }
+
+          it "returns a hash containing the protocol only" do
+            expect(subject.protocol_options).to eq({ protocol: "https" })
+          end
+        end
+
+        context "and the PORT environment variable is defined as 8080" do
+          include_context "with PORT environment variable"
+
+          let(:local_port) { 8080 }
+
+          it "returns a hash containing the port" do
+            expect(subject.protocol_options).to eq({ port: 8080 })
+          end
+        end
+
+        context "and force_ssl is enabled" do
+          include_context "with force_ssl enabled"
+
+          it "returns a hash containing the protocol only" do
+            expect(subject.protocol_options).to eq({ protocol: "https" })
+          end
+
+          context "and the PORT environment variable is defined as 8080" do
+            include_context "with PORT environment variable"
+
+            let(:local_port) { 8080 }
+
+            it "returns a hash containing the port and protocol" do
+              expect(subject.protocol_options).to eq({ port: 8080, protocol: "https" })
+            end
+          end
+        end
+      end
+
+      context "with CDN host defined in the secrets" do
+        let(:cdn_host) { "https://cdn.example.org" }
+
+        before do
+          allow(Rails.application.secrets).to receive(:dig).with(:storage, :cdn_host).and_return(cdn_host)
+        end
+
+        it "returns a hash containing the host only" do
+          expect(subject.protocol_options).to eq({ host: "https://cdn.example.org" })
+        end
+      end
+    end
+  end
+end

decidim-elections/spec/presenters/decidim/votings/voting_presenter_spec.rb

@@ -26,11 +26,11 @@ module Decidim
 
     describe "when images are attached" do
       it "resolves introductory_image_url" do
-        expect(subject.introductory_image_url).to eq("http://#{organization_host}#{voting.attached_uploader(:introductory_image).path}")
+        expect(subject.introductory_image_url).to eq("http://#{organization_host}:3000#{voting.attached_uploader(:introductory_image).path}")
       end
 
       it "resolves banner_image_url" do
-        expect(subject.banner_image_url).to eq("http://#{organization_host}#{voting.attached_uploader(:banner_image).path}")
+        expect(subject.banner_image_url).to eq("http://#{organization_host}:3000#{voting.attached_uploader(:banner_image).path}")
       end
     end
   end

decidim-generators/lib/decidim/generators/test/generator_examples.rb

@@ -656,7 +656,7 @@
       "Rails.application.config.log_level" => "info",
       "Rails.application.config.action_controller.asset_host" => nil,
       "Rails.application.config.active_storage.service" => "local",
-      "Decidim::ApplicationUploader.new(nil, :file).protocol_option" => { "protocol" => "https" }
+      "Decidim::ApplicationUploader.new(nil, :file).protocol_options" => { "protocol" => "https" }
     }
   end
 
@@ -666,7 +666,7 @@
       "Rails.application.config.log_level" => "fatal",
       "Rails.application.config.action_controller.asset_host" => "http://assets.example.org",
       "Rails.application.config.active_storage.service" => "test",
-      "Decidim::ApplicationUploader.new(nil, :file).protocol_option" => { "host" => "https://cdn.example.org" },
+      "Decidim::ApplicationUploader.new(nil, :file).protocol_options" => { "host" => "https://cdn.example.org" },
       "Decidim::Api::Schema.default_max_page_size" => 31,
       "Decidim::Api::Schema.max_complexity" => 3001,
       "Decidim::Api::Schema.max_depth" => 11