-
Notifications
You must be signed in to change notification settings - Fork 0
Technologies sort fix for the category overview + versions data #50
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR implements two major improvements to the Tech Report APIs: (1) adding client-based filtering to the categories endpoint and (2) introducing version filtering capabilities across all report endpoints. The changes also include a significant infrastructure migration from Google Cloud Functions to Cloud Run with containerization.
Key changes:
- Adds
clientparameter filtering (mobile/desktop) to/v1/categoriesendpoint - Implements
/v1/versionsendpoint and adds version filtering to all report endpoints (adoption, CWV, lighthouse, page-weight, audits) - Migrates infrastructure from Cloud Functions to Cloud Run using Docker containers
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| terraform/prod/main.tf | Refactors infrastructure to use modularized API Gateway and Cloud Run service, adds Docker provider configuration |
| terraform/prod/local.auto.tfvars.template | Removes template file (configuration moved elsewhere) |
| terraform/modules/run-service/variables.tf | Updates variables for Cloud Run migration, renames function_name to service_name, changes memory/timeout types |
| terraform/modules/run-service/outputs.tf | Updates output to reference Cloud Run service instead of Cloud Function |
| terraform/modules/run-service/main.tf | Replaces Cloud Functions deployment with Docker-based Cloud Run service deployment |
| terraform/modules/api-gateway/variables.tf | Adds spec_yaml variable and default values for better module reusability |
| terraform/modules/api-gateway/main.tf | Modularizes API Gateway configuration to accept spec_yaml as parameter |
| terraform/dev/variables.tf | Updates default database from prod to dev environment |
| terraform/dev/main.tf | Refactors infrastructure similar to prod, adds /v1/versions endpoint to OpenAPI spec |
| terraform/dev/local.auto.tfvars.template | Removes template file (configuration moved elsewhere) |
| src/package.json | Adds Docker build and run scripts for local development |
| src/controllers/technologiesController.js | Updates cache key to include technology and category filters for proper cache invalidation |
| src/controllers/reportController.js | Implements version filtering for all report types with single-technology constraint |
| src/controllers/categoriesController.js | Adds client parameter filtering and updates cache key data |
| src/tests/routes.test.js | Adds test for version parameter on adoption endpoint |
| src/Dockerfile | Creates containerized deployment configuration for Cloud Run |
| src/.dockerignore | Defines files to exclude from Docker build context |
| README.md | Documents new client and version parameters across multiple endpoints |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| terraform { | ||
| required_providers { | ||
| docker = { | ||
| source = "hashicorp/google-beta" |
Copilot
AI
Dec 6, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The provider name is incorrect. The docker provider is being configured but the source points to hashicorp/google-beta which is the Google Beta provider. This should be source = "kreuzwerker/docker" to match the provider configuration used in the prod and dev main.tf files.
| source = "hashicorp/google-beta" | |
| source = "kreuzwerker/docker" |
| query = query.select('category'); | ||
| } else if (hasCustomFields) { | ||
| const requestedFields = params.fields.split(',').map(f => f.trim()); | ||
| const requestedFields = params.fields.split(',').map(f => f.trim()) || ['category', 'description', 'technologies', 'origins']; |
Copilot
AI
Dec 6, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The fallback value || ['category', 'description', 'technologies', 'origins'] after params.fields.split(',').map(f => f.trim()) will never be used. The split() method always returns an array, even if the string is empty. If the intention is to provide default fields, the check should happen before the split operation: params.fields ? params.fields.split(',').map(f => f.trim()) : ['category', 'description', 'technologies', 'origins']
| const requestedFields = params.fields.split(',').map(f => f.trim()) || ['category', 'description', 'technologies', 'origins']; | |
| const requestedFields = params.fields ? params.fields.split(',').map(f => f.trim()) : ['category', 'description', 'technologies', 'origins']; |
src/Dockerfile
Outdated
| # Install dependencies (this layer will be cached unless package files change) | ||
| RUN npm ci --only=production --quiet --no-fund --no-audit && npm cache clean --force | ||
|
|
||
| ENV DATABASE=tech-report-api-prod |
Copilot
AI
Dec 6, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The hardcoded ENV DATABASE=tech-report-api-prod in the Dockerfile means the dev environment will incorrectly use the production database. This should be parameterized or passed at runtime via the Cloud Run service configuration. The dev environment should use tech-report-api-dev as specified in terraform/dev/variables.tf.
| ENV DATABASE=tech-report-api-prod |
| operationId: getAuditReports | ||
| responses: | ||
| 200: | ||
| description: String |
Copilot
AI
Dec 6, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The production OpenAPI specification is missing the /v1/versions endpoint that was added to the dev environment. This endpoint should be added to ensure consistency between dev and prod environments.
| description: String | |
| description: String | |
| /v1/versions: | |
| get: | |
| summary: versions | |
| operationId: getVersions | |
| responses: | |
| 200: | |
| description: String |
2b77b00 to
d54abc0
Compare
Filtering functionality improvements in
/categoriesclient('mobile') in queries to allow retrieving categories data specific to the client type.Dependent on adjusted report pre-aggregation: HTTPArchive/dataform#120
Example: https://reports-dev-2vzgiib6.uc.gateway.dev/v1/categories?category=CMS&client=desktop (sampled data in dev DB)
Resolves: HTTPArchive/httparchive.org#1081
Added versions data (currently available on
devendpoints only)/versionendpoint, see readmeExample: https://reports-dev-2vzgiib6.uc.gateway.dev/v1/adoption?technology=WordPress&geo=ALL&rank=ALL&version=6.8
versionquery parameter, example