Major Refactor #143
Major Refactor #143
Conversation
There was a problem hiding this comment.
Didn't @sabarixr say AttendanceSummary was necessary? I remember thinking it was bloat too though
I had confirmed with him that this query wasn't being used before removing it |
There was a problem hiding this comment.
Pull Request Overview
This PR implements a major refactor of the Root GraphQL API to improve code organization, follow GraphQL best practices, and update streak calculation logic. The refactor consolidates queries into fewer modules, removes unused functionality, and restructures data models for better API design.
Key changes:
- Removed project and legacy streak functionality that was no longer needed
- Consolidated GraphQL queries and mutations into fewer, more focused modules
- Updated streak calculation to use a new SQL-based approach instead of relying on downstream clients
- Renamed database fields for consistency (e.g.,
is_updatedtois_sent)
Reviewed Changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/models/status_update.rs | Simplified status update models, removing streak-related structs and renaming fields |
| src/models/project.rs | Removed entire project model as it's no longer needed |
| src/models/attendance.rs | Simplified attendance model by removing summary and info structs |
| src/graphql/queries/member_queries.rs | Major restructuring with new streak calculation logic and consolidated query patterns |
| src/graphql/queries/streak_queries.rs | Removed entire file as streak logic moved to member queries |
| src/graphql/queries/project_queries.rs | Removed entire file as project functionality was removed |
| src/graphql/mutations/status_mutations.rs | New file implementing status update mutations |
| src/graphql/mutations/streak_mutations.rs | Removed entire file as streak logic was refactored |
| src/graphql/mutations/project_mutations.rs | Removed entire file as project functionality was removed |
| src/database_seeder/seed.sql | Updated to reflect new database schema without unnecessary tables |
| migrations/ | Database migration scripts to drop tables and rename columns |
| docs/attendance.md | Updated documentation to reflect removed functionality |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Going ahead with the merge then, docs are still out of date though, could probably assign one of the probated members :P |
Breaking Change: The manual handling of the status update streaks has been removed in anticipation of moving the streak logic to the business layer instead of the client. In its place, the API will only expose a singular mutation to mark a particular day's status update status for the member.
The streak calculation has been moved into the database layer using a gaps-and-islands CTE. Hence streaks can now be dynamically generated from the existing status update history data and thus the existing streak tables are now redundant.
The project and statusupdatestreak tables are now redundant and hence can be dropped from the database
2 participants
This PR refactors/rewrites significant portions of Root, the motivations for the same are varied: