feat(finance): add functionality to fetch and display recurring finan…#131
feat(finance): add functionality to fetch and display recurring finan…#131PhantomDave merged 3 commits intomainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds functionality to fetch and display recurring finance records separately from regular transactions. It introduces a new GraphQL query that retrieves only active recurring templates (records with IsRecurring=true that haven't expired) and displays them in the "Recurring Movements" section of the tracking component. This complements the existing system where recurring templates are materialized into instances by the background service.
Key Changes:
- Added backend query
GetAllRecurringFinanceRecordsto fetch active recurring templates filtered by account and expiration date - Created frontend service method
getAllRecurringFinanceRecords()with dedicated signal for recurring records state - Modified tracking component to filter regular movements (excluding templates) and fetch/display recurring templates separately
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
PhantomDave.BankTracking.Api/Types/Queries/FinanceRecordQueries.cs |
Added GetAllRecurringFinanceRecords GraphQL query resolver with authorization |
PhantomDave.BankTracking.Api/Services/FinanceRecordService.cs |
Added GetAllRecurringFinanceRecordsAsync service method with filtering logic for active recurring templates |
frontend/src/app/models/finance-record/gql/get-recurring-finance-records.query.graphql |
Defined GraphQL query operation to fetch all recurring finance record fields |
frontend/src/app/models/finance-record/finance-record-service.ts |
Added GQL injection, signal state, and async method to fetch and store recurring records |
frontend/src/app/components/tracking/tracking/tracking.component.ts |
Added OnInit lifecycle hook to fetch recurring records; filtered regular movements to exclude templates; added computed signal for recurring records |
frontend/src/app/components/tracking/tracking/tracking.component.html |
Updated recurring movements component to display data from new recurring records signal |
frontend/src/app/models/finance-record/finance-record-service.ts
Outdated
Show resolved
Hide resolved
frontend/src/app/models/finance-record/finance-record-service.ts
Outdated
Show resolved
Hide resolved
frontend/src/app/models/finance-record/finance-record-service.ts
Outdated
Show resolved
Hide resolved
PhantomDave.BankTracking.Api/Types/Queries/FinanceRecordQueries.cs
Outdated
Show resolved
Hide resolved
frontend/src/app/models/finance-record/gql/get-recurring-finance-records.query.graphql
Outdated
Show resolved
Hide resolved
|
@copilot open a new pull request to apply changes based on the comments in this thread |
|
@PhantomDave I've opened a new pull request, #132, to work on those changes. Once the pull request is ready, I'll request review from you. |
…132) Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: PhantomDave <34485699+PhantomDave@users.noreply.github.com>
| @@ -18,7 +18,7 @@ | |||
| style="width: 100%" | |||
| [title]="'Recurring Movements'" | |||
| [loading]="loading()" | |||
There was a problem hiding this comment.
The template passes loading() to the recurring movements component, but this signal represents the loading state of regular finance records, not recurring ones. This should use financeRecordService.recurringLoading() instead to accurately reflect the loading state of recurring records.
Consider defining a computed signal in the component:
readonly recurringLoading = computed(() => this.financeRecordService.recurringLoading());Then update the template to use [loading]="recurringLoading()".
| [loading]="loading()" | |
| [loading]="recurringLoading()" |
| } catch (error) { | ||
| this._recurringError.set(`Failed to fetch recurring finance records: ${error}`); |
There was a problem hiding this comment.
The error message template literal interpolates the error object directly, which will result in unhelpful messages like "Failed to fetch recurring finance records: [object Object]" for GraphQL errors.
Consider extracting a meaningful error message:
const errorMessage = error instanceof Error ? error.message : String(error);
this._recurringError.set(`Failed to fetch recurring finance records: ${errorMessage}`);This pattern is inconsistent with other error handlers in the service (e.g., lines 87, 172, 196) which don't include error details, so you might also consider just using the static message like the other handlers.
| } catch (error) { | |
| this._recurringError.set(`Failed to fetch recurring finance records: ${error}`); | |
| } catch { | |
| this._recurringError.set('Failed to fetch recurring finance records'); |
…ce records