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)
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...