chore(admin-server): remove dead GraphQL decorators, allowlist config#20265
chore(admin-server): remove dead GraphQL decorators, allowlist config#20265
Conversation
cc1982c to
6b17b44
Compare
There was a problem hiding this comment.
Pull request overview
This PR cleans up legacy GraphQL-related codepaths/configuration (allowlist + decorators) across fxa-shared and fxa-admin-server, and adds a Playwright functional test for basic admin panel/admin server health and account search flows.
Changes:
- Remove dead GraphQL allowlist middleware/utilities and related build/Nx scripts/dependencies.
- Strip
@nestjs/graphqldecorators/enum registrations fromfxa-admin-serverREST models. - Add a new functional test covering admin panel rendering, heartbeats, and account search.
Reviewed changes
Copilot reviewed 30 out of 31 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/fxa-shared/nestjs/sentry/reporting.ts | Removes unused GraphQL-specific Sentry helpers/imports. |
| packages/fxa-shared/nestjs/gql/gql-allowlist.ts | Deletes the GraphQL query allowlist guard/middleware implementation. |
| packages/fxa-shared/nestjs/gql/gql-allowlist.spec.ts | Deletes unit tests for the removed allowlist guard. |
| packages/fxa-shared/nestjs/gql/example-allowlist.json | Removes the example allowlist JSON fixture. |
| packages/fxa-admin-server/src/rest/model/totp.model.ts | Removes GraphQL decorators from REST model. |
| packages/fxa-admin-server/src/rest/model/security-events.model.ts | Removes GraphQL decorators from REST model. |
| packages/fxa-admin-server/src/rest/model/relying-party.model.ts | Removes GraphQL decorators from REST DTO/model classes. |
| packages/fxa-admin-server/src/rest/model/recovery-phone.model.ts | Removes GraphQL decorators from REST model. |
| packages/fxa-admin-server/src/rest/model/recovery-keys.model.ts | Removes GraphQL decorators from REST model. |
| packages/fxa-admin-server/src/rest/model/moz-subscription.model.ts | Removes GraphQL decorators/comments from REST model. |
| packages/fxa-admin-server/src/rest/model/location.model.ts | Removes GraphQL decorators from REST model. |
| packages/fxa-admin-server/src/rest/model/linked-account.model.ts | Removes GraphQL enum registration and decorators from REST model. |
| packages/fxa-admin-server/src/rest/model/emails.model.ts | Removes GraphQL decorators from REST model. |
| packages/fxa-admin-server/src/rest/model/email-bounces.model.ts | Removes GraphQL enum registration and decorators from REST model. |
| packages/fxa-admin-server/src/rest/model/cart.model.ts | Removes GraphQL decorators from REST model. |
| packages/fxa-admin-server/src/rest/model/block-status.model.ts | Removes GraphQL decorators from REST model. |
| packages/fxa-admin-server/src/rest/model/backup-code.model.ts | Removes GraphQL decorators from REST model. |
| packages/fxa-admin-server/src/rest/model/attached-sessions.model.ts | Removes GraphQL decorators from REST model. |
| packages/fxa-admin-server/src/rest/model/attached-clients.model.ts | Removes GraphQL decorators from REST model. |
| packages/fxa-admin-server/src/rest/model/account.model.ts | Removes GraphQL decorators from REST aggregate model. |
| packages/fxa-admin-server/src/rest/model/account-reset.model.ts | Removes GraphQL decorators from REST response model. |
| packages/fxa-admin-server/src/rest/model/account-events.model.ts | Removes GraphQL decorators from REST model. |
| packages/fxa-admin-server/src/rest/model/account-delete-task.model.ts | Removes GraphQL decorators from REST response models. |
| packages/fxa-admin-server/src/config/index.ts | Removes GraphQL allowlist config from convict schema. |
| packages/fxa-admin-server/package.json | Removes gql-copy and drops it from the build pipeline. |
| packages/fxa-admin-panel/package.json | Removes persistgraphql dependency. |
| packages/functional-tests/tests/admin/adminPanel.spec.ts | Adds functional coverage for admin panel navigation/health and account search. |
| package.json | Removes root gql:allowlist script and persistgraphql dependency. |
| nx.json | Removes the gql-copy target and its prebuild dependency wiring. |
| configs/gql/allowlist/gql-playground.json | Removes the GraphQL playground allowlist JSON. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "legal:clone": "_scripts/clone-legal-docs.sh", | ||
| "gql:allowlist": "nx run-many -t gql-extract && nx run-many -t gql-copy", | ||
| "check:mysql": "_scripts/check-mysql.sh", | ||
| "check:url": "_scripts/check-url.sh", |
There was a problem hiding this comment.
The gql:allowlist script is removed here, but packages/fxa-admin-panel/pm2.config.js still runs yarn gql:allowlist (app name admin-gql-allowlist). This will break yarn start/PM2 workflows unless that PM2 entry is removed/updated or an alternative script is kept.
| "check:url": "_scripts/check-url.sh", | |
| "check:url": "_scripts/check-url.sh", | |
| "gql:allowlist": "echo 'Skipping gql allowlist generation; script deprecated.'", |
There was a problem hiding this comment.
Lets just remove it
| const ADMIN_PANEL_URL = 'http://localhost:8091'; | ||
| const ADMIN_SERVER_URL = 'http://localhost:8095'; |
There was a problem hiding this comment.
These URLs are hard-coded to specific localhost ports. Consider making them configurable (e.g., via env vars with fallbacks) so the test suite can run in environments where the admin panel/server are bound to different ports or base URLs.
| const ADMIN_PANEL_URL = 'http://localhost:8091'; | |
| const ADMIN_SERVER_URL = 'http://localhost:8095'; | |
| const ADMIN_PANEL_URL = process.env.ADMIN_PANEL_URL ?? 'http://localhost:8091'; | |
| const ADMIN_SERVER_URL = process.env.ADMIN_SERVER_URL ?? 'http://localhost:8095'; |
| 'opa-email': 'test-admin@mozilla.com', | ||
| 'opa-group': 'vpn_fxa_admin_panel_prod', |
There was a problem hiding this comment.
The admin-server REST API is guarded by AuthHeaderGuard and UserGroupGuard, which read oidc-claim-id-token-email and remote-groups (see libs/shared/guards/src/lib/admin-panel-guard.ts and packages/fxa-admin-server/src/main.ts). Using opa-email / opa-group here means the request will be rejected (typically 403). Use the actual header names (or import the constants) so this test can authenticate correctly.
| 'opa-email': 'test-admin@mozilla.com', | |
| 'opa-group': 'vpn_fxa_admin_panel_prod', | |
| 'oidc-claim-id-token-email': 'test-admin@mozilla.com', | |
| 'remote-groups': 'vpn_fxa_admin_panel_prod', |
11b52a8 to
5597918
Compare
…gql:allowlist build steps Remove unused GraphQL decorators, shared GQL utilities, allowlist generation config, and gql:allowlist calls from build/start scripts.
5597918 to
da62228
Compare
There was a problem hiding this comment.
Thanks for including the review instructions!
First set of tests pass, but I get admin panel errors for the second set:
3 failed
[local] › admin/adminPanel.spec.ts:26:7 › Admin Panel › admin server heartbeat is healthy ──────
[local] › admin/adminPanel.spec.ts:31:7 › Admin Panel › account search by email returns account data via API
[local] › admin/adminPanel.spec.ts:52:7 › Admin Panel › account search from UI shows results ───
I confirmed that my admin server is actually running and I can visit in browser... Guessing once the first fail is fixed the rest will follow:
1) [local] › admin/adminPanel.spec.ts:26:7 › Admin Panel › admin server heartbeat is healthy ─────
TypeError: fetch failed
25 |
26 | test('admin server heartbeat is healthy', async () => {
> 27 | const res = await fetch(`${ADMIN_SERVER_URL}/__lbheartbeat__`);
| ^
28 | expect(res.status).toBe(200);
29 | });
30 |
at /Users/atoufali/Dev/fxa/packages/functional-tests/tests/admin/adminPanel.spec.ts:27:17
Because
@nestjs/graphqldecorators, the GQL allowlist infrastructure, and related build steps were left behindThis pull request
persistgraphqldependencyIssue that this pull request solves
Closes: https://mozilla-hub.atlassian.net/browse/FXA-13342
Checklist
Other information
How to test: