-
Notifications
You must be signed in to change notification settings - Fork 23
Ck 7vn/id 508 #1042
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
Ck 7vn/id 508 #1042
Conversation
e68e2e7 to
ffb4cce
Compare
carddev81
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.
Comments
- Not sure what you thoughts are on this, I think that the reports should be labeled with the word 'Report' for example "Export Facility Comparison Report" and I also think the reports should be centralized in one location. I liked your original idea of creating a report link and centralizing the reports. I just wanted to note this and get everyone else's opinion (@calisio and @corypride ).
- Maybe for the Status we could use human readable value 'Absent Excused' rather than the coding syntax of 'absent_excused'. Also for any code type we should come up with user friendly format such as changing Life_Skills to 'Life Skills', ect. I think as a future enhancement we should utilize code tables that contains the code, label, description, start_dt, end_dt, ect. This would be easier to maintain.
- I think the reports should contain our logo (backend/src/assets/ul-symbol-k.png) and a header ie 'Facility Comparison Report'. (Just a thought--I would like the addition of the filters that the user selected to be included at the top of the report)
- Go version has been updated which is great. I was hoping this was going to happen. Can you please also upgrade the following silos of code (for consistency across the code base): migrations and seeder.
I deferred the centralized reports page, and the code tables for enum management. What I did fix/change was
|
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.
f0adb7b to
1447ec4
Compare
corypride
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.
Tests good!
carddev81
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.
Approving this but just want us to be aware that the text doesn't wrap correctly when text characters exceed a certain threshold. @corypride s PR Jasper Report integration will take care of this once these PDF reports are converted over to Jasper.







Description of the change
This PR introduces a modular approach to building and providing reports in CSV, PDF, and Excel formats.
Features
3 Report Types: Attendance, Program Outcomes, and Facility Comparison
Role-Based Access Control: Facility scoping for facility admins
Date Range Filtering: Configurable reporting periods
Audit logging: Export tracking
Comprehensive Test Coverage : Database level regression tests
Approach
Chose a modular object oriented design with the understanding reporting will be an ever-evolving feature of our application growing quickly in importance and that the 3 currently implemented report types are just the beginning.
Screenshot(s)
Drive with generated reports: https://drive.google.com/drive/folders/1cyxppr4X5ru6dtENFVttLFUSltMY23O6?usp=drive_link
Additional context
Report Details
Test coverage
Database tests
This is a very large ticket, and it is doing a lot of things, it was built with the future in mind as much as it was built for this moment, It has been a very long week doing it, so don't be surprised if I missed something, and I just ask for some grace
Also updated the Go version in quite a few places as well