diff --git a/app/controllers/accounts/terms_controller.rb b/app/controllers/accounts/terms_controller.rb
index 568abcdb9..de8b8dbe7 100644
--- a/app/controllers/accounts/terms_controller.rb
+++ b/app/controllers/accounts/terms_controller.rb
@@ -18,11 +18,6 @@ module Accounts
render :partial => "terms"
else
@title = t ".title"
-
- if current_user.terms_agreed?
- # Already agreed to terms, so just show settings
- redirect_to account_path
- end
end
end
@@ -32,13 +27,11 @@ module Accounts
flash[:notice] = { :partial => "accounts/terms/terms_declined_flash" } if current_user.save
else
- unless current_user.terms_agreed?
- current_user.tou_agreed = Time.now.utc
- current_user.terms_agreed = Time.now.utc
- current_user.terms_seen = true
+ current_user.tou_agreed = Time.now.utc
+ current_user.terms_agreed = Time.now.utc
+ current_user.terms_seen = true
- flash[:notice] = t ".terms accepted" if current_user.save
- end
+ flash[:notice] = t ".terms accepted" if current_user.save
end
referer = safe_referer(params[:referer]) if params[:referer]
diff --git a/app/views/accounts/show.html.erb b/app/views/accounts/show.html.erb
index 5c626fc9f..1643a8adf 100644
--- a/app/views/accounts/show.html.erb
+++ b/app/views/accounts/show.html.erb
@@ -29,26 +29,6 @@
(" target="_new"><%= t ".openid.link text" %>)
-
<%= f.primary t(".save changes button") %>
@@ -57,8 +37,49 @@
<%= link_to t(".delete_account"), account_deletion_path, :class => "btn btn-outline-danger" %>
+
<% end %>
+
+
+ <%= t ".contributor_terms.heading" %>
+
+ <% if current_user.terms_agreed? %>
+ <%= t ".contributor_terms.agreed", :date => l(@current_user.terms_agreed.to_date, :format => :long) %>
+ <% else %>
+ <%= t ".contributor_terms.not_agreed" %>
+ <% end %>
+
+ <% if current_user.consider_pd? %>
+ <%= t ".contributor_terms.agreed_with_pd" %>
+ <% else %>
+ <%= t ".contributor_terms.not_agreed_with_pd" %>
+ <% end %>
+
+ <%= link_to current_user.terms_agreed? ? t(".contributor_terms.review") : t(".contributor_terms.review_and_accept"), account_terms_path %>
+ <% unless current_user.consider_pd? %>
+ ·
+ <%= link_to t(".contributor_terms.consider_pd"), account_pd_declaration_path %>
+ <% end %>
+
+
+
+
+ <%= t ".terms_of_use.heading" %>
+
+ <% if current_user.tou_agreed? %>
+ <%= t ".terms_of_use.agreed", :date => l(@current_user.tou_agreed.to_date, :format => :long) %>
+ <% else %>
+ <%= t ".terms_of_use.not_agreed" %>
+ <% end %>
+
+ <%= link_to current_user.tou_agreed? ? t(".terms_of_use.review") : t(".terms_of_use.review_and_accept"), account_terms_path %>
+
+
+
+
<% unless current_user.data_public? %>
<%= render :partial => "go_public" %>
<% end %>
diff --git a/config/locales/en.yml b/config/locales/en.yml
index bc2f3b1b3..d40897ecd 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -252,16 +252,21 @@ en:
openid:
link: "https://wiki.openstreetmap.org/wiki/OpenID"
link text: "what is this?"
- contributor terms:
+ contributor_terms:
heading: "Contributor Terms"
- agreed: "You have agreed to the new Contributor Terms."
- not yet agreed: "You have not yet agreed to the new Contributor Terms."
- review link text: "Please follow this link at your convenience to review and accept the new Contributor Terms."
+ agreed: "You have agreed to the Contributor Terms on %{date}."
+ not_agreed: "You have not yet agreed to the Contributor Terms."
agreed_with_pd: "You have also declared that you consider your edits to be in the Public Domain."
- link: "https://osmfoundation.org/wiki/Licence/Contributor_Terms"
- link text: "what is this?"
not_agreed_with_pd: "You haven't declared that you consider your edits to be in the Public Domain."
- pd_link_text: "declare"
+ review: "Review the Terms"
+ review_and_accept: "Review and accept the Terms"
+ consider_pd: "Consider Public Domain"
+ terms_of_use:
+ heading: "Terms of Use"
+ agreed: "You have agreed to the Terms of Use on %{date}."
+ not_agreed: "You have not yet agreed to the Terms of Use."
+ review: "Review the Terms"
+ review_and_accept: "Review and accept the Terms"
save changes button: Save Changes
delete_account: Delete Account...
go_public:
diff --git a/test/controllers/accounts/terms_controller_test.rb b/test/controllers/accounts/terms_controller_test.rb
index ea8cd4e9f..4e9fbed25 100644
--- a/test/controllers/accounts/terms_controller_test.rb
+++ b/test/controllers/accounts/terms_controller_test.rb
@@ -29,7 +29,7 @@ module Accounts
session_for(user)
get account_terms_path
- assert_redirected_to account_path
+ assert_response :success
end
def test_show_not_seen_without_referer
@@ -48,32 +48,72 @@ module Accounts
assert_response :success
end
- def test_update_not_seen_without_referer
- user = create(:user, :terms_seen => false, :terms_agreed => nil)
+ def test_update_decline
+ user = create(:user, :terms_seen => false, :terms_agreed => nil, :tou_agreed => nil)
session_for(user)
- put account_terms_path, :params => { :read_ct => 1, :read_tou => 1 }
- assert_redirected_to account_path
- assert_equal "Thanks for accepting the new contributor terms!", flash[:notice]
+ freeze_time do
+ put account_terms_path
- user.reload
+ assert_redirected_to account_path
+ assert_equal({ :partial => "accounts/terms/terms_declined_flash" }, flash[:notice])
- assert_not_nil user.terms_agreed
- assert user.terms_seen
+ user.reload
+ assert user.terms_seen
+ assert_nil user.terms_agreed
+ assert_nil user.tou_agreed
+ end
+ end
+
+ def test_update_not_seen
+ user = create(:user, :terms_seen => false, :terms_agreed => nil, :tou_agreed => nil)
+ session_for(user)
+
+ freeze_time do
+ put account_terms_path, :params => { :read_ct => 1, :read_tou => 1 }
+
+ assert_redirected_to account_path
+ assert_equal "Thanks for accepting the new contributor terms!", flash[:notice]
+
+ user.reload
+ assert user.terms_seen
+ assert_equal Time.now.utc, user.terms_agreed
+ assert_equal Time.now.utc, user.tou_agreed
+ end
end
def test_update_not_seen_with_referer
- user = create(:user, :terms_seen => false, :terms_agreed => nil)
+ user = create(:user, :terms_seen => false, :terms_agreed => nil, :tou_agreed => nil)
session_for(user)
- put account_terms_path, :params => { :referer => "/test", :read_ct => 1, :read_tou => 1 }
- assert_redirected_to "/test"
- assert_equal "Thanks for accepting the new contributor terms!", flash[:notice]
+ freeze_time do
+ put account_terms_path, :params => { :referer => "/test", :read_ct => 1, :read_tou => 1 }
- user.reload
+ assert_redirected_to "/test"
+ assert_equal "Thanks for accepting the new contributor terms!", flash[:notice]
- assert_not_nil user.terms_agreed
- assert user.terms_seen
+ user.reload
+ assert user.terms_seen
+ assert_equal Time.now.utc, user.terms_agreed
+ assert_equal Time.now.utc, user.tou_agreed
+ end
+ end
+
+ def test_update_previously_accepted
+ user = create(:user, :terms_seen => true, :terms_agreed => Date.yesterday, :tou_agreed => Date.yesterday)
+ session_for(user)
+
+ freeze_time do
+ put account_terms_path, :params => { :read_ct => 1, :read_tou => 1 }
+
+ assert_redirected_to account_path
+ assert_equal "Thanks for accepting the new contributor terms!", flash[:notice]
+
+ user.reload
+ assert user.terms_seen
+ assert_equal Time.now.utc, user.terms_agreed
+ assert_equal Time.now.utc, user.tou_agreed
+ end
end
# Check that if you haven't seen the terms, and make a request that requires authentication,
diff --git a/test/system/account_terms_test.rb b/test/system/account_terms_test.rb
new file mode 100644
index 000000000..35b2f9193
--- /dev/null
+++ b/test/system/account_terms_test.rb
@@ -0,0 +1,83 @@
+require "application_system_test_case"
+
+class AccountTermsTest < ApplicationSystemTestCase
+ test "should inform about terms if not agreed" do
+ user = create(:user, :terms_seen => true, :terms_agreed => nil, :tou_agreed => nil)
+
+ sign_in_as(user)
+ visit account_path
+
+ within_content_body do
+ assert_text(/You have not yet agreed to.*Contributor Terms/)
+ assert_text(/You have not yet agreed to.*Terms of Use/)
+ assert_link "Review and accept the Terms"
+
+ click_on "Review and accept the Terms", :match => :first
+ end
+
+ assert_current_path account_terms_path
+ end
+
+ test "should inform about terms if partially agreed" do
+ user = create(:user, :terms_seen => true, :terms_agreed => "2022-03-14", :tou_agreed => nil)
+
+ sign_in_as(user)
+ visit account_path
+
+ within_content_body do
+ assert_text(/You have agreed to.*Contributor Terms.*March 14, 2022/)
+ assert_text(/You have not yet agreed to.*Terms of Use/)
+ assert_link "Review the Terms"
+ assert_link "Review and accept the Terms"
+
+ click_on "Review and accept the Terms", :match => :first
+ end
+
+ assert_current_path account_terms_path
+ end
+
+ test "should inform about terms if agreed" do
+ user = create(:user, :terms_seen => true, :terms_agreed => "2023-04-15", :tou_agreed => "2024-05-16")
+
+ sign_in_as(user)
+ visit account_path
+
+ within_content_body do
+ assert_text(/You have agreed to.*Contributor Terms.*April 15, 2023/)
+ assert_text(/You have agreed to.*Terms of Use.*May 16, 2024/)
+ assert_link "Review the Terms"
+
+ click_on "Review the Terms", :match => :first
+ end
+
+ assert_current_path account_terms_path
+ end
+
+ test "should ask to consider pd if not considered" do
+ user = create(:user, :consider_pd => false)
+
+ sign_in_as(user)
+ visit account_path
+
+ within_content_body do
+ assert_text(/You haven't declared.*Public Domain/)
+ assert_link "Consider Public Domain"
+
+ click_on "Consider Public Domain"
+ end
+
+ assert_current_path account_pd_declaration_path
+ end
+
+ test "should not ask to consider pd if considered" do
+ user = create(:user, :consider_pd => true)
+
+ sign_in_as(user)
+ visit account_path
+
+ within_content_body do
+ assert_text(/You have also declared.*Public Domain/)
+ assert_no_link "Consider Public Domain"
+ end
+ end
+end