fix(admin): open the date column newest-first - #155
Conversation
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 10 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: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Comment |
f5ad7ca to
474682d
Compare
474682d to
740b6ae
Compare
740b6ae to
27ecfe3
Compare
An admin coming back to /admin/accounts asks one question of "Tier changed": what moved while I was away. The column opened ascending, so the top of the table filled with the most settled accounts on the roster — the ones nothing has happened to since last spring — and the answer sat at the bottom, behind a second click, every visit. On a page whose own lede says the sync jobs change things without telling anyone, that is the wrong end of the column to land on. `SORT_OPENS` gives each key the direction it opens on its FIRST press. Name, Tier and Cryo keep ascending, which is right for a value you already have in mind and are scanning for. Only `tierChangedAt` opens descending. Once a column is active it toggles exactly as before — this changes the opening direction and nothing else. The spec asserts the URL, `aria-sort`, and the rendered row order, with the two seeded accounts named so alphabetical order and recency order disagree — otherwise a sort that did nothing at all would pass. The tier-change instants are stamped with a direct update rather than by widening `seedMember`, which five other spec files share. Verified to fail with the fix reverted (`Received: …dir=asc`). typecheck, eslint, prettier --check .: clean. npm test: 77 files, 1134 tests passed. npx playwright test: 233 passed, 233 ✓ lines counted.
27ecfe3 to
0847278
Compare
|
@coderabbitai review |
|
1 participant
Round five of an unattended design sweep. One sentence: the "Tier changed" column now opens newest-first.
Based on #154 — merge #148, #149, #151, #154, then this.Superseded: #148, #149, #151 and #154 have all merged, and GitHub retargeted this PR tomain. It is now standalone and merges on its own. See Merge order at the bottom.What changed
admin/accounts/page.tsx— a newSORT_OPENSmap gives each sortable column the direction it opens on its first press. Name, Tier and Cryo keep ascending. OnlytierChangedAtchanges.The cost it removes. An admin returning to the roster asks one question of that column: what moved while I was away. Ascending answers the opposite question — the top of the table fills with the accounts nothing has happened to since last spring, and the thing they came for sits at the bottom of a three-screen table behind a second click, on every visit. The page's own lede says the sync jobs change tiers without telling anyone; the column that reports when is the one place that has to be readable in one press.
Scope held deliberately narrow. Once a column is active it toggles exactly as before — ascending ⇄ descending. This is the opening direction only, and the other three keys are unchanged, because a value you already have in mind (a name, a tier) is found from the top and a recency question is not.
The spec
admin.spec.ts:415, verified to fail with the fix reverted (Received: …&dir=asc).It asserts three things rather than one, because each alone is weak: the URL's
dir, the header'saria-sort, and the rendered row order. The URL alone would pass ifdirstopped reaching the query; the row order alone would pass on a table that never sorted at all. The two seeded accounts are namedAncientandZetaso alphabetical order and recency order disagree — with names that happened to agree, a sort doing nothing would pass every assertion.The tier-change instants are stamped with a direct
db.updaterather than by adding a field toseedMember, which five other spec files share. Widening a shared helper for one spec is the kind of change this run leaves alone.Skipped this round, and why
The Tokens badge's wording (
page.tsx:631) was ranked next and is being reported rather than changed. The finding was that a red badge reading "4/5 ok" says "ok" on the row that is broken. On reading the actual markup that argument is thinner than the backlog entry made it sound: the word qualifies the numerator, the number beside it already says 4 of 5, and the badge's tone already carries the severity. Cutting it to "4/5" trades one ambiguity for another — "4/5" of what — and this is a copy judgement on a scanning surface, which is exactly the class of call an unattended run should not make on the strength of a one-line finding. Left open..log td.num > .stackis dead CSS (globals.css:3159-3170) and stays that way this round. It is verifiable and the deletion is safe, but it costs no user anything, which puts it below this run's floor. Named so it is not rediscovered as new.docs/settled-design-decisions.md:155records the inverse ofInlineEdit's current contract. It says actions must reject via redirect;inline-edit.tsx:17-32deliberately reversed that and five actions now returnStringFieldEditState. This one matters more than its size — every reviewer in this sweep is told that file is closed, so a future pass will "restore" the redirect contract and reintroduce the defect of a rejected edit wiping an operator's typed number on a money screen. It is left for a human on purpose: the settled-decisions file is the record of what people already decided, and an unattended run editing it is the one thing this loop should never do, even to correct it. Worth someone's five minutes.Gates
Run on this branch's tip in the worktree, quoted:
tsc --noEmit— cleaneslint .— cleanprettier --check .— "All matched files use Prettier code style!"npm test— 77 files, 1134 tests passed, matching the known-good baselinenpx playwright test— 233 passed (4.5m), exit 0, with 233✓lines counted in the captured log. That is fix(admin): armgrantthe way its own opposite is armed #154's 232 plus exactly the one spec this round adds.Not run on this branch:
docker build .andscripts/check-node-version.sh— environment-bound and slow. CI runs both on this PR. Treat them as unchecked until it does.Merge order
No ordering left to respect. Every branch this one stacked on has merged (#148 → #149 → #151 → #154, in that order), and this PR's base is now
main. The squash caveat that used to sit here no longer applies to it.One thing worth knowing about the history below it:
mainmoved under this run — #150, #152, #153 and #156 merged while the sweep was working, and #153 landed inconfirm-submit.tsx, which round one had rewritten. That conflict was resolved on the round-one branch (kept both edits: round one's CSS-generated-content width reservation, #153'sconfirmprop and its below-the-hooks throw) and verified by running #153's own regression spec,e2e/payouts.spec.ts:2212, on the merge. It passed. That resolution is inmainnow.#158 is also open and independent of this one — it touches only
e2e/payouts.spec.ts.