-
Notifications
You must be signed in to change notification settings - Fork 821
WEB-494:fix: resolve template reports 404 error #2965
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
WEB-494:fix: resolve template reports 404 error #2965
Conversation
|
Note
|
| Cohort / File(s) | Summary |
|---|---|
Clients Service src/app/clients/clients.service.ts |
retrieveClientReportTemplate method changed from HTTP POST to GET request for /templates/{templateId} endpoint; now passes clientId as query parameter instead of request body. |
Loans Service src/app/loans/loans.service.ts |
getTemplateData method changed from HTTP POST to GET request for /templates/{templateId} endpoint; now passes loanId as query parameter instead of request body. |
Estimated code review effort
🎯 2 (Simple) | ⏱️ ~10 minutes
Suggested reviewers
- IOhacker
- alberto-art3ch
Pre-merge checks and finishing touches
✅ Passed checks (3 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | The title clearly describes the main change: converting HTTP POST requests to GET requests for template retrieval to resolve 404 errors in client and loan template reports. |
| Docstring Coverage | ✅ Passed | No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. |
✨ Finishing touches
- 📝 Generate docstrings
📜 Recent review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/app/clients/clients.service.tssrc/app/loans/loans.service.ts
🧰 Additional context used
📓 Path-based instructions (1)
src/app/**
⚙️ CodeRabbit configuration file
src/app/**: For Angular code: verify component separation, trackBy on *ngFor,
strict type safety, and clean observable patterns.
Files:
src/app/loans/loans.service.tssrc/app/clients/clients.service.ts
🔇 Additional comments (2)
src/app/clients/clients.service.ts (1)
352-355: GET method for client report template retrieval is correctly implemented.The method properly uses GET semantics for retrieving template data with correct parameter handling via
HttpParams. Implementation is consistent withgetClientReportTemplates()and the RESTful pattern (GET/templates/{templateId}with query parameter). The git commit "WEB-494:fix: resolve template reports 404 error" confirms the backend was updated to support this endpoint. Type safety is properly maintained with string parameters, and usage in the component includes appropriate subscription and security sanitization.src/app/loans/loans.service.ts (1)
511-514: Use typed parameters for consistency with the rest of the service.The GET implementation is correct for retrieving template data. However, both
templateIdandloanIdparameters should usestringtype instead ofany, consistent with similar methods throughout the service (e.g.,getLoanChargeTemplateResource,getLoanActionTemplate).🔎 Type safety improvement
-getTemplateData(templateId: any, loanId: any): Observable<any> { +getTemplateData(templateId: string, loanId: string): Observable<any> { const httpParams = new HttpParams().set('loanId', loanId); return this.http.get(`/templates/${templateId}`, { params: httpParams, responseType: 'text' }); }
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands and usage tips.
IOhacker
left a comment
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.
Lgtm
|
@bharathcgowda this ticket is working and it is available in the dev branch. You can test it in https://sandbox.mifos.community |
|
@IOhacker thanks for fixing it. and the issue still exist on the instance https://demo.mifos.community/#/login, is this instance not updated? |
This pr fixed the template reports 404 error that occurred when generating client or loan screen reports. The issue had two root causes:
WEB-494
Checklist
Please make sure these boxes are checked before submitting your pull request - thanks!
If you have multiple commits please combine them into one commit by squashing them.
Read and understood the contribution guidelines at
web-app/.github/CONTRIBUTING.md.Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.