Bug 30553

Summary: Add a count to the Files tab in the patron account
Product: Koha Reporter: Lucas Gass (lukeg) <lucas>
Component: PatronsAssignee: 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
If you have the sys pref EnableBorrowerFiles turned on you will see a tab for 'Files' on the patroninfo navigation bar. Some librarians would find it useful to have count on that tab so they can see how many files a borrower has. 

i.e.: Files(2)
Comment 1 Lucas Gass (lukeg) 2022-04-15 21:29:43 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
Comment 2 ByWater Sandboxes 2022-04-20 14:47:20 UTC
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>
Comment 3 Katrin Fischer 2022-04-25 10:52:16 UTC
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?
Comment 4 Lucas Gass (lukeg) 2022-04-25 22:42:19 UTC
Cait,

Thanks for looking. I think, unless there is another reason, we should just return the file count.
Comment 5 Katrin Fischer 2022-04-25 22:43:44 UTC
I am fine with either, I just felt like here was some mismatch here.
Comment 6 Lucas Gass (lukeg) 2022-04-25 22:45:28 UTC
Created attachment 133847 [details] [review]
Bug 30510: Return only the count from the  new filecount method
Comment 7 Lucas Gass (lukeg) 2022-04-28 17:54:45 UTC
Created attachment 134287 [details] [review]
Bug 30553: Unit tests
Comment 8 Martin Renvoize (ashimema) 2022-04-29 09:57:39 UTC
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?
Comment 9 Katrin Fischer 2022-04-29 10:39:01 UTC
Hm, but no SQL in Koha namespace, I think. The DBIX equivalent?
Comment 10 Katrin Fischer 2022-04-29 10:58:42 UTC
FQA = Feedback for QA
Comment 11 Martin Renvoize (ashimema) 2022-04-29 12:42:40 UTC
(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.
Comment 12 Marcel de Rooy 2022-04-29 15:05:30 UTC
(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?
Comment 13 Juliet Heltibridle 2023-10-20 14:34:02 UTC
This enhancement would be incredibly useful for our library. I would love to see someone pick up the work on it again!
Comment 14 Lucas Gass (lukeg) 2023-10-24 20:07:12 UTC
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...