diff --git a/app/models/spree/app_configuration.rb b/app/models/spree/app_configuration.rb index 847a067537..97f704eae3 100644 --- a/app/models/spree/app_configuration.rb +++ b/app/models/spree/app_configuration.rb @@ -105,7 +105,6 @@ module Spree preference :embedded_shopfronts_whitelist, :text, default: nil # Legal Preferences - preference :footer_tos_url, :string, default: "/Terms-of-service.pdf" preference :enterprises_require_tos, :boolean, default: false preference :shoppers_require_tos, :boolean, default: false preference :privacy_policy_url, :string, default: nil diff --git a/app/models/terms_of_service_file.rb b/app/models/terms_of_service_file.rb index 2fb35aab3c..8b9a36b263 100644 --- a/app/models/terms_of_service_file.rb +++ b/app/models/terms_of_service_file.rb @@ -11,11 +11,7 @@ class TermsOfServiceFile < ApplicationRecord end def self.current_url - if current - Rails.application.routes.url_helpers.url_for(current.attachment) - else - Spree::Config.footer_tos_url - end + Rails.application.routes.url_helpers.url_for(current.attachment) if current end # If no file has been uploaded, we don't know when the old terms have diff --git a/app/services/terms_of_service.rb b/app/services/terms_of_service.rb index 5a55010272..bae86c4eec 100644 --- a/app/services/terms_of_service.rb +++ b/app/services/terms_of_service.rb @@ -4,11 +4,11 @@ class TermsOfService def self.tos_accepted?(customer, distributor = nil) return false unless accepted_at = customer&.terms_and_conditions_accepted_at - accepted_at > if distributor - distributor.terms_and_conditions_blob.created_at - else - TermsOfServiceFile.updated_at - end + return accepted_at > distributor.terms_and_conditions_blob.created_at if distributor + + return true unless TermsOfServiceFile.exists? + + accepted_at > TermsOfServiceFile.updated_at end def self.required?(distributor) @@ -16,7 +16,8 @@ class TermsOfService end def self.platform_terms_required? - Spree::Config.shoppers_require_tos + TermsOfServiceFile.exists? && + Spree::Config.shoppers_require_tos end def self.distributor_terms_required?(distributor) 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 2f3c5b387b..924ed4c516 100644 --- a/app/views/admin/terms_of_service_files/show.html.haml +++ b/app/views/admin/terms_of_service_files/show.html.haml @@ -11,7 +11,6 @@ - else %p = t(".no_files") - = t(".using_default_terms_html", tos_link: link_to_platform_terms) = form_for [main_app, :admin, @new_file] do |f| = f.label :attachment, t(".attachment") diff --git a/app/views/layouts/mailer.html.haml b/app/views/layouts/mailer.html.haml index f559f67d62..0a70687f14 100644 --- a/app/views/layouts/mailer.html.haml +++ b/app/views/layouts/mailer.html.haml @@ -43,8 +43,9 @@ %tr %td{:align => "center"} %p - = link_to_platform_terms - | + - if platform_terms_required? + = link_to_platform_terms + | %a{:href => "#{ main_app.root_url }"} = Spree::Config[:site_name] / | Unsubscribe diff --git a/app/views/shared/_footer.html.haml b/app/views/shared/_footer.html.haml index bed768751c..5d121a4ea2 100644 --- a/app/views/shared/_footer.html.haml +++ b/app/views/shared/_footer.html.haml @@ -103,9 +103,10 @@ %img{src: ContentConfig.url_for(:footer_logo), width: "220"} .small-12.medium-5.columns.text-left %p.text-small - = t '.footer_legal_call' - = link_to_platform_terms - | + - if platform_terms_required? + = t '.footer_legal_call' + = link_to_platform_terms + | = t '.footer_legal_visit' %a{href:"https://github.com/openfoodfoundation/openfoodnetwork", target: "_blank"} GitHub %p.text-small diff --git a/config/locales/en.yml b/config/locales/en.yml index 9020961d32..4b40a6166e 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -645,7 +645,6 @@ en: show: title: "Terms of Service files" no_files: "No terms of services have been uploaded yet." - using_default_terms_html: "The site is currently linking to your old %{tos_link}." current_terms_html: "View the current %{tos_link}. Upload time: %{datetime}." terms_of_service: "Terms of Service" delete: "Delete file" diff --git a/spec/helpers/terms_and_conditions_helper_spec.rb b/spec/helpers/terms_and_conditions_helper_spec.rb index 4121725d2a..320426ea51 100644 --- a/spec/helpers/terms_and_conditions_helper_spec.rb +++ b/spec/helpers/terms_and_conditions_helper_spec.rb @@ -4,14 +4,29 @@ require 'spec_helper' describe TermsAndConditionsHelper, type: :helper do describe "#platform_terms_required?" do - it "returns true" do - expect(Spree::Config).to receive(:shoppers_require_tos).and_return(true) - expect(helper.platform_terms_required?).to eq true - end + context 'when ToS file is present' do + before do + allow(TermsOfServiceFile).to receive(:exists?).and_return(true) + end + it "returns true" do + expect(Spree::Config).to receive(:shoppers_require_tos).and_return(true) + expect(helper.platform_terms_required?).to eq true + end - it "returns false" do - expect(Spree::Config).to receive(:shoppers_require_tos).and_return(false) - expect(helper.platform_terms_required?).to eq false + it "returns false" do + expect(Spree::Config).to receive(:shoppers_require_tos).and_return(false) + expect(helper.platform_terms_required?).to eq false + end + end + context 'when ToS file is not present' do + before do + allow(TermsOfServiceFile).to receive(:exists?).and_return(false) + end + + it "returns false" do + expect(Spree::Config).not_to receive(:shoppers_require_tos) + expect(helper.platform_terms_required?).to eq false + end end end end diff --git a/spec/models/terms_of_service_file_spec.rb b/spec/models/terms_of_service_file_spec.rb index ca30c64546..21718a0e9b 100644 --- a/spec/models/terms_of_service_file_spec.rb +++ b/spec/models/terms_of_service_file_spec.rb @@ -24,8 +24,8 @@ describe TermsOfServiceFile do describe ".current_url" do let(:subject) { TermsOfServiceFile.current_url } - it "points to the old default" do - expect(subject).to eq "/Terms-of-service.pdf" + it "points to nil if not ToS file is present" do + expect(subject).to eq nil end it "points to a stored file" do diff --git a/spec/services/terms_of_service_spec.rb b/spec/services/terms_of_service_spec.rb index e6e78e4101..28ed199546 100644 --- a/spec/services/terms_of_service_spec.rb +++ b/spec/services/terms_of_service_spec.rb @@ -22,12 +22,25 @@ describe TermsOfService do allow(TermsOfServiceFile).to receive(:updated_at) { 2.weeks.ago } end - it "should reflect whether the platform TOS have been accepted since the last update" do - expect { - allow(TermsOfServiceFile).to receive(:updated_at) { Time.zone.now } - }.to change { - TermsOfService.tos_accepted?(customer) - }.from(true).to(false) + context "ToS file is present" do + it "should reflect whether the platform TOS have been accepted since the last update" do + expect { + allow(TermsOfServiceFile).to receive(:updated_at) { Time.zone.now } + allow(TermsOfServiceFile).to receive(:exists?) { true } + }.to change { + TermsOfService.tos_accepted?(customer) + }.from(true).to(false) + end + end + + context "ToS file is not present" do + it "should always return true" do + expect { + allow(TermsOfServiceFile).to receive(:exists?) { false } + }.to_not change { + TermsOfService.tos_accepted?(customer) + }.from(true) + end end end diff --git a/spec/system/admin/configuration/terms_of_service_files_spec.rb b/spec/system/admin/configuration/terms_of_service_files_spec.rb index fc3f0c6185..f917c54dbd 100644 --- a/spec/system/admin/configuration/terms_of_service_files_spec.rb +++ b/spec/system/admin/configuration/terms_of_service_files_spec.rb @@ -15,8 +15,6 @@ describe "Terms of Service files" do click_link "Terms of Service" expect(page).to have_content "No terms of services have been uploaded yet." - expect(page).to have_content "your old Terms of service" - expect(page).to have_link "Terms of service", href: "/Terms-of-service.pdf" end it "can be uploaded" do @@ -25,6 +23,7 @@ describe "Terms of Service files" do click_button "Create Terms of service file" expect(page).to have_link "Terms of Service" + expect(page).to have_link "Delete file" end it "provides Rails' standard action for a new file" do diff --git a/spec/system/consumer/split_checkout_spec.rb b/spec/system/consumer/split_checkout_spec.rb index 1ee0350fae..f805eb81bc 100644 --- a/spec/system/consumer/split_checkout_spec.rb +++ b/spec/system/consumer/split_checkout_spec.rb @@ -1029,13 +1029,16 @@ describe "As a consumer, I want to checkout my order" do context "when the platform's terms of service have to be accepted" do before do allow(Spree::Config).to receive(:shoppers_require_tos).and_return(true) - allow(Spree::Config).to receive(:footer_tos_url).and_return(tos_url) + end + + let!(:tos) do + TermsOfServiceFile.create!(attachment: system_terms) end it "shows the terms which need to be accepted" do visit checkout_step_path(:summary) - expect(page).to have_link "Terms of service", href: tos_url + expect(page).to have_link("Terms of service", href: /Terms-of-service.pdf/, count: 2) expect(find_link("Terms of service")[:target]).to eq "_blank" expect(page).to have_field "order_accept_terms", checked: false end @@ -1043,9 +1046,8 @@ describe "As a consumer, I want to checkout my order" do context "when the terms have been accepted in the past" do context "with a dedicated ToS file" do before do - TermsOfServiceFile.create!( - attachment: system_terms, - updated_at: 1.day.ago, + tos.update!( + updated_at: 1.day.ago ) customer.update(terms_and_conditions_accepted_at: Time.zone.now) end @@ -1080,14 +1082,17 @@ describe "As a consumer, I want to checkout my order" do order.distributor.update!(terms_and_conditions: shop_terms) allow(Spree::Config).to receive(:shoppers_require_tos).and_return(true) - allow(Spree::Config).to receive(:footer_tos_url).and_return(tos_url) + end + + let!(:tos) do + TermsOfServiceFile.create!(attachment: system_terms) end it "shows links to both terms and all need accepting" do visit checkout_step_path(:summary) expect(page).to have_link "Terms and Conditions", href: /#{shop_terms_path.basename}$/ - expect(page).to have_link "Terms of service", href: tos_url + expect(page).to have_link("Terms of service", href: /Terms-of-service.pdf/, count: 2) expect(page).to have_field "order_accept_terms", checked: false end end