-
Notifications
You must be signed in to change notification settings - Fork 0
Extend API and TypeDB schema #3
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 significantly extends the API and TypeDB schema to support a comprehensive user and group management system. The changes introduce hierarchical groups, membership relationships, and transitive queries for complex organizational structures.
- Implements core data models for users, groups, and memberships with support for nested groups
- Adds comprehensive API endpoints for user/group management and membership operations
- Introduces transitive query capabilities for inherited group memberships
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_lambda.py | Comprehensive test suite covering user creation, group management, and membership operations |
| main.tf | Terraform infrastructure for all new API Gateway resources and endpoints |
| app/lambda/schema.tql | Complete TypeDB schema defining users, groups, memberships, and recursive functions |
| app/lambda/handler.py | Lambda handler implementing all API endpoints with proper routing and error handling |
| README.md | Updated setup instructions with TypeDB development mode configuration |
| Makefile | Updated build process and test targets for the expanded functionality |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| resource "aws_api_gateway_resource" "user_groups_resource" { | ||
| rest_api_id = aws_api_gateway_rest_api.user_api.id | ||
| parent_id = aws_api_gateway_rest_api.user_api.root_resource_id | ||
| path_part = "users" | ||
| } | ||
|
|
||
| resource "aws_api_gateway_resource" "user_groups_nested_resource" { | ||
| rest_api_id = aws_api_gateway_rest_api.user_api.id | ||
| parent_id = aws_api_gateway_resource.user_groups_resource.id |
Copilot
AI
Sep 29, 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 'user_groups_resource' creates a duplicate '/users' path that already exists as 'user_resource'. This creates resource duplication and potential conflicts. Consider using the existing 'user_resource' as the parent for the nested user groups resources.
| resource "aws_api_gateway_resource" "user_groups_resource" { | |
| rest_api_id = aws_api_gateway_rest_api.user_api.id | |
| parent_id = aws_api_gateway_rest_api.user_api.root_resource_id | |
| path_part = "users" | |
| } | |
| resource "aws_api_gateway_resource" "user_groups_nested_resource" { | |
| rest_api_id = aws_api_gateway_rest_api.user_api.id | |
| parent_id = aws_api_gateway_resource.user_groups_resource.id | |
| # Removed duplicate '/users' resource. Nest under existing 'user_resource'. | |
| resource "aws_api_gateway_resource" "user_groups_nested_resource" { | |
| rest_api_id = aws_api_gateway_rest_api.user_api.id | |
| parent_id = aws_api_gateway_resource.user_resource.id |
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.
Awesome set of changes - love to see the extended sample app schema, as well as the Web UI 🚀
Just one thought - should we add a make target to simplify deploying the Web app..? Still need to look into it and do more testing locally, but potentially there would be an opportunity to simplify the Web app deployment on LocalStack (will follow up with more details soon..)
|
@flyingsilverfin going to merge this PR as-is, but I had some smaller issues running the sample locally. Will create a follow-up PR with some suggested changes shortly. 👍 |
Changes
We add a series of new APIs and capabilities:
Core Data Model
API Endpoints
User Management
Membership Management
Utility
Key Features