User must review TOS when updated#3494
Conversation
|
|
||
| def permited_paths? | ||
| permited_paths = [tos_path, decidim.delete_account_path, decidim.accept_tos_path] | ||
| case request.path |
There was a problem hiding this comment.
permitted_paths.include?(request.path) instead of all this case
|
|
||
| private | ||
|
|
||
| def permited_paths? |
There was a problem hiding this comment.
s/permited/permitted everywhere
There was a problem hiding this comment.
Ooops, I'll fix it
| module TosPageHelper | ||
| # Renders the Call To Action button. Link and text can be configured | ||
| # per organization. | ||
|
|
| html.html_safe | ||
| end | ||
|
|
||
| def tos_page_sticky_form |
There was a problem hiding this comment.
Move this method and following ones to a cell
| class AddAcceptedTosVersionFieldToUsers < ActiveRecord::Migration[5.1] | ||
| def up | ||
| add_column :decidim_users, :accepted_tos_version, :datetime | ||
| Decidim::Organization.find_each do |organization| |
There was a problem hiding this comment.
Use fake classes in migrations, please:
class Organization < ApplicationRecord
self.table_name = "decidim_organizations"
endEtc.
There was a problem hiding this comment.
Ok, I'll use fake classes
| organization | ||
|
|
||
| # rubocop:disable RSpec/EmptyLineAfterSubject | ||
| # Bug in rubocop-rspec |
There was a problem hiding this comment.
Remove this line too, since this is fixed now 😄
|
Same as in #3491, can @decidim/product approve this? 😄 |
25190e6 to
d6a96f7
Compare
|
@mrcasals I've made the changes you requested. I've created the |
|
Tests are failing, can you check them @agustibr please? |
|
@mrcasals yes I'll fix them 🪲 |
af12681 to
4efdd99
Compare
|
@mrcasals tests are fixed, All checks have passed! |
decidim-core/db/seeds.rb
Outdated
| tos_agreement: true, | ||
| personal_url: Faker::Internet.url, | ||
| about: Faker::Lorem.paragraph(2) | ||
| # accepted_tos_version: Time.current |
There was a problem hiding this comment.
I'll fix it 😓 this sould be accepted_tos_version: organization.tos_version
decidim-core/db/seeds.rb
Outdated
| tos_agreement: true, | ||
| personal_url: Faker::Internet.url, | ||
| about: Faker::Lorem.paragraph(2) | ||
| # accepted_tos_version: Time.current |
There was a problem hiding this comment.
same as before 😿 I'll fix it
| include NeedsTosAccepted | ||
|
|
||
| before_action :configure_permitted_parameters | ||
| helper_method :terms_and_conditions_page |
There was a problem hiding this comment.
Maybe move this to the concern?
| add_column :decidim_users, :accepted_tos_version, :datetime | ||
| Organization.find_each do |organization| | ||
| # rubocop:disable Rails/SkipsModelValidations | ||
| organization.users.update_all(accepted_tos_version: organization.tos_version) |
There was a problem hiding this comment.
Shouldn't we set a tos_version value to the organization first?
There was a problem hiding this comment.
Yes, there's a migration at master branch from PR #3491 that sets the tos_version to the organization:
decidim/20180508111640_add_tos_version_to_organization.rb at master · decidim/decidim
4efdd99 to
e5f0178
Compare
|
@oriolgual the requested changes have been made and the checks have passed |
|
🎉 thanks! |
* User must review TOS when updated + AnnouncementCell * Fix update_organization_tos_version_spec and user_tos_acceptance_spec * fix seeds admin and user :accepted_tos_version * move `helper_method :terms_and_conditions_page` to `NeedsTosAccepted` concern
🎩 What? Why?
When a user has accepted the TOS (Term Of Service), if it gets updated the user must review and accept again. Or an option to refuse.
Invited users must also accept the TOS.
📌 Related Issues
📋 Subtasks
CHANGELOGentryaccepted_tos_versionto userhigh_voltagegem version📷 Screenshots (optional)
Redirected to tos page:
If TOS are accepted:
If the Refuse option is clicked a modal is shown with options:
Accept Invitation screen with TOS checkbox and text:
