Skip to content

Commit

Permalink
CV2-5388: Improve ToS and Privacy Policy UX (#2098)
Browse files Browse the repository at this point in the history
* CV2-5388: add flag for terms_last_updated_at notification

* CV2-5388: apply PR comment and fix tests

* CV2-5388: update the mails notification flag

* CV2-5388: disable dialog for google signin
  • Loading branch information
melsawy authored and caiosba committed Nov 7, 2024
1 parent bf92845 commit 051cab6
Show file tree
Hide file tree
Showing 5 changed files with 14 additions and 49 deletions.
1 change: 1 addition & 0 deletions app/models/concerns/user_multi_auth_login.rb
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ def self.create_omniauth_user(u, auth)
user.login = auth.info.nickname || auth.info.name.tr(' ', '-').downcase
user.from_omniauth_login = true
user.skip_confirmation!
user.last_accepted_terms_at = Time.now if user.last_accepted_terms_at.nil?
User.current = user
user.save!
user.confirm unless user.is_confirmed?
Expand Down
25 changes: 2 additions & 23 deletions app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -267,30 +267,9 @@ def accepted_terms
self.last_accepted_terms_at.to_i > User.terms_last_updated_at
end

def self.terms_last_updated_at_by_page(page)
mapping = {
tos: 'tos_url',
tos_smooch: 'tos_smooch_url',
privacy_policy: 'privacy_policy_url'
}.with_indifferent_access
return 0 unless mapping.has_key?(page)
Rails.cache.fetch("last_updated_at:#{page}", expires_in: 24.hours) do
html = URI(CheckConfig.get(mapping[page], nil, :json)).open(read_timeout: 5, open_timeout: 5).read
date = ActionController::Base.helpers.strip_tags(html).match(/Last modified(?<modified_date>(Jan(?:uary)?|Feb(?:ruary)?|Mar(?:ch)?|Apr(?:il)?|May|Jun(?:e)?|Jul(?:y)?|Aug(?:ust)?|Sep(?:tember)?|Oct(?:ober)?|Nov(?:ember)?|Dec(?:ember)?)\s\d{1,2},\s\d{2,4})/)["modified_date"]
Time.parse(date).to_i
end
end

def self.terms_last_updated_at
begin
tos = User.terms_last_updated_at_by_page(:tos)
pp = User.terms_last_updated_at_by_page(:privacy_policy)
tos > pp ? tos : pp
rescue StandardError => e
error = ToSOrPrivacyPolicyReadError.new(e)
CheckSentry.notify(error)
0
end
# Cached value should be an integer timestamp i.e Time.now.to_i
Rails.cache.read('terms_last_updated_at').to_i || 0
end

def self.delete_check_user(user)
Expand Down
2 changes: 1 addition & 1 deletion app/workers/terms_of_service_update_worker.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ class TermsOfServiceUpdateWorker
sidekiq_options queue: 'terms_mailer', retry: 0

def perform
last_updated = Time.at(User.terms_last_updated_at)
last_updated = Time.at(Rails.cache.read('terms_last_updated_at_notification').to_i)
updated_time = Time.now
# Based on our AWS SES account (Maximum send rate: 200 emails per second) I set a batch size 200 and do a sleep 1
User.where(is_active: true).where('email IS NOT NULL')
Expand Down
28 changes: 4 additions & 24 deletions test/models/user_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -791,35 +791,15 @@ def setup
end

test "should return the last time that the terms were updated" do
assert User.terms_last_updated_at > 0
end

test "should return the last time that the terms of service were updated" do
assert User.terms_last_updated_at_by_page(:tos) > 0
end

test "should return the last time that the terms of privacy were updated" do
assert User.terms_last_updated_at_by_page(:privacy_policy) > 0
end

test "should return the last time that invalid terms were updated" do
assert_equal 0, User.terms_last_updated_at_by_page(:invalid)
end

test "should not crash but notify if could not get the last time that the terms were updated" do
stub_configs({ 'tos_url' => 'invalid-tos-url' }) do
assert_nothing_raised do
assert_equal 0, User.terms_last_updated_at
end
end
assert_equal 0, User.terms_last_updated_at
time_now = Time.now.to_i
Rails.cache.write('terms_last_updated_at', time_now)
assert_equal time_now, User.terms_last_updated_at
end

test "should return if user accepted terms" do
u = create_user
assert !u.reload.accepted_terms
u.last_accepted_terms_at = Time.parse('2018-08-01')
u.save!
assert !u.reload.accepted_terms
u.last_accepted_terms_at = Time.now
u.save!
assert u.reload.accepted_terms
Expand Down
7 changes: 6 additions & 1 deletion test/workers/terms_of_service_update_worker_test.rb
Original file line number Diff line number Diff line change
@@ -1,12 +1,16 @@
require_relative '../test_helper'

class TermsOfServiceUpdateWorkerTest < ActiveSupport::TestCase
test "should notify users based on last term update" do
test "should notify users based on terms_last_updated_at_notification flag" do
User.stubs(:terms_last_updated_at).returns(Time.now.to_i)
terms_update = Time.now - 1.day
u = create_user
u.last_received_terms_email_at = terms_update
u.save!
last_received_terms_email_at = u.reload.last_received_terms_email_at
TermsOfServiceUpdateWorker.new.perform
assert_equal last_received_terms_email_at, u.reload.last_received_terms_email_at
Rails.cache.write('terms_last_updated_at_notification', Time.now.to_i)
TermsOfServiceUpdateWorker.new.perform
last_term_update = u.reload.last_received_terms_email_at
assert last_term_update > terms_update
Expand All @@ -15,6 +19,7 @@ class TermsOfServiceUpdateWorkerTest < ActiveSupport::TestCase
end

test "should notify users in background" do
Rails.cache.write('terms_last_updated_at_notification', Time.now.to_i)
User.stubs(:terms_last_updated_at).returns(Time.now.to_i)
terms_update = Time.now - 1.day
u = create_user
Expand Down

0 comments on commit 051cab6

Please sign in to comment.