Major refactor: remove GraphQL surface and consolidate REST/Swagger architecture#4
Conversation
There was a problem hiding this comment.
Pull request overview
This PR represents a major architectural shift from GraphQL to a pure REST/Swagger API architecture. The refactor removes all GraphQL resolvers, models, and dependencies while consolidating the API surface around REST controllers with Swagger documentation. The work also includes runtime hardening improvements and code organization enhancements.
Changes:
- Removed GraphQL stack including resolvers, auth guards, config service, and all Apollo/GraphQL dependencies from package.json
- Migrated DTOs and models from GraphQL decorators (
@Field,@ObjectType) to Swagger decorators (@ApiProperty,@ApiPropertyOptional) - Reorganized notification providers into dedicated
providers/subdirectory for better code organization - Enhanced runtime bootstrap with DATABASE_URL construction from components, password redaction in logs, and improved Redis password handling
Reviewed changes
Copilot reviewed 66 out of 75 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| tsconfig.json | Added TypeScript compiler options to suppress deprecations and disable strict property initialization |
| src/wss/wss.gateway.ts | Minor whitespace formatting fix |
| src/wss/wss.adapter.ts | Commented out unused error imports |
| src/users/user.resolver.ts | Deleted GraphQL resolver (removed file) |
| src/users/user.module.ts | Removed UserResolver from providers |
| src/users/user.controller.ts | Removed placeholder secured endpoint |
| src/users/models/user.model.ts | Migrated from GraphQL to Swagger decorators |
| src/users/dto/update-user.input.ts | Added Swagger decorators and validation |
| src/users/dto/change-password.input.ts | Migrated to Swagger decorators |
| src/notifications/providers/webpush.provider.ts | Moved provider to dedicated subdirectory |
| src/notifications/providers/firebase.provider.ts | Removed debug logging and moved to subdirectory |
| src/notifications/notifications.service.ts | Updated imports for moved providers |
| src/notifications/notifications.module.ts | Updated imports for moved providers |
| src/notifications/index.ts | Deleted barrel export file |
| src/metadata.ts | Removed user.model import and cleaned up GraphQL plugin metadata |
| src/main.ts | Added URL prefix middleware for /api compatibility |
| src/jobs/models/*.ts | Deleted GraphQL model files |
| src/jobs/job.resolver.ts | Deleted GraphQL resolver |
| src/jobs/job.module.ts | Removed resolver reference and updated comments |
| src/jobs/job.controller.ts | Updated imports, commented out unused guards, improved logging |
| src/jobs/dto/*.ts | Migrated all DTOs from GraphQL to Swagger decorators |
| src/jobs/args/job.args.ts | Converted from GraphQL ArgsType to REST query parameters |
| src/gql-config.service.ts | Deleted GraphQL config service |
| src/generated/nestjs-dto/*.ts | Removed JobSource-related DTOs |
| src/generated/nestjs-dto/company.entity.ts | Formatting inconsistencies in generated file |
| src/common/redis/redis.service.ts | Removed unused capitalize utility function |
| src/common/redis/redis.providers.ts | Improved password handling with length check |
| src/common/pagination/*.ts | Migrated from GraphQL to Swagger decorators |
| src/common/order/*.ts | Deleted GraphQL-specific ordering files |
| src/common/models/*.ts | Migrated base models to Swagger, removed ApiResponse interface |
| src/common/decorators/user.decorator.ts | Changed from GraphQL context extraction to HTTP request |
| src/common/decorators/field.decorator.ts | Removed GraphQL Field decorator usage |
| src/common/decorators/api.decorator.ts | Commented out RedisAuthGuard usage |
| src/common/decorators/api-nested-query.decorator.ts | Deleted unused decorator file |
| src/common/database/pagination/*.ts | Deleted unused pagination interceptor and models |
| src/common/database/mongo-database.service.ts | Deleted unused MongoDB service |
| src/common/database/database.service.ts | Added password redaction in connection logging |
| src/common/configs/*.ts | Removed GraphQL config, moved UnflattenApplicationConfig, changed env override behavior |
| src/auth/models/*.ts | Migrated token and auth models from GraphQL to Swagger |
| src/auth/gql-auth.guard.ts | Deleted GraphQL auth guard |
| src/auth/dto/*.ts | Migrated all auth DTOs from GraphQL to Swagger |
| src/auth/auth.resolver.ts | Deleted GraphQL resolver |
| src/auth/auth.module.ts | Removed resolver and GqlAuthGuard from providers |
| src/auth/auth.controller.ts | Updated import path for GoogleAuthInput |
| src/app.resolver.ts | Deleted GraphQL resolver |
| src/app.module.ts | Switched to @nestjs/config ConfigModule, added debug logging |
| pnpm-lock.yaml | Removed all GraphQL and Apollo dependencies |
| package.json | Removed GraphQL dependencies, added dev script |
| nest-cli.json | Removed GraphQL plugin from build plugins |
| docker-entrypoint.sh | Added DATABASE_URL construction and enhanced migration debugging |
| default.json | Removed GraphQL configuration blocks |
| .gitignore | Added google-services.json and bak/ directory |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| //import { RedisAuthGuard } from '../auth/redis-auth.guard'; | ||
| import { UserEntity as User } from '../common/decorators/user.decorator'; | ||
| import { ApiPaginationQuery } from './../common/decorators/api-nested-query.decorator'; | ||
| //import { ApiPaginationQuery } from './../common/decorators/api-nested-query.decorator'; |
There was a problem hiding this comment.
Commented-out imports on lines 22 and 24 should be removed entirely. If RedisAuthGuard and ApiPaginationQuery are no longer needed after the GraphQL removal, delete these commented lines rather than leaving them as dead code.
| //import { Field, ObjectType } from '@nestjs/graphql'; | ||
| //import { GraphQLJWT } from 'graphql-scalars'; | ||
| //const J = require('joi'); | ||
|
|
||
| @ObjectType() | ||
| //@ObjectType() | ||
| export class Token { | ||
| @Field(() => GraphQLJWT, { description: 'JWT access token' }) | ||
| //@Field(() => GraphQLJWT, { description: 'JWT access token' }) | ||
| accessToken: string; | ||
|
|
||
| @Field(() => GraphQLJWT, { description: 'JWT refresh token' }) | ||
| //@Field(() => GraphQLJWT, { description: 'JWT refresh token' }) | ||
| refreshToken: string; |
There was a problem hiding this comment.
Commented-out GraphQL decorators and imports should be removed entirely from this file. Lines 1-3, 5, 7, and 10 contain dead code that serves no purpose after the GraphQL removal. Clean removal would improve code clarity.
| import { verify } from 'jsonwebtoken'; | ||
|
|
||
| import { access_denied, account_blocked, access_token_expired_signature, authorization_failed } from '../common/errors'; | ||
| //import { access_denied, account_blocked, access_token_expired_signature, authorization_failed } from '../common/errors'; |
There was a problem hiding this comment.
The commented-out import on line 11 should be removed entirely rather than left as a comment. Dead code in comments reduces code clarity and can confuse future developers about whether this import is needed.
| // Routes are registered under /api/* via RouterModule, so this lets both | ||
| // `/jobs` and `/api/jobs` work without changing controllers. | ||
| // NOTE: We exclude Swagger paths from rewriting. | ||
| const swaggerPath = swaggerConfig.path || 'api'; |
There was a problem hiding this comment.
The swaggerPath variable is defined on line 87 but never used. The middleware doesn't actually exclude Swagger paths from rewriting, despite the comment on line 86 stating that it does. This could cause issues if Swagger paths are meant to be excluded. Either implement the Swagger path exclusion logic or remove the misleading comment and unused variable.
| // Inject parsed values into process.env. Values from the selected env file | ||
| // OVERRIDE existing process.env values to ensure .env.local takes precedence | ||
| // over .env (which Prisma/dotenv may have auto-loaded). | ||
| for (const [k, v] of Object.entries(dotenvConfig)) { | ||
| if (process.env[k] === undefined) { | ||
| process.env[k] = v; | ||
| } | ||
| process.env[k] = v; | ||
| } |
There was a problem hiding this comment.
The config helper now unconditionally overrides all process.env values with values from the .env file (line 43). This is a breaking change from the previous behavior where process.env values took precedence if already set. This could cause issues in container environments where environment variables are set externally (e.g., Kubernetes secrets, Docker Compose). Consider documenting this behavior change prominently, or reverting to only setting values when they don't already exist in process.env.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
digitalnomad91
deleted the
refactor/remove-graphql-rest-consolidation
branch
Overview
This PR captures a broad architectural refactor that removes the remaining GraphQL-facing surface area and consolidates the API around REST + Swagger DTOs/decorators. It also includes runtime/bootstrap hardening and provider/module cleanup.
The work was split into 4 major commits to keep review focused:
refactor(graphql): remove GraphQL stack and related dependenciesrefactor(api): migrate DTOs and decorators to REST/Swaggerrefactor(notifications): move providers into dedicated folderchore(runtime): update env/bootstrap, db logging, and platform wiringDetailed changelog
1) GraphQL decommissioning
src/app.resolver.tssrc/auth/auth.resolver.tssrc/jobs/job.resolver.tssrc/users/user.resolver.tssrc/jobs/models/*(legacy GraphQL-oriented models)src/auth/gql-auth.guard.tssrc/gql-config.service.tsdefault.jsonGraphQL blocks removedsrc/common/configs/config.ts/ related typing updatesnest-cli.json).package.json,pnpm-lock.yaml).2) REST/Swagger DTO + decorator migration
src/common/decorators/api.decorator.tssrc/common/decorators/field.decorator.tssrc/common/decorators/user.decorator.tssrc/common/pagination/*andsrc/common/models/*.src/auth/dto/oauth-google.dto.tsand switched auth controller to it.3) Notifications provider/module cleanup
src/notifications/*.provider.tssrc/notifications/providers/*.provider.tssrc/notifications/index.tsbarrel.4) Runtime / platform hardening
DATABASE_URL/SHADOW_DATABASE_URLwhen only component env vars are present./apirouting inmain.ts.tsconfig.json.Notes for reviewers
.nogic/tsconfig.build.tsbuildinfoValidation