Summary: | Add a count to the Files tab in the patron account | ||
---|---|---|---|
Product: | Koha | Reporter: | Lucas Gass (lukeg) <lucas> |
Component: | Patrons | Assignee: | Lucas Gass (lukeg) <lucas> |
Status: | BLOCKED --- | QA Contact: | Testopia <testopia> |
Severity: | enhancement | ||
Priority: | P5 - low | CC: | barbara.johnson, clackman, gmcharlt, jheltibridle, kyle.m.hall, m.de.rooy, martin.renvoize |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
GIT URL: | Change sponsored?: | --- | |
Patch complexity: | Small patch | Documentation contact: | |
Documentation submission: | Text to go in the release notes: | ||
Version(s) released in: | Circulation function: | ||
Bug Depends on: | 40302 | ||
Bug Blocks: | |||
Attachments: |
Bug 30553: Add file count to patron circ-menu
Bug 30553: Add file count to patron circ-menu Bug 30510: Return only the count from the new filecount method Bug 30553: Unit tests |
Description
Lucas Gass (lukeg)
2022-04-15 14:34:31 UTC
Created attachment 133365 [details] [review] Bug 30553: Add file count to patron circ-menu To test: 1. Apply patch 2. Enable the system preference 'EnableBorrowerFiles' 3. Go to a patron account and noticet the Files tab on left nav bar. 4. It should read "Files (0)" 5. Begin uploading some files to the borrower. The tab should reflect the number of files uploaded. 6. Go to other pages on the borrower account ( Checkout, Details, etc. ) 7. The number of files should display from all borrower pages Created attachment 133480 [details] [review] Bug 30553: Add file count to patron circ-menu To test: 1. Apply patch 2. Enable the system preference 'EnableBorrowerFiles' 3. Go to a patron account and noticet the Files tab on left nav bar. 4. It should read "Files (0)" 5. Begin uploading some files to the borrower. The tab should reflect the number of files uploaded. 6. Go to other pages on the borrower account ( Checkout, Details, etc. ) 7. The number of files should display from all borrower pages Signed-off-by: Barbara Johnson <barbara.johnson@bedfordtx.gov> Hi Lucas, someone else might be able to help better here, but this leaves some question marks for me. 1) We are missing unit tests for the new method. 2) The new method is named filecount, but it doesn't appear to return an actual count, as then in the templates we have: patron.filecount.size I believe, either the name is not quite right or the return value should be the number. Maybe it should be patron_files instead? Cait, Thanks for looking. I think, unless there is another reason, we should just return the file count. I am fine with either, I just felt like here was some mismatch here. Created attachment 133847 [details] [review] Bug 30510: Return only the count from the new filecount method Created attachment 134287 [details] [review] Bug 30553: Unit tests Hmm, my initial thought here is what are we grabbing so much data for just a count? I see Koha::Patron::Files isn't a modern Koha::Object based class, more of a half way between C4 module and an object.. so it's not as simple as it initially looks. However, maybe it's worth going half way and introducing a 'GetFileCount' method in the underlying Koha::Patron::Files class which does the simpler 'SELECT COUNT(*) FROM borrower_files WHERE borrowernumber = ?' instead? Hm, but no SQL in Koha namespace, I think. The DBIX equivalent? FQA = Feedback for QA (In reply to Katrin Fischer from comment #9) > Hm, but no SQL in Koha namespace, I think. The DBIX equivalent? I would suggest allowing the SQL in Koha::Patron::Files and using the method from Koha::Patron.. and then having a follow-up bug to convert Koha::Patron::Files to proper Koha::Object based classes as a modernisation effort. To me, that's the most pragmatic route to goal for this simple enhancement. (In reply to Martin Renvoize from comment #11) > (In reply to Katrin Fischer from comment #9) > > Hm, but no SQL in Koha namespace, I think. The DBIX equivalent? > > I would suggest allowing the SQL in Koha::Patron::Files and using the method > from Koha::Patron.. and then having a follow-up bug to convert > Koha::Patron::Files to proper Koha::Object based classes as a modernisation > effort. > > To me, that's the most pragmatic route to goal for this simple enhancement. With the risk that we push this arguable code and the follow=up is delayed ;) Since it is a very simple development, it could probably be done here? This enhancement would be incredibly useful for our library. I would love to see someone pick up the work on it again! I don't think I fully understand what y'all need me to do here. To move this forward do I need to convert Koha::Patron::Files into a modern Koha::Object? Sorry for confusion... |