fix(account): stop telling non-flygd members their first sync is pending - #46
Conversation
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 2 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
Comment |
9528882 to
f4e9482
Compare
db6283d to
ced35e4
Compare
ced35e4 to
643cece
Compare
contact_sync_state rows are written only for characters in getFlygdCharacters (src/jobs/contacts.ts:74) — tier flygd, affiliation valid. Nothing else calls recordResult. So for every blue and green member contactSyncResult is null permanently, and the account page read that structural absence as a pending first run: - the STANDINGS cell said "not yet run" forever - the first-run notice fired forever - the contact-label footnote claimed authGD manages a label on their characters, which it never does A flygd character CCP reports as gone hits the same path: excluded from the desired set, so it stops accruing results for the same reason. AccountView now carries contactsTarget per character, derived by isContactsTarget — a pure predicate colocated with getFlygdCharacters so the two definitions cannot drift, with a test asserting they agree on the same rows. Where the concept does not apply the cell reads "—" rather than asserting a state, the notice is scoped to characters that can actually be waiting on a first run, and the footnote is shown only to accounts it describes. The account-level question is untouched: LAST PUSHED already answers "is my standing being pushed, and when next" for every tier, which is where that belongs.
643cece to
6086c74
Compare
1 participant
Stacked on #42 — review that one first; this diff is only the last commit.
The bug
contact_sync_staterows are written only insidefor (const target of flygd)in
src/jobs/contacts.ts:74, whereflygdisgetFlygdCharacters(tierflygd,affiliation_invalid = false). Nothing else callsrecordResult.So for every blue and green member
contactSyncResultisnullpermanently,and the account page read that structural absence as a pending first run:
not yet runforeverbeing told their first sync hadn't happened
which for a blue member it never does
A flygd character CCP reports as gone (
affiliation_invalid) hits the same path:dropped from the desired set, so it stops accruing results for the same reason.
The fix
AccountViewcarriescontactsTargetper character, derived byisContactsTarget— a pure predicate colocated withgetFlygdCharactersso thetwo definitions cannot drift, with a test asserting they agree on the same rows.
Where the concept doesn't apply the cell reads
—rather than asserting a state.The notice is scoped to characters that can actually be waiting on a first run.
The footnote is shown only to accounts it describes.
No schema change, no migration, no new query —
affiliation_invalidwas alreadyin the row being selected.
What this deliberately does not do
There is no per-character record of "your standing was delivered into the FLYGD
members' contact lists", and there can't be one without new persisted state. The
account-level question is already answered honestly by the LAST PUSHED section
from #42, for every tier. Building a per-character delivery ledger to re-answer
it would mean a migration for no new information.
Tradeoff: a brand-new blue member no longer sees a first-run reassurance. That
message was never true-by-measurement for them — it was a null being read as a
state — and LAST PUSHED gives them the real cadence instead.
Conflicts
Resolved. #41 has merged, and its column note is where the fix now lands: the
header it introduces reads
Standingsafter #42's rename, andshowContactsNotenow requires
contactsTargetas well ascontactsNoteApplies, so a blue memberkeeps the note in the accessible tree as the column's description without it
becoming visible copy about something that never happens to them.
Rebased onto the rewritten #42 branch after #42 was rebased on
main.Verification
npx tsc --noEmit— cleannpx prettier --check .— all files matchnpx eslint .— 0 errors, 5 warnings (pre-existing-styleno-img-element)npx vitest run— 356 passed (47 files)npx playwright test— 18 passed