Redirect to a Terms and conditions form

[AdrianDAlessandro]

Description

This PR builds on #677 by adding a redirect to a form that allows users to accept the terms and conditions if they have not done so

Type of change

• Documentation (non-breaking change that adds or improves the documentation)
• New feature (non-breaking change which adds functionality)
• Optimization (non-breaking, back-end change that speeds up the code)
• Technical work (non-breaking, change which is work as part of a new feature)
• Bug fix (non-breaking change which fixes an issue)
• Breaking change (whatever its nature)

Key checklist

• All tests pass (eg. python -m pytest)
• The documentation builds and looks OK (eg. mkdocs serve)
• Pre-commit hooks run successfully (eg. pre-commit run --all-files)

Further checks

• Code is commented, particularly in hard-to-understand areas
• Tests added or an issue has been opened to tackle that in the future. (Indicate issue here: # (issue))

[codecov]

Codecov Report

❌ Patch coverage is 97.50000% with 1 line in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
main/views/account_views.py	96.77%	1 Missing ⚠️

📢 Thoughts on this report? Let us know!

[anenadic]

I ran this change locally- I am getting an error about the non-existent agreed_to_tos field in the database - I need to run some DB migration but now sure know which one so just leaving this comment to make sure we do this on the production server.

Another comment is the suggestion to add the following text to Privacy Policy:

"Privacy policy changes
DIRECT reserves the right to update its privacy policy periodically. We, therefore, encourage visitors to frequently check this page for any changes to the privacy policy. Your continued use of this site after any change in this privacy policy will constitute your acceptance of such changes."

[davehorsfall]

Very nice. This is working as expected, but I've left some comments for discussion.

[davehorsfall]

This is a great extension to the PR #677. I was wondering how you would deal with users who registered before this feature was added. It is nice to be able to capture those people too.

However, I also think this could be useful when we update our terms and conditions in the future. It would be good to have the ability to notify users about updated terms, and ask them to agree again?

<h1>Terms and Privacy Policy</h1>
<p class="mb-3">
    To continue using your account, please review and accept our latest Terms and Conditions and Privacy Policy.
</p>

There are a few approaches that might work for that, and it may involve adding an additional field to the model. Therefore, this might be outside the scope of this PR. Let me know what you think.

[AdrianDAlessandro]

I did think about this too. I think one way to do this is to, whenever we include an update to the terms and conditions, we also include a data migration that resets all users agreed_to_tos field to false, which would trigger the behaviour implemented in this PR. Does that sound like a good approach to you? It's not automated, but I'm not sure I can see a way to automate it without building a Terms model that, when updated, will trigger the same behaviour as the aforementioned data migration.

[anenadic]

I added the suggestion to the policy above - this is how SSI deals with policy changes. Not sure if it is acceptable for us.

"Privacy policy changes
DIRECT reserves the right to update its privacy policy periodically. We, therefore, encourage visitors to frequently check this page for any changes to the privacy policy. Your continued use of this site after any change in this privacy policy will constitute your acceptance of such changes."

[davehorsfall]

I also initially thought of that @AdrianDAlessandro however, I don't think it is a good approach because, if we wipe the record of their initial consent, it means they have effectively never consented. This is a problem because if a user exists but looks like they have never consented to our terms and privacy policy, we wouldn't have permission to work with their data.

I think we need to keep the agreed_to_tos, as a record of their consent.

We can potentially work with date_agreed as meaning "the date they last agreed to the tos". If we compare that date with the last update of the terms, you could build logic around that to determine if they need to consent again?

[AdrianDAlessandro]

That would be achievable, but we'd need to save the date to compare it against somewhere, which we'd need to update every time the terms are updated.

Maybe it would be better to have the date_agreed as a list of dates to include every time they've agreed? Because it could imply they hadn't consented before the most recent one. That way we could still use the agreed_to_tos field as a flag that get updated every time the terms are updated.

Alternatively, we go with Alek's proposal and keep it simple, without needing any checks for updates

[AdrianDAlessandro]

I think this issue about how we handle changes in the future is blocking us from getting this current behaviour that we need (blocking people from using the app if they haven't agreed to terms and conditions). So let's open an issue to handle this later down the line and merge this PR as is.

[davehorsfall]

Whereas other authenticated pages redirected, I noted it was possible to access the "Change Password" page before I had accepted the TOS. I'm not sure if this was by design, and wanted to highlight it.

[AdrianDAlessandro]

Sorry, I thought I had responded to this already. This was initially an accident, but then I thought it was fine. It's because the change password view is one that comes built-in to Django. We would have to override it and use a custom one to get this behaviour.

Considering that changing a password is a security feature and does not involve them submitting any personal information, I figured it was actually fine to leave as-is

[AdrianDAlessandro]

I ran this change locally- I am getting an error about the non-existent agreed_to_tos field in the database - I need to run some DB migration but now sure know which one so just leaving this comment to make sure we do this on the production server.

@anenadic It is just python manage.py migrate and that is run every time the server is re-deployed.

Another comment is the suggestion to add the following text to Privacy Policy:

"Privacy policy changes DIRECT reserves the right to update its privacy policy periodically. We, therefore, encourage visitors to frequently check this page for any changes to the privacy policy. Your continued use of this site after any change in this privacy policy will constitute your acceptance of such changes."

I think this is maybe not necessary if we do the automatic approach suggested by @davehorsfall above?

[anenadic]

I ran this change locally- I am getting an error about the non-existent agreed_to_tos field in the database - I need to run some DB migration but now sure know which one so just leaving this comment to make sure we do this on the production server.

@anenadic It is just python manage.py migrate and that is run every time the server is re-deployed.
Great, thank you. Maybe we can add this to the instructions in README too. I keep forgating the command to run as I do not use it often.

Another comment is the suggestion to add the following text to Privacy Policy:
"Privacy policy changes DIRECT reserves the right to update its privacy policy periodically. We, therefore, encourage visitors to frequently check this page for any changes to the privacy policy. Your continued use of this site after any change in this privacy policy will constitute your acceptance of such changes."

I think this is maybe not necessary if we do the automatic approach suggested by @davehorsfall above?
Indeed - might not be needed. Cannot harm either to have it in the privacy policy?

[anenadic]

Juste tested the latest code on this branch locally - works as expected.

[davehorsfall]

Happy to approve current this as-is, and revisit how to deal with updated TOS in the future.