diff --git a/app/models/terms_of_service_file.rb b/app/models/terms_of_service_file.rb index d652c37f7e..2fb35aab3c 100644 --- a/app/models/terms_of_service_file.rb +++ b/app/models/terms_of_service_file.rb @@ -1,11 +1,9 @@ # frozen_string_literal: true class TermsOfServiceFile < ApplicationRecord - include HasMigratingFile + has_one_attached :attachment - has_one_migrating :attachment - - validates :attachment, presence: true + validates :attachment, attached: true # The most recently uploaded file is the current one. def self.current @@ -13,7 +11,11 @@ class TermsOfServiceFile < ApplicationRecord end def self.current_url - current&.attachment&.url || Spree::Config.footer_tos_url + if current + Rails.application.routes.url_helpers.url_for(current.attachment) + else + Spree::Config.footer_tos_url + end end # If no file has been uploaded, we don't know when the old terms have @@ -21,9 +23,4 @@ class TermsOfServiceFile < ApplicationRecord def self.updated_at current&.updated_at || Time.zone.now end - - def touch(_) - # Ignore Active Storage changing the timestamp during migrations. - # This can be removed once we got rid of Paperclip. - end end diff --git a/app/views/admin/terms_of_service_files/show.html.haml b/app/views/admin/terms_of_service_files/show.html.haml index 94b7633c52..fd78d4e7ee 100644 --- a/app/views/admin/terms_of_service_files/show.html.haml +++ b/app/views/admin/terms_of_service_files/show.html.haml @@ -6,7 +6,7 @@ .admin-current-terms-of-service - if @current_file - %p= t(".current_terms_html", tos_link: link_to(t(".terms_of_service"), @current_file.attachment.url), datetime: @current_file.updated_at) + %p= t(".current_terms_html", tos_link: link_to(t(".terms_of_service"), @current_file.attachment), datetime: @current_file.updated_at) %p= link_to t(".delete"), main_app.admin_terms_of_service_files_path, method: "delete", data: { confirm: t(".confirm_delete") } - else %p diff --git a/spec/models/terms_of_service_file_spec.rb b/spec/models/terms_of_service_file_spec.rb index 7f638d0d69..74a28697a9 100644 --- a/spec/models/terms_of_service_file_spec.rb +++ b/spec/models/terms_of_service_file_spec.rb @@ -4,6 +4,7 @@ require 'spec_helper' describe TermsOfServiceFile do let(:pdf) { File.open(Rails.root.join("public/Terms-of-service.pdf")) } + let(:upload) { Rack::Test::UploadedFile.new(pdf, "application/pdf") } describe ".current" do it "returns nil" do @@ -12,8 +13,8 @@ describe TermsOfServiceFile do it "returns the last one" do existing = [ - TermsOfServiceFile.create!(attachment: pdf), - TermsOfServiceFile.create!(attachment: pdf), + TermsOfServiceFile.create!(attachment: upload), + TermsOfServiceFile.create!(attachment: upload), ] expect(TermsOfServiceFile.current).to eq existing.last @@ -27,10 +28,10 @@ describe TermsOfServiceFile do expect(subject).to eq "/Terms-of-service.pdf" end - it "points to the last uploaded file with timestamp parameter" do - file = TermsOfServiceFile.create!(attachment: pdf) + it "points to a stored file" do + file = TermsOfServiceFile.create!(attachment: upload) - expect(subject).to match %r{^/system/terms_of_service_files/attachments.*Terms-of-service\.pdf\?\d+$} + expect(subject).to match /active_storage.*Terms-of-service\.pdf$/ end end @@ -45,7 +46,8 @@ describe TermsOfServiceFile do it "returns the time when the terms were last updated" do update_time = 1.day.ago - file = TermsOfServiceFile.create!(attachment: pdf, updated_at: update_time) + file = TermsOfServiceFile.create!(attachment: upload) + file.update(updated_at: update_time) # The database isn't as precise as Ruby's time and rounds. expect(subject).to be_within(0.001).of(update_time)