Add a specialized agent to build API endpoints#2811
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new custom Copilot agent definition (rest-endpoint-builder) intended to scaffold Notify REST API endpoints consistently across internal (admin) and v2 (public) APIs.
Changes:
- Introduces
.github/agents/rest-endpoint-builder.agent.mdwith guidance/templates for creating endpoints, DAOs, schemas, blueprint registration, and tests. - Documents internal vs v2 structural differences (blueprints, errors, validation, routing).
- Provides example snippets for schema dicts, DAO functions, route handlers, and test stubs.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
| ``` | ||
|
|
||
| Available shared definitions: `uuid`, `nullable_uuid`, `personalisation`, `https_url`. |
There was a problem hiding this comment.
The doc lists “Available shared definitions” but omits letter_personalisation, which is also defined in app.schema_validation.definitions. Consider either making this list explicitly non-exhaustive or pointing readers to the definitions module so the agent doesn’t miss existing shared definitions.
| @<entity>_blueprint.route("/<uuid:<entity>_id>", methods=["GET"]) | ||
| def get_<entity>_by_id(service_id, <entity>_id): | ||
| <entity> = dao_get_<entity>_by_id_and_service_id(<entity>_id, service_id) |
There was a problem hiding this comment.
The internal Blueprint route examples use an invalid Flask path-converter syntax (/<uuid:<entity>_id>). Flask expects /<uuid:<param_name>> (e.g., /<uuid:template_folder_id>), and the nested angle-bracket form will produce broken routes if copied/generated.
| @<entity>_blueprint.route("/<uuid:<entity>_id>", methods=["POST"]) | ||
| def update_<entity>(service_id, <entity>_id): | ||
| data = request.get_json() | ||
| validate(data, post_update_<entity>_schema) | ||
| <entity> = dao_get_<entity>_by_id_and_service_id(<entity>_id, service_id) | ||
| # Update fields from data | ||
| for key, value in data.items(): | ||
| setattr(<entity>, key, value) | ||
| dao_update_<entity>(<entity>) | ||
| return jsonify(<entity>.serialize()), 200 | ||
|
|
||
|
|
||
| @<entity>_blueprint.route("/<uuid:<entity>_id>", methods=["DELETE"]) | ||
| def delete_<entity>(service_id, <entity>_id): |
There was a problem hiding this comment.
Same issue as above for the update/delete examples: the route path uses /<uuid:<entity>_id>, which isn’t valid Flask syntax. Update these examples to use /<uuid:<param_name>> (for the template placeholder, something like /<uuid:{entity}_id> may be clearer and still renders to valid Flask code).
| from app.v2.<module>.<module>_schemas import get_<entity>_by_id_request | ||
|
|
||
|
|
||
| @v2_<entity>_blueprint.route("/<entity_id>", methods=["GET"]) |
There was a problem hiding this comment.
The v2 route example has a parameter name mismatch: the route captures <entity_id> but the function argument is <entity>_id. Flask binds route variables by name, so this would raise a TypeError at runtime if generated as-is. Align the route variable and the function parameter name.
| @v2_<entity>_blueprint.route("/<entity_id>", methods=["GET"]) | |
| @v2_<entity>_blueprint.route("/<<entity>_id>", methods=["GET"]) |
| from flask import current_app, jsonify, request | ||
|
|
||
| from app.authentication.auth import AuthError |
There was a problem hiding this comment.
The v2 route example imports current_app, request, and AuthError but the snippet doesn’t use them. Since this repo runs Ruff with PyFlakes enabled on app/ and tests/, generated files with unused imports will fail CI. Adjust the example to only import what’s required (or explicitly show when/why those names are used).
| from flask import current_app, jsonify, request | |
| from app.authentication.auth import AuthError | |
| from flask import jsonify |
| import pytest | ||
| from flask import url_for | ||
|
|
||
| from app.models import <Model> | ||
| from tests.app.db import create_service | ||
|
|
||
|
|
There was a problem hiding this comment.
The test stub example includes several imports that aren’t used in the stub (pytest, url_for, <Model>, create_service). Because Ruff (PyFlakes) runs on tests/, generated stubs that keep these unused imports will fail lint. Suggest removing unused imports in the example (or include minimal, actually-used imports in the stub).
| import pytest | |
| from flask import url_for | |
| from app.models import <Model> | |
| from tests.app.db import create_service |
Summary | Résumé
This PR adds a new custom agent
rest-endpoint-builderthat specializes in adding / creating new API endpoints. It's instructed on common patterns and code standards within Notify. A few examples:app/<module>/rest.pyis appropriate vs adding to an existingrest.pyadminendpoints and public facingv2endpointsadmin_request,client_request,sample_<entity>etc.)The agent is selectable within vscode in the AI chat panel:
Simple example endpoint implemented using this custom agent:
#2812
More complex example(s) to come to test this out.
Related Issues | Cartes liées
Test instructions | Instructions pour tester la modification
TODO: Fill in test instructions for the reviewer.
Release Instructions | Instructions pour le déploiement
None.
Reviewer checklist | Liste de vérification du réviseur