From af314cbbdf70dc232ee7f526e731b048d76f8876 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Fri, 23 Apr 2021 15:35:16 +1000 Subject: [PATCH] Allow fast checkout without re-accepting old terms We did that for the shop's terms already and now do it for the platform terms as well. --- app/helpers/terms_and_conditions_helper.rb | 13 ++++++++++ app/models/terms_of_service_file.rb | 6 +++++ .../_all_terms_and_conditions.html.haml | 2 +- .../_platform_terms_of_service.html.haml | 2 +- .../consumer/shopping/checkout_spec.rb | 26 +++++++++++++++++++ spec/models/terms_of_service_file_spec.rb | 18 +++++++++++++ 6 files changed, 65 insertions(+), 2 deletions(-) diff --git a/app/helpers/terms_and_conditions_helper.rb b/app/helpers/terms_and_conditions_helper.rb index 59a9e5cdb2..70925156e5 100644 --- a/app/helpers/terms_and_conditions_helper.rb +++ b/app/helpers/terms_and_conditions_helper.rb @@ -23,6 +23,19 @@ module TermsAndConditionsHelper current_order.distributor.terms_and_conditions.file? end + def all_terms_and_conditions_already_accepted? + platform_tos_already_accepted? && terms_and_conditions_already_accepted? + end + + def platform_tos_already_accepted? + customer_terms_and_conditions_accepted_at = spree_current_user&. + customer_of(current_order.distributor)&.terms_and_conditions_accepted_at + + customer_terms_and_conditions_accepted_at.present? && + (customer_terms_and_conditions_accepted_at > + TermsOfServiceFile.updated_at) + end + def terms_and_conditions_already_accepted? customer_terms_and_conditions_accepted_at = spree_current_user&. customer_of(current_order.distributor)&.terms_and_conditions_accepted_at diff --git a/app/models/terms_of_service_file.rb b/app/models/terms_of_service_file.rb index 418a93ca71..471bdb8ca3 100644 --- a/app/models/terms_of_service_file.rb +++ b/app/models/terms_of_service_file.rb @@ -13,4 +13,10 @@ class TermsOfServiceFile < ApplicationRecord def self.current_url current&.attachment&.url || Spree::Config.footer_tos_url end + + # If no file has been uploaded, we don't know when the old terms have + # been updated last. So we return the most recent possible update time. + def self.updated_at + current&.updated_at || Time.zone.now + end end diff --git a/app/views/checkout/_all_terms_and_conditions.html.haml b/app/views/checkout/_all_terms_and_conditions.html.haml index 0a21a0dbda..5a90b15cfa 100644 --- a/app/views/checkout/_all_terms_and_conditions.html.haml +++ b/app/views/checkout/_all_terms_and_conditions.html.haml @@ -1,4 +1,4 @@ %p - %input{ type: 'checkbox', id: 'accept_terms', ng: { model: "terms_and_conditions_accepted", init: "terms_and_conditions_accepted=false" } } + %input{ type: 'checkbox', id: 'accept_terms', ng: { model: "terms_and_conditions_accepted", init: "terms_and_conditions_accepted = #{all_terms_and_conditions_already_accepted?}" } } %label.small{for: "accept_terms"} = t('.message_html', terms_and_conditions_link: link_to( t(".terms_and_conditions"), current_order.distributor.terms_and_conditions.url, target: '_blank'), tos_link: link_to_platform_terms) diff --git a/app/views/checkout/_platform_terms_of_service.html.haml b/app/views/checkout/_platform_terms_of_service.html.haml index 3d46cdd6ee..8f2cb03096 100644 --- a/app/views/checkout/_platform_terms_of_service.html.haml +++ b/app/views/checkout/_platform_terms_of_service.html.haml @@ -1,4 +1,4 @@ %p - %input{ type: "checkbox", id: "platform_tos_accepted", ng: { model: "platform_tos_accepted", init: "platform_tos_accepted = false" } } + %input{ type: "checkbox", id: "platform_tos_accepted", ng: { model: "platform_tos_accepted", init: "platform_tos_accepted = #{platform_tos_already_accepted?}" } } %label.small{for: "platform_tos_accepted"} = t(".message_html", tos_link: link_to_platform_terms) diff --git a/spec/features/consumer/shopping/checkout_spec.rb b/spec/features/consumer/shopping/checkout_spec.rb index 50a92b7a5f..373e2096d0 100644 --- a/spec/features/consumer/shopping/checkout_spec.rb +++ b/spec/features/consumer/shopping/checkout_spec.rb @@ -200,6 +200,32 @@ feature "As a consumer I want to check out my cart", js: true do uncheck "Terms of service" expect(page).to have_button("Place order now", disabled: true) end + + context "when the terms have been accepted in the past" do + before do + TermsOfServiceFile.create!( + attachment: File.open(Rails.root.join("public/Terms-of-service.pdf")), + updated_at: 1.day.ago, + ) + customer = create(:customer, enterprise: order.distributor, user: user) + customer.update(terms_and_conditions_accepted_at: Time.zone.now) + end + + it "remembers the acceptance" do + visit checkout_path + + within "#checkout_form" do + expect(page).to have_link("Terms of service") + expect(page).to have_button("Place order now", disabled: false) + end + + uncheck "Terms of service" + expect(page).to have_button("Place order now", disabled: true) + + check "Terms of service" + expect(page).to have_button("Place order now", disabled: false) + end + end end context "when the seller's terms and the platform's terms have to be accepted" do diff --git a/spec/models/terms_of_service_file_spec.rb b/spec/models/terms_of_service_file_spec.rb index 3cff542b3a..16acf2a16a 100644 --- a/spec/models/terms_of_service_file_spec.rb +++ b/spec/models/terms_of_service_file_spec.rb @@ -33,4 +33,22 @@ describe TermsOfServiceFile do expect(subject).to match /^\/system\/terms_of_service_files\/attachments.*Terms-of-service\.pdf\?\d+$/ end end + + describe ".updated_at" do + let(:subject) { TermsOfServiceFile.updated_at } + + it "gives the most conservative time if not known" do + Timecop.freeze do + expect(subject).to eq Time.zone.now + end + end + + 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) + + # The database isn't as precise as Ruby's time and rounds. + expect(subject).to be_within(0.001).of(update_time) + end + end end