Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[MNOE-570] Storing the time of agreement to TOS #407

Merged
merged 4 commits into from
Aug 21, 2017

Conversation

enizor
Copy link
Contributor

@enizor enizor commented Aug 17, 2017

The time of agreement of TOS is stored in user[:metadata][tos_accepted_at]
It is then used in the /confirmation to create (or not) the TOS checkbox.
@ouranos

Remi Garde added 2 commits August 18, 2017 15:46
The time of agreement of TOS is stored in user[:metadata][tos_accepted_at]
It is then used in the /confirmation to create (or not) the TOS checkbox.
@ouranos ouranos self-requested a review August 21, 2017 00:42
end
end

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove the extra space

@@ -92,6 +92,9 @@ def finalize
end

if resource.errors.empty?
if params[:tos]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should you test params[:tos] == "accept"?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The form can be submitted only when the TOS are accepted so it souldn't be required.
However, checking the exact string will improve clarity at no cost, I'm adding it.

@ouranos ouranos added this to the v3.3 milestone Aug 21, 2017
@ouranos ouranos merged commit 45c7443 into maestrano:3.3 Aug 21, 2017
@enizor enizor deleted the tos-agreement branch August 22, 2017 00:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants