From d83ec1a0271d33a0157bb959f4b58b3cd7c43ccc Mon Sep 17 00:00:00 2001 From: John Ma Date: Fri, 14 Sep 2018 14:33:58 -0400 Subject: [PATCH] Fix #260 issues with privacy policy (#261) * * * * * * * * * * * * * * --- app/controllers/users_controller.rb | 11 +++++------ app/models/user.rb | 3 +++ .../shared/components/_terms_button.html.erb | 18 ++++++++++++++++++ app/views/users/new.html.erb | 5 +++++ app/views/users/terms.html.erb | 6 +++--- config/locales/en.yml | 7 ++++++- config/routes.rb | 7 ++++--- spec/controllers/users_controller_spec.rb | 4 +++- spec/factories.rb | 1 + spec/models/user_spec.rb | 2 ++ 10 files changed, 50 insertions(+), 14 deletions(-) create mode 100644 app/views/shared/components/_terms_button.html.erb diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index b7565e04..28d9fa80 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -97,13 +97,11 @@ class UsersController < ApplicationController redirect_to root_path end - # GET /u/terms + # GET /terms def terms - redirect_to root_path unless current_user - if params[:accept] == "true" - current_user.update_attribute(accepted_terms: true) - redirect_to current_user.main_room + current_user.update_attributes(accepted_terms: true) + redirect_to current_user.main_room if current_user end end @@ -118,6 +116,7 @@ class UsersController < ApplicationController end def user_params - params.require(:user).permit(:name, :email, :image, :password, :password_confirmation, :new_password, :provider) + params.require(:user).permit(:name, :email, :image, :password, :password_confirmation, + :new_password, :provider, :accepted_terms) end end diff --git a/app/models/user.rb b/app/models/user.rb index f71699a8..06b2b81a 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -34,6 +34,9 @@ class User < ApplicationRecord validates :password, length: { minimum: 6 }, confirmation: true, if: :greenlight_account?, on: :create + # Bypass validation if omniauth + validates :accepted_terms, acceptance: true, unless: proc { !greenlight_account? } + # We don't want to require password validations on all accounts. has_secure_password(validations: false) diff --git a/app/views/shared/components/_terms_button.html.erb b/app/views/shared/components/_terms_button.html.erb new file mode 100644 index 00000000..de46d390 --- /dev/null +++ b/app/views/shared/components/_terms_button.html.erb @@ -0,0 +1,18 @@ +<% +# BigBlueButton open source conferencing system - http://www.bigbluebutton.org/. +# Copyright (c) 2018 BigBlueButton Inc. and by respective authors (see below). +# This program is free software; you can redistribute it and/or modify it under the +# terms of the GNU Lesser General Public License as published by the Free Software +# Foundation; either version 3.0 of the License, or (at your option) any later +# version. +# +# BigBlueButton is distributed in the hope that it will be useful, but WITHOUT ANY +# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A +# PARTICULAR PURPOSE. See the GNU Lesser General Public License for more details. +# You should have received a copy of the GNU Lesser General Public License along +# with BigBlueButton; if not, see . +%> + +
+ <%= button_to t("terms.accept_existing"), terms_path, params: { accept: true }, class: "btn btn-primary btn-space" %> +
\ No newline at end of file diff --git a/app/views/users/new.html.erb b/app/views/users/new.html.erb index b2c3a48e..52fe644e 100644 --- a/app/views/users/new.html.erb +++ b/app/views/users/new.html.erb @@ -57,6 +57,11 @@ <%= f.password_field :password_confirmation, class: "form-control #{form_is_invalid?(@user, :password_confirmation)}", placeholder: t("signup.password_confirm") %>
<%= @user.errors.full_messages_for(:password_confirmation).first %>
+
+ <%= f.check_box :accepted_terms, class: "form-control #{form_is_invalid?(@user, :accepted_terms)}", placeholder: t("signup.password_confirm") %> + <%= f.label :accepted_terms, t("terms.accept", href: link_to(t("terms.title"), terms_path, target: "_blank", class: "ml-1 text-blue")).html_safe, class: "ml-1" %> +
<%= @user.errors.full_messages_for(:accepted_terms).first %>
+
<%= f.submit t("signup.title"), class: "btn btn-primary float-right ml-2" %> <%= link_to t("cancel"), root_path, class: "btn btn-secondary float-right ml-2" %> diff --git a/app/views/users/terms.html.erb b/app/views/users/terms.html.erb index c8ff6e8a..488d3009 100644 --- a/app/views/users/terms.html.erb +++ b/app/views/users/terms.html.erb @@ -23,9 +23,9 @@
<%= markdown(Rails.configuration.terms) %>
-
- <%= button_to t("terms.accept"), terms_path, params: {accept: true}, class: "btn btn-primary btn-space" %> -
+ <% if Rails.configuration.terms && current_user && !current_user.accepted_terms %> + <%= render "/shared/components/terms_button" %> + <% end %>
diff --git a/config/locales/en.yml b/config/locales/en.yml index 0d9af608..fecb64dc 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -17,6 +17,10 @@ # English (en) locale. en: + activerecord: + attributes: + user: + accepted_terms: "Terms and Conditions" bigbluebutton: BigBlueButton cancel: Cancel copy: Copy @@ -152,7 +156,8 @@ en: title: Signup with: Signup with %{provider} terms: - accept: I accept the terms and conditions. + accept: I accept the %{href} + accept_existing: I accept the terms and conditions title: Terms and Conditions test_install: > This deployment is using a pre-configured testing server, you should replace this with your own. diff --git a/config/routes.rb b/config/routes.rb index 87fbd34e..66b7d111 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -26,12 +26,13 @@ Rails.application.routes.draw do get '/signup', to: 'users#new', as: :signup post '/signup', to: 'users#create', as: :create_user + # Redirect to terms page + match '/terms', to: 'users#terms', via: [:get, :post] + # User resources. scope '/u' do - match '/terms', to: 'users#terms', via: [:get, :post] - # Handles login of greenlight provider accounts. - post '/login', to: 'sessions#create', as: :create_session + post '/login', to: 'sessions#create', as: :create_session # Log the user out of the session. get '/logout', to: 'sessions#destroy' diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb index e546903f..fd1c18b2 100644 --- a/spec/controllers/users_controller_spec.rb +++ b/spec/controllers/users_controller_spec.rb @@ -26,6 +26,7 @@ def random_valid_user_params email: Faker::Internet.email, password: pass, password_confirmation: pass, + accepted_terms: true, }, } end @@ -37,7 +38,8 @@ describe UsersController, type: :controller do name: "Invalid", email: "example.com", password: "pass", - passwrd_confirmation: "invalid", + password_confirmation: "invalid", + accepted_terms: false, }, } end diff --git a/spec/factories.rb b/spec/factories.rb index 54bc9586..7d4cd00c 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -27,6 +27,7 @@ FactoryBot.define do email { Faker::Internet.email } password { password } password_confirmation { password } + accepted_terms { true } end factory :room do diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 6677a740..33fffc04 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -34,6 +34,8 @@ describe User, type: :model do it { should allow_value("", nil).for(:email) } it { should allow_value("valid@email.com").for(:email) } it { should_not allow_value("invalid_email").for(:email) } + it { should allow_value(true).for(:accepted_terms) } + it { should allow_value(false).for(:accepted_terms) } it { should allow_value("valid.jpg").for(:image) } it { should allow_value("valid.png").for(:image) }