#76

Improve active term overlap handling utilities

Openjasonappahwants to merge intofrom

This PR updates how the codebase evaluates user membership during overlapping terms, since the codebase previously supported only having one active term at a time.

I wrote some new utilities allowing each call site to be a bit more explicit about which terms we are evaluating a user for membership for. Since different behavior is desirable for different use cases, we can't evaluate user membership by implicitly assuming which term the call site is wanting to evaluate membership for now that we have multiple active terms at any given point in time. Most of those utilities should all have docstrings that I hope pretty clearly explain their use cases and why they're necessary. If not, please do let me know, because I want those utilities to be clear to other readers.

After writing those utilities in the model files, I updated the rest of the application to use those new utilities and deprecated the old utilities, which should get removed in a future PR.

Please start review in:

  • core/models.py
  • payments/models.py

Those two files contain the core utility changes and behavior updates. Most of the remaining files are call-site updates to use those utilities.

Activity

38
jasonappahreviewed5mo ago
jasonappahreviewed5mo ago
jasonappah1mo ago

tested this, looks like it is good to go

test results from codex

Membership Overlap Staging — Execution Record

Preflight evidence

  • The active local Discord server configuration matches the RoboBot Staging Ground server ID. No configuration values were recorded.
  • RoboBot Staging Ground was opened in Discord and the RoboBot application account was visibly online.
  • No production server, credential, payment, membership fixture, event, or account link was changed. The labelled local fixture, its attendance rows, and the explicitly authorized staging member-role lifecycle are the only campaign mutations.

Fixture state

  • Created a clearly labeled, temporary fixture in the local staging-compatible database: two active overlapping terms, two test profiles mapped to the plan's participant Discord IDs, staff-verified non-Square payments, and four fresh check-in events.
  • Initial membership state is operator old-term-only and Colin renewal-term-only. No production data or payment was used.
  • The local Django UI is running on http://localhost:8000; its check-in page requires a staff login before recording the interactive scenarios.

Event check-in evidence

  • Old-term-only: the fresh event rendered Membership Renewal Needed and named the later-ending renewal term. One attendance row exists.
  • Renewal-term-only: the fresh event rendered Successfully Checked In! with no renewal prompt. One attendance row exists.
  • Renewal-term-only repeat: a second scan rendered Already Signed In; the attendance count remains one.
  • Unpaid: after removing only Colin's labelled renewal purchased-product record, the fresh unpaid event rendered Member has not paid dues yet! Checked in: and identified the Colin fixture account. Attendance was still created.
  • Both terms paid: after restoring Colin's labelled renewal purchase and adding the labelled old-term purchase, the fresh both-terms-paid event rendered Successfully Checked In! with no renewal prompt.
  • These browser-visible results satisfy the functional portion of the old-term-only, renewal-term-only, and repeat U3 checks. No screen-video artifacts were retained; the operator accepted the supplied screenshots and direct browser verification as the evidence variance.

Regression coverage

The plan-required focused renewal coverage passed:

pipenv run python manage.py test events.tests.SignInMembershipRenewalTest --keepdb

Both tests passed: the old-term-only member receives the renewal prompt and the renewal-term-only member succeeds without it. The wider events.tests module is not an authoritative result yet because its unrelated lookup tests make optional UTD directory requests and log local TLS verification warnings. The Python runtime also logs unavailable blake2b and blake2s hash implementations; neither warning affected the two required assertions.

Remote Discord command evidence and data-boundary finding

  • In RoboBot Staging Ground's #bot-commands, the operator's visible /profile response reported the account as Not a member, with historical terms and a Fall 2026 dues link. This does not match the local fixture's intended old-term-only membership.
  • The same account's visible /pay response offered Fall 2026 and Spring 2027, rather than the two clearly labelled local overlap terms.
  • These are successful live command responses, but they demonstrate a data-boundary mismatch: the live RoboBot service is reading a different database from the local Django application and local fixture. Therefore they cannot validate U4 for the local staging fixture.
  • No payment link was opened and no roles, membership links, or data were changed. The required Colin recordings were not requested or altered.

Local RoboBot Beta evidence

  • The local discord_bot.py process is now running against the same local database as the fixture and has connected to RoboBot Staging Ground as the separate RoboBot Beta application. Its local HTTP endpoint is listening on port 2468.
  • Discord visibly shows RoboBot Beta online alongside the remote RoboBot instance. The earlier remote-command evidence remains invalid for the fixture; all subsequent membership commands must explicitly select RoboBot Beta.
  • Discord initially held a stale command definition after RoboBot Beta synchronized its commands. Reloading the Discord client corrected that client-cache problem; it was not a membership-service failure.
  • Operator old-term-only /profile: RoboBot Beta reported the expiring term as the current membership and offered payment for the later-ending renewal term.
  • Operator old-term-only /pay: RoboBot Beta listed both active terms, marked the expiring term as already paid, and presented the renewal-term payment link. No payment link was opened.
  • Colin renewal-term-only /profile and /pay: Colin supplied a RoboBot Beta screenshot showing only the renewal term as active, with no renewal-needed prompt; /pay lists both active terms and marks the renewal term as already paid.
  • Colin unpaid /profile and /pay: Colin supplied a RoboBot Beta screenshot showing Not a member, Dues needed for the renewal term, and neither /pay term marked already paid.
  • The supplied screenshots prove the state-specific content but are not the plan-required recordings. They are retained as a video-evidence variance.

Initial role and export evidence

  • Bulk role addition: RoboBot Beta's /givememberroles response reported that it added the staging member role to two active fixture users. This includes the old-term-only operator and renewal-term-only Colin. No before-and-after recording was retained; the operator accepted the direct command and role-state checks as the evidence variance.
  • Role purge: a read-only staging-role check found seven current holders: the two fixture identities and five non-test members. The operator then explicitly authorized the staging-only global command. RoboBot Beta's /purgememberroles reported processing 11 non-active members. The actual holder count fell to one: the active operator retained the role and unpaid Colin lost it. The command's 11 is its candidate count, not a literal before/after role-holder count.
  • Automatic role grant: recreating Colin's labelled renewal purchased-product record against the previously staff-verified payment restored Colin's staging role automatically. The final read-only role check reports two holders, and both the operator and Colin hold the role. This passed.
  • Created two additional, clearly labelled post-cutoff local fixture events (IDs 8 and 9). Together with the prior state-specific events, each fixture identity has three qualifying attendance rows.
  • The standard export scripts produced initial single-term member and voter CSVs. Both contain the operator old-term-only and Colin renewal-term-only fixture rows, as required.
  • In Colin's unpaid state, the member and voter CSVs retain the active operator fixture row and exclude Colin despite Colin's three qualifying attendances.
  • After moving Colin to both-terms-paid, the member and voter CSVs contain both fixture rows.
  • The voter job emitted its existing naive-datetime warning; the rows nevertheless reflect the expected three qualifying attendances. Both export jobs also emitted the pre-existing unavailable-BLAKE2 runtime warning.

Intentionally skipped coverage

  • The current fixture usernames are intentionally descriptive labels rather than valid nine-character NetIDs, so they cannot exercise /link's email-backed flow. Replacing or renaming a fixture user would require a new account-link email path and must not be substituted by directly editing the Discord ID. The operator elected to skip this scenario.
  • Colin is now both-terms-paid: the retained, staff-verified renewal payment was reconnected to its labelled purchase for the automatic-grant check, then a separate labelled old-term payment and purchase were added. The both-terms-paid browser check, automatic purchase-role grant, and final exports passed. No further Colin evidence is required: the already supplied renewal-only and unpaid screenshots are retained as the plan's recording-evidence variance, and the final state is covered by the local browser, export, and Discord-role checks.
  • The operator rejoin scenario requires the prepared staging invite and a recording; it was not attempted. No test user was asked to leave the server, and the operator elected to skip this scenario.
  • No video recordings were retained. The supplied state screenshots are recorded as the accepted evidence variance.