Conversation
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
SQL null values currently become false, causing inapplicable events to display a misleading “No.”
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds session verification status to the Admin panel’s Account History table.
Changes:
- Adds a Verified column using Yes/No/dash formatting.
- Tests the header and verification states.
File summaries
| File | Description |
|---|---|
Account/index.tsx |
Renders verification status for security events. |
Account/index.test.tsx |
Tests the new column and displayed values. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| <>{securityEvent.name}</> | ||
| <>{getFormattedDate(securityEvent.createdAt)}</> | ||
| <>{securityEvent.ipAddr}</> | ||
| <>{formatVerified(securityEvent.verified)}</> |
## Because - The Account History table in the Admin panel showed Event, Timestamp, IP, and Additional Info. It did not show the session `verified` flag, so support staff could not tell a verified sign in from an unverified one. - `typeCasting` in the admin server cast every MySQL TINY(1) column with `field.string() === '1'`, so a SQL NULL became `false`. - `securityEvents.verified` is a nullable BOOLEAN column. The new column showed "No" for events that never set the flag, and the code that shows a dash never ran. ## This pull request - Adds a "Verified" column to the Account History table, between IP and Additional Info. - Shows Yes or No with the panel's `ResultBoolean` component, and a grey dash when the value is null. - Fixes `typeCasting` to read `field.string()` once and return `null` when the field is null. - Widens `verified` to `boolean | null` in the admin server REST model and in the shared types that the panel imports. - Adds tests for the NULL cast and for the null render. ## Issue that this pull request solves Closes: FXA-14576
clouserw
approved these changes
Sep 18, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Because
verifiedflag, so support staff could not tell a verified sign in from an unverified one.typeCastingin the admin server cast every MySQL TINY(1) column withfield.string() === '1', so a SQL NULL becamefalse.securityEvents.verifiedis a nullable BOOLEAN column. The new column showed "No" for events that never set the flag, and the code that shows a dash never ran.This pull request
ResultBooleancomponent, and a grey dash when the value is null.typeCastingto readfield.string()once and returnnullwhen the field is null.verifiedtoboolean | nullin the admin server REST model and in the shared types that the panel imports.Issue that this pull request solves
Closes: FXA-14576
Checklist
Put an
xin the boxes that applyHow to review (Optional)
database.service.tsfor the cast, andPageAccountSearch/Account/index.tsxfor the column.typeCastingruns on every TINY(1) column that the admin server reads, not only onverified.Screenshots (Optional)
Please attach the screenshots of the changes made in case of change in user interface.
Other information (Optional)
packages/fxa-admin-server/src/rest/model/account.model.tsand thesecurity-events.model.tsthat it imports are not imported anywhere. They are dead code, and this pull request leaves them in place.false. The only other nullable TINY(1) column,accounts.atLeast18AtReg, is not read by the admin server.