Skip to content

feat(TOS): allows admins to force TOS reaccpetance DEV-2178#7235

Merged
rgraber merged 6 commits into
mainfrom
beccagraber/dev-2178-clear-tos-acceptance
Jul 13, 2026
Merged

feat(TOS): allows admins to force TOS reaccpetance DEV-2178#7235
rgraber merged 6 commits into
mainfrom
beccagraber/dev-2178-clear-tos-acceptance

Conversation

@rgraber

@rgraber rgraber commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

🗒️ Checklist

  1. run linter locally
  2. update developer docs (API, README, inline, etc.), if any
  3. for user-facing doc changes create a Zulip thread at #Support Docs Updates, if any
  4. draft PR with a title <type>(<scope>)<!>: <title> DEV-1234
  5. assign yourself, tag PR: at least Front end and/or Back end or workflow
  6. fill in the template below and delete template comments
  7. review thyself: read the diff and repro the preview as written
  8. open PR & confirm that CI passes & request reviewers, if needed
  9. act on any greptile review below a 5/5 score or leave comment explaining why you won't
  10. delete this checklist section from the final squash commit before merging

📣 Summary

Add an admin action to require all users to reaccept the Terms of Service.

📖 Description

Adds an admin action to the SitewideMessage admin. When triggered, as long as there is a terms of service sitewide message present, all users will be prompted to accept the TOS regardless of whether they have already done so.

💭 Notes

It's a bit hacky to use the Constance config to store the last updated time since it's not really meant to be set programatically but the django singleton pattern is kind of a pain and this way we can just use an existing singleton object.

👀 Preview steps

  1. ℹ️ have an admin account
  2. In Django admin, create a SitewideMessage with the slug 'terms_of_service'
  3. Attempt to create a second SitewideMessage with the slug 'terms_of_service'
  4. 🔴 [on main] object is created
  5. 🟢 [on PR] Error Sitewide message with this Slug already exists.
  6. Register a new non-admin user (should require accepting Terms of Service)
  7. Log in as admin again
  8. 🟢 [on PR] In django-admin, go to SitewideMessages and run the Require all users to reaccept the Terms of Service action
  9. Log in again as the new user
  10. 🟢 [on PR] You are prompted to reaccept the terms of service

@rgraber rgraber self-assigned this Jul 10, 2026
@rgraber

rgraber commented Jul 10, 2026

Copy link
Copy Markdown
Contributor Author

@greptileai

@greptile-apps

greptile-apps Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds a Django admin action on SitewideMessageAdmin that forces all users to re-accept the Terms of Service by writing a formatted UTC timestamp to CONSTANCE_CONFIG['LAST_TOS_UPDATE']. The get_accepted_tos serializer method is updated to compare that timestamp against each user's stored last_tos_accept_time, returning False if the user has not accepted since the last update. A UNIQUE constraint is also added to SitewideMessage.slug to prevent duplicate TOS messages.

  • The admin action patches request.POST before calling super().changelist_view() so the queryset-wide action works without requiring individual row selection, and includes a guard that blocks execution when no terms_of_service slug exists.
  • Both the stored last_tos_accept_time (written by tos.py and adapter.py) and LAST_TOS_UPDATE (written by the admin action) use the same %Y-%m-%dT%H:%M:%SZ format, making lexicographic string comparison semantically valid.

Confidence Score: 5/5

Safe to merge — the core logic is correct, previously flagged concerns have been addressed, and the string comparison of identically-formatted ISO 8601 timestamps is semantically sound.

Both stored timestamps use the same format so lexicographic comparison is valid; the missing-TOS guard is in place; the changelist_view override fires the action exactly once; no new defects identified.

No files require special attention beyond what has already been discussed in the review thread.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant Admin as Django Admin (SitewideMessageAdmin)
    participant Constance as Constance Config
    participant DB as Database
    participant User as User Browser
    participant API as /api/v2/me/

    Admin->>DB: "Filter SitewideMessage slug='terms_of_service'"
    alt TOS message exists
        Admin->>Constance: "LAST_TOS_UPDATE = now().strftime('%Y-%m-%dT%H:%M:%SZ')"
        Admin-->>Admin: message_user(All users will be prompted)
    else No TOS message
        Admin-->>Admin: message_user(Add a SitewideMessage)
    end

    User->>API: GET /api/v2/me/
    API->>DB: Fetch user.extra_details.private_data['last_tos_accept_time']
    API->>Constance: config.LAST_TOS_UPDATE
    alt LAST_TOS_UPDATE is empty
        API-->>User: accepted_tos: True
    else "last_accepted > LAST_TOS_UPDATE"
        API-->>User: accepted_tos: True
    else last_accepted le LAST_TOS_UPDATE
        API-->>User: accepted_tos: False
    end

    User->>API: POST /tos (re-accept)
    API->>DB: "private_data['last_tos_accept_time'] = now().strftime()"
    API-->>User: 204 No Content
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant Admin as Django Admin (SitewideMessageAdmin)
    participant Constance as Constance Config
    participant DB as Database
    participant User as User Browser
    participant API as /api/v2/me/

    Admin->>DB: "Filter SitewideMessage slug='terms_of_service'"
    alt TOS message exists
        Admin->>Constance: "LAST_TOS_UPDATE = now().strftime('%Y-%m-%dT%H:%M:%SZ')"
        Admin-->>Admin: message_user(All users will be prompted)
    else No TOS message
        Admin-->>Admin: message_user(Add a SitewideMessage)
    end

    User->>API: GET /api/v2/me/
    API->>DB: Fetch user.extra_details.private_data['last_tos_accept_time']
    API->>Constance: config.LAST_TOS_UPDATE
    alt LAST_TOS_UPDATE is empty
        API-->>User: accepted_tos: True
    else "last_accepted > LAST_TOS_UPDATE"
        API-->>User: accepted_tos: True
    else last_accepted le LAST_TOS_UPDATE
        API-->>User: accepted_tos: False
    end

    User->>API: POST /tos (re-accept)
    API->>DB: "private_data['last_tos_accept_time'] = now().strftime()"
    API-->>User: 204 No Content
Loading

Reviews (5): Last reviewed commit: "fixup!: um" | Re-trigger Greptile

Comment thread kpi/serializers/current_user.py Outdated
Comment thread hub/admin/sitewide_message.py Outdated
Comment on lines +37 to +42
@admin.action(description='Require all users to reaccept the Terms of Service')
def require_terms_of_service_reacceptance(self, request, *args, **kwargs):
setattr(config, 'LAST_TOS_UPDATE', timezone.now())
self.message_user(
request, 'All users will be prompted to re-accept the Terms of Service'
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 These two user-visible strings are missing translation wrappers, violating the project rule that all text constants must be translatable. The @admin.action description and the message_user call should both use t() (which the rest of the codebase imports as from django.utils.translation import gettext as t).

Suggested change
@admin.action(description='Require all users to reaccept the Terms of Service')
def require_terms_of_service_reacceptance(self, request, *args, **kwargs):
setattr(config, 'LAST_TOS_UPDATE', timezone.now())
self.message_user(
request, 'All users will be prompted to re-accept the Terms of Service'
)
@admin.action(description=t('Require all users to reaccept the Terms of Service'))
def require_terms_of_service_reacceptance(self, request, *args, **kwargs):
setattr(config, 'LAST_TOS_UPDATE', timezone.now())
self.message_user(
request, t('All users will be prompted to re-accept the Terms of Service')
)

Rule Used: All text constants on both backend and frontend mu... (source)

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Comment thread hub/admin/sitewide_message.py Outdated
@rgraber

rgraber commented Jul 10, 2026

Copy link
Copy Markdown
Contributor Author

@greptileai

Comment thread kobo/apps/accounts/tests/test_current_user.py Outdated
@rgraber

rgraber commented Jul 10, 2026

Copy link
Copy Markdown
Contributor Author

@greptileai

Comment thread hub/migrations/0023_alter_sitewidemessage_slug.py
@rgraber rgraber marked this pull request as ready for review July 10, 2026 15:51
@rgraber rgraber requested review from jnm and noliveleger as code owners July 10, 2026 15:51
@rgraber rgraber removed request for jnm and noliveleger July 10, 2026 15:51
Comment thread hub/admin/sitewide_message.py Outdated
@rgraber rgraber requested a review from rajpatel24 July 13, 2026 11:29

@rajpatel24 rajpatel24 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@rgraber rgraber merged commit af131bf into main Jul 13, 2026
18 checks passed
@rgraber rgraber deleted the beccagraber/dev-2178-clear-tos-acceptance branch July 13, 2026 12:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants