[Bug 35788] New: Remove Koha::Template::Plugin::Biblio::BookingsCount
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35788 Bug ID: 35788 Summary: Remove Koha::Template::Plugin::Biblio::BookingsCount Change sponsored?: --- Product: Koha Version: unspecified Hardware: All OS: All Status: ASSIGNED Severity: enhancement Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: jonathan.druart+koha@gmail.com Reporter: jonathan.druart+koha@gmail.com QA Contact: testopia@bugs.koha-community.org Depends on: 35787 Blocks: 35781 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35781 [Bug 35781] [omnibus] Remove unneeded Koha::Template::Plugin methods https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35787 [Bug 35787] Remove Koha::Template::Plugin::Biblio::CanBook -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35788 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35788 --- Comment #1 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 160961 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=160961&action=edit Bug 35788: Remove Koha::Template::Plugin::Biblio::BookingsCount We need a new method filter_by_future in Koha::Bookings. Additionally this TT plugin's method was not covered by tests. Neither is the whole Koha::Booking[s] objects btw. Test plan: Confirm that the number of bookings is displayed in the "Bookings" tab in the left menu of the bibliographic record detail page. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35788 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |martin.renvoize@ptfs-europe | |.com, tomascohen@gmail.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35788 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35788 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #160961|0 |1 is obsolete| | --- Comment #2 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 161039 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=161039&action=edit Bug 35788: Remove Koha::Template::Plugin::Biblio::BookingsCount We need a new method filter_by_future in Koha::Bookings. Additionally this TT plugin's method was not covered by tests. Neither is the whole Koha::Booking[s] objects btw. Test plan: Confirm that the number of bookings is displayed in the "Bookings" tab in the left menu of the bibliographic record detail page. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35788 --- Comment #3 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 161040 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=161040&action=edit Bug 35788: Test for current day booking The implemented tests checked past and future dates for filtering bookings. But `> NOW()` excludes the current date, and this case was not covered. It is now. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35788 --- Comment #4 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 161041 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=161041&action=edit Bug 35788: Simplify code We should not load DateUtils and deal with DateTime objects and formatting when we can use the DB for it, using \'NOW()' should be enough. This patch makes that change. To test: 1. Apply the previous patches 2. Run: $ ktd --shell k$ prove t/db_dependent/Koha/Bookings.t => SUCCESS: Tests pass! 3. Apply this patch 4. Repeat 2 => SUCCESS: Tests pass! 5. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35788 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |35789 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35789 [Bug 35789] Remove Koha::Template::Plugin::Biblio::ArticleRequestsActiveCount -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35788 --- Comment #5 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Jonathan: if you agree with the follow-up, I'd say this is PQA. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35788 --- Comment #6 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- I have a branch with tests in progress, they need submitting up.. I'll get in that. Thanks for this patchset chaps, looking good to me. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35788 --- Comment #7 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to Tomás Cohen Arazi from comment #5)
Jonathan: if you agree with the follow-up, I'd say this is PQA.
I don't know, we never decided what was the correct pattern. I always use format_datetime and dt_from_string for this. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35788 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |29002 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29002 [Bug 29002] Add ability to book items ahead of time -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35788 --- Comment #8 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- I think Jonathan is right here.. as much as I'd love to just lean on the database to handle this, it doesn't take our timezone handling into account.. These are DATETIME fields, not TIMESTAMPS.. so there's no internal munging of the dates to UTC and back.. we need the koha munging of that to consistently get back the datetimes we put in. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35788 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA --- Comment #9 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- OK, I've been talked around having been shown the DB connection logic where we pass timezone information on connect. I think the use of \'NOW()' is a little more performant so am happy with the follow-up. I'd love to see a coding guideline around this at some point, it would be nice to clean up existing cases where we could use this and currently don't. Consistency is good to aim for. As such.. I'll switch to PQA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35788 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |martin.renvoize@ptfs-europe |y.org |.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35788 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on|29002 | Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29002 [Bug 29002] Add ability to book items ahead of time -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35788 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |35248 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35248 [Bug 35248] Bookings needs unit tests -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35788 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks|35781 | Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35781 [Bug 35781] [omnibus] Remove unneeded Koha::Template::Plugin methods -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35788 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |david@davidnind.com Status|Passed QA |Patch doesn't apply --- Comment #10 from David Nind <david@davidnind.com> --- Patch doesn't apply - sha1/fake ancestor error: ... Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 35788: Remove Koha::Template::Plugin::Biblio::BookingsCount error: sha1 information is lacking or useless (Koha/Template/Plugin/Biblio.pm). error: could not build fake ancestor Patch failed at 0001 Bug 35788: Remove Koha::Template::Plugin::Biblio::BookingsCount -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35788 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- URL| |https://github.com/PTFS-Eur | |ope/koha/tree/bug_35788 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35788 --- Comment #11 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Branch pushed to https://github.com/PTFS-Europe/koha/tree/bug_35788 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35788 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Patch doesn't apply |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35788 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |24.05.00 released in| | Status|Passed QA |Pushed to master -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35788 --- Comment #12 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Pushed for 24.05! Well done everyone, thank you! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35788 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to master |Needs documenting CC| |fridolin.somers@biblibre.co | |m --- Comment #13 from Fridolin Somers <fridolin.somers@biblibre.com> --- Enhancement not pushed to 23.11.x -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35788 Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |caroline.cyr-la-rose@inlibr | |o.com Resolution|--- |FIXED Status|Needs documenting |RESOLVED --- Comment #14 from Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com> --- Architecture change, nothing to add/edit in the Koha manual. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35788 Bug 35788 depends on bug 35787, which changed state. Bug 35787 Summary: Remove Koha::Template::Plugin::Biblio::CanBook https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35787 What |Removed |Added ---------------------------------------------------------------------------- Status|Needs documenting |RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org