[Bug 42418] New: Add Koha::Checkin(s) â checkin as a first-class resource
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 Bug ID: 42418 Summary: Add Koha::Checkin(s) â checkin as a first-class resource Initiative type: --- Sponsorship --- status: Product: Koha Version: Main Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Circulation Assignee: koha-bugs@lists.koha-community.org Reporter: tomascohen@gmail.com QA Contact: testopia@bugs.koha-community.org CC: gmcharlt@gmail.com, kyle@bywatersolutions.com Depends on: 41728 Blocks: 24401 Checkins in Koha are currently a side effect â they happen to checkouts, transfers, and holds, but there is no single record that captures 'a staff member scanned this item and here is what happened.' This bug introduces checkins as a proper resource: - New `checkins` table tracking the full context (who, what, where, when) and outcomes (checkout returned, transfer triggered, hold filled) - Koha::Checkin and Koha::Checkins object classes - Wire into AddReturn so every checkin path (returns.pl, SIP, API) produces a Koha::Checkin record This addresses several long-standing gaps: - 'Who checked this in?' is currently unreliable (no checkin_user_id on old_issues) - Transfers and holds triggered by a return have no causal link back to the scan - Lost item fee refunds cannot be traced to the return event that caused them - Plugins (e.g. Rapido ILL) have to reconstruct context from side effects Depends on bug 41728 (checkin availability). Blocks bug 24401 (checkin REST API endpoint). Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24401 [Bug 24401] REST API: Check-in https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41728 [Bug 41728] Add `Koha::Item::Checkin::Availability` to centralize logic -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |tomascohen@gmail.com |ity.org | Patch complexity|--- |Small patch Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 --- Comment #1 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 197710 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=197710&action=edit Bug 42418: Add checkins table and Koha::Checkin object Introduces checkins as a first-class resource with its own table, tracking the full context and outcomes of a return scan: - who: user_id (staff member), desk_id (optional) - what: item_id - where: library_id - when: timestamp - outcomes: checkout_id (returned), transfer_id (triggered), hold_id (filled) New files: - Koha::Checkin / Koha::Checkins (Koha::Object classes) - Koha::Schema::Result::Checkin (DBIC, generated via dbic) - atomicupdate/bug_42418.pl + kohastructure.sql Test plan: 1. Apply patch 2. Run updatedatabase.pl 3. Verify checkins table exists with correct structure 4. Sign off :-D Assisted-by: Sonnet 4.6 (Anthropic) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 --- Comment #2 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 197711 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=197711&action=edit Bug 42418: Wire Koha::Checkin into AddReturn Every call to AddReturn now creates a Koha::Checkin record capturing the full context and outcomes of the return scan: - user_id and desk_id from C4::Context->userenv - library_id from the branch parameter - checkout_id from the issue being returned (old_issues) - transfer_id from $messages->{WasTransfered} - hold_id from $messages->{ResFound}{reserve_id} The checkin record is also passed to the after_circ_action plugin hook, so plugins can access it directly instead of reconstructing context from side effects. AddReturn now returns a fifth value: the Koha::Checkin object. Existing callers are unaffected — Perl silently ignores extra return values. Assisted-by: Sonnet 4.6 (Anthropic) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|Add Koha::Checkin(s) â |Add Koha::Checkin(s) - |checkin as a first-class |checkin as a first-class |resource |resource -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 Lisette Scheer <lisette@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |lisette@bywatersolutions.co | |m -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 --- Comment #3 from Lisette Scheer <lisette@bywatersolutions.com> --- Would be helpful to have a clear path to deleted_items so when an item is deleted the link isn't lost. Also might be worth adding a local_use column for flagging if a checkin was a local use. Would make tracking that more effectively, especially if there could be a checkbox or something so local use could be specifically set. I know an adjustment to me when I came to Koha was that my old ils had a separate tab for local_use checkins and things like holds we'd pull and check in to fill the hold wouldn't count as local use since it wasn't really used locally, just pulled by staff to fill the hold. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #197710|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #197711|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 --- Comment #4 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 197715 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=197715&action=edit Bug 42418: Add checkins table and Koha::Checkin object Introduces checkins as a first-class resource with its own table, tracking the full context and outcomes of a return scan: - who: user_id (staff member), desk_id (optional) - what: item_id - where: library_id - when: timestamp - outcomes: checkout_id (returned), transfer_id (triggered), hold_id (filled) New files: - Koha::Checkin / Koha::Checkins (Koha::Object classes) - Koha::Schema::Result::Checkin (DBIC, generated via dbic) - atomicupdate/bug_42418.pl + kohastructure.sql Test plan: 1. Apply patch 2. Run updatedatabase.pl 3. Verify checkins table exists with correct structure 4. Sign off :-D Assisted-by: Sonnet 4.6 (Anthropic) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 --- Comment #5 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 197716 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=197716&action=edit Bug 42418: Wire Koha::Checkin into AddReturn Every call to AddReturn now creates a Koha::Checkin record capturing the full context and outcomes of the return scan: - user_id and desk_id from C4::Context->userenv - library_id from the branch parameter - checkout_id from the issue being returned (old_issues) - transfer_id from $messages->{WasTransfered} - hold_id from $messages->{ResFound}{reserve_id} The checkin record is also passed to the after_circ_action plugin hook, so plugins can access it directly instead of reconstructing context from side effects. AddReturn now returns a fifth value: the Koha::Checkin object. Existing callers are unaffected — Perl silently ignores extra return values. Assisted-by: Sonnet 4.6 (Anthropic) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |tomascohen@gmail.com --- Comment #6 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- (In reply to Lisette Scheer from comment #3)
Would be helpful to have a clear path to deleted_items so when an item is deleted the link isn't lost.
The deleteditems pattern is a known anti-pattern we'd like to move away from. I'm not sure we want to add a FK here perpetuating it. Happy to discuss it in a broader context.
Also might be worth adding a local_use column for flagging if a checkin was a local use. Would make tracking that more effectively, especially if there could be a checkbox or something so local use could be specifically set.
Done! Thanks for bringing that up! I totally missed it, and fits perfectly within the reasons we need a proper checkins table :-D
I know an adjustment to me when I came to Koha was that my old ils had a separate tab for local_use checkins and things like holds we'd pull and check in to fill the hold wouldn't count as local use since it wasn't really used locally, just pulled by staff to fill the hold.
This sounds great. File a bug depending on this one so we discuss it in context! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #197715|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #197716|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 --- Comment #7 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 197722 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=197722&action=edit Bug 42418: Add checkins table and Koha::Checkin object Introduces checkins as a first-class resource with its own table, tracking the full context and outcomes of a return scan: - who: user_id (staff member), desk_id (optional) - what: item_id - where: library_id - when: timestamp - outcomes: checkout_id (returned), transfer_id (triggered), hold_id (filled) New files: - Koha::Checkin / Koha::Checkins (Koha::Object classes) - Koha::Schema::Result::Checkin (DBIC, generated via dbic) - atomicupdate/bug_42418.pl + kohastructure.sql Test plan: 1. Apply patch 2. Run updatedatabase.pl 3. Verify checkins table exists with correct structure 4. Sign off :-D Assisted-by: Sonnet 4.6 (Anthropic) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 --- Comment #8 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 197723 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=197723&action=edit Bug 42418: Wire Koha::Checkin into AddReturn Every call to AddReturn now creates a Koha::Checkin record capturing the full context and outcomes of the return scan: - user_id and desk_id from C4::Context->userenv - library_id from the branch parameter - checkout_id from the issue being returned (old_issues) - transfer_id from $messages->{WasTransfered} - hold_id from $messages->{ResFound}{reserve_id} The checkin record is also passed to the after_circ_action plugin hook, so plugins can access it directly instead of reconstructing context from side effects. AddReturn now returns a fifth value: the Koha::Checkin object. Existing callers are unaffected — Perl silently ignores extra return values. Assisted-by: Sonnet 4.6 (Anthropic) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 --- Comment #9 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Ok, on writing the API found a few more outcomes from AddReturn that should be tracked here. Latest version got added: - Return claims - Recalls - Restrictions This is ready for review! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |martin.renvoize@openfifth.c | |o.uk -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 --- Comment #10 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Can you explain a bit how this will affect old_issues or if there are plans for old_issues down the road? Another solution could have been to add additional columns to old issues as that represents a checkin currently. I guess you had reasons not to go this route, but am a bit worried about the duplicated data. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 --- Comment #11 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- (In reply to Katrin Fischer from comment #10)
Can you explain a bit how this will affect old_issues or if there are plans for old_issues down the road?
Another solution could have been to add additional columns to old issues as that represents a checkin currently. I guess you had reasons not to go this route, but am a bit worried about the duplicated data.
I always think of `old_issues` as the `issues` table with information about a special status of the checkout: returned (which is inferred from a return date being set). With that in mind my only plan is to merge those tables in the mid (?) term. But -regardless of what we do with those tables- this development highlights that the checkin action can (or not) trigger some status change for a checkout, and many more things. All of those nicely handled and tested here :-). Taking accountlines as an example, I will propose we ditch the old_issues link and replace with a checkin link. That way we actually gain tracking possibilities for (say) refunding lost fines even when there might not be a linked checkout, and things like that. The link to old checkouts could be reconstructed through the checkin link. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 --- Comment #12 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- (In reply to Tomás Cohen Arazi (tcohen) from comment #11)
(In reply to Katrin Fischer from comment #10)
Can you explain a bit how this will affect old_issues or if there are plans for old_issues down the road?
Another solution could have been to add additional columns to old issues as that represents a checkin currently. I guess you had reasons not to go this route, but am a bit worried about the duplicated data.
I always think of `old_issues` as the `issues` table with information about a special status of the checkout: returned (which is inferred from a return date being set). With that in mind my only plan is to merge those tables in the mid (?) term.
The other option would be to have a checkins and a checkouts table long term.
But -regardless of what we do with those tables- this development highlights that the checkin action can (or not) trigger some status change for a checkout, and many more things. All of those nicely handled and tested here :-).
I was not saying this is not a solid development, I just wonder how it will fit with the existing structures.
Taking accountlines as an example, I will propose we ditch the old_issues link and replace with a checkin link. That way we actually gain tracking possibilities for (say) refunding lost fines even when there might not be a linked checkout, and things like that. The link to old checkouts could be reconstructed through the checkin link.
"ditching" will require some good thinking as we will need to retain the old link for old data or move the data into checkins first. And if we move that link, that means for data migrations we will need to fill one more table. Right now it's already a bit confusing as some reports use statistics, others use old_issues and now we will have checkins in the future as well. All recording the checkin. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #197722|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #197723|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 --- Comment #13 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 197863 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=197863&action=edit Bug 42418: Add checkins table and Koha::Checkin object Introduces checkins as a first-class resource with its own table, tracking the full context and outcomes of a return scan: - who: user_id (staff member), desk_id (optional) - what: item_id - where: library_id - when: timestamp - outcomes: checkout_id (returned), transfer_id (triggered), hold_id (filled) New files: - Koha::Checkin / Koha::Checkins (Koha::Object classes) - Koha::Schema::Result::Checkin (DBIC, generated via dbic) - atomicupdate/bug_42418.pl + kohastructure.sql Test plan: 1. Apply patch 2. Run updatedatabase.pl 3. Verify checkins table exists with correct structure 4. Sign off :-D Assisted-by: Sonnet 4.6 (Anthropic) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 --- Comment #14 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 197864 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=197864&action=edit Bug 42418: Wire Koha::Checkin into AddReturn Every call to AddReturn now creates a Koha::Checkin record capturing the full context and outcomes of the return scan: - user_id and desk_id from C4::Context->userenv - library_id from the branch parameter - checkout_id from the issue being returned (old_issues) - transfer_id from $messages->{WasTransfered} - hold_id from $messages->{ResFound}{reserve_id} The checkin record is also passed to the after_circ_action plugin hook, so plugins can access it directly instead of reconstructing context from side effects. AddReturn now returns a fifth value: the Koha::Checkin object. Existing callers are unaffected — Perl silently ignores extra return values. Assisted-by: Sonnet 4.6 (Anthropic) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 --- Comment #15 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 197865 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=197865&action=edit Bug 42418: (follow-up) Make checkins.user_id nullable SIP and CLI-driven returns have no userenv, so user_id must accept NULL. The FK action is also changed from CASCADE to SET NULL so deleting a staff patron preserves the checkin history. Changes: - kohastructure.sql: user_id DEFAULT NULL, FK ON DELETE SET NULL - atomicupdate: same - DBIC schema regenerated - Test updated to expect SET NULL behavior -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |42425 --- Comment #16 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- (In reply to Katrin Fischer from comment #12)
(In reply to Tomás Cohen Arazi (tcohen) from comment #11)
(In reply to Katrin Fischer from comment #10)
Can you explain a bit how this will affect old_issues or if there are plans for old_issues down the road?
Another solution could have been to add additional columns to old issues as that represents a checkin currently. I guess you had reasons not to go this route, but am a bit worried about the duplicated data.
I always think of `old_issues` as the `issues` table with information about a special status of the checkout: returned (which is inferred from a return date being set). With that in mind my only plan is to merge those tables in the mid (?) term.
The other option would be to have a checkins and a checkouts table long term.
A checkin action can trigger a 'return' which would be affecting an existing 'checkout' (issues table). But it can also not trigger a 'return' but a transfer and some fees. That data is not currently tracked properly.
But -regardless of what we do with those tables- this development highlights that the checkin action can (or not) trigger some status change for a checkout, and many more things. All of those nicely handled and tested here :-).
I was not saying this is not a solid development, I just wonder how it will fit with the existing structures.
This will be storing more relational data. Shouldn't change what we do today. We are filling a data gap.
Taking accountlines as an example, I will propose we ditch the old_issues link and replace with a checkin link. That way we actually gain tracking possibilities for (say) refunding lost fines even when there might not be a linked checkout, and things like that. The link to old checkouts could be reconstructed through the checkin link.
"ditching" will require some good thinking as we will need to retain the old link for old data or move the data into checkins first. And if we move that link, that means for data migrations we will need to fill one more table. Right now it's already a bit confusing as some reports use statistics, others use old_issues and now we will have checkins in the future as well. All recording the checkin.
Yeah, data migration would be challenging. The safest thing to do is creating a 'checkin' entry for each `old_isses row. They wouldn't have the rest of the info we'll be adding in the future, but at least things will be sound. Migrating old circulation info should be simple. This is adding a new table, but is basically trivial to fill it (just a link to the 'old_issues'. Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42425 [Bug 42425] Add checkin_id FK to accountlines to link fee adjustments to checkin events -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 --- Comment #17 from Kyle M Hall (khall) <kyle@bywatersolutions.com> --- This looks to be a solid concept to me! It will definitely be useful to know when an item was scanned in with or without a return being processed. The conversation has drifted a bit, so here are my two bits; If we created a new table for all of the $table/old_$table combos, we could we could recreate the existing current/old tables as views to avoid the need to rewrite reports. It looks like DBIC also supports views, but I haven't looked into it deeply. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #197863|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #197864|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #197865|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 --- Comment #18 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 198164 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198164&action=edit Bug 42418: DB changes Adds the checkins table to track individual checkin events with references to the item, staff member, library, desk, checkout, transfer, hold, recall, restriction, and return claim involved. Includes backfill of historical checkin records from old_issues. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 --- Comment #19 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 198165 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198165&action=edit Bug 42418: Add Koha::Schema::Result::Checkin DBIC Result class for the new checkins table. Includes relationships to items, borrowers, branches, desks, old_issues, branchtransfers, reserves, recalls, borrower_debarments, and return_claims. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 --- Comment #20 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 198166 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198166&action=edit Bug 42418: Add Koha::Checkin and Koha::Checkins classes Koha::Object-based classes for the checkins table, with tests. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 --- Comment #21 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 198167 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198167&action=edit Bug 42418: DBIC schema changes [DO NOT PUSH] Auto-generated has_many relationships added to related Result classes for the new checkins table. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 --- Comment #22 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 198168 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198168&action=edit Bug 42418: Wire Koha::Checkin into AddReturn Creates a Koha::Checkin record during AddReturn, capturing the checkin context (item, staff, library, desk, checkout, transfer, hold, recall, restriction, claim). The checkin_id is passed to the after_circ_action hook payload for plugin consumption. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #198164|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #198165|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #198166|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #198167|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #198168|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 --- Comment #23 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 198169 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198169&action=edit Bug 42418: DB changes Adds the checkins table to track individual checkin events with references to the item, staff member, library, desk, checkout, transfer, hold, recall, restriction, and return claim involved. Includes backfill of historical checkin records from old_issues. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 --- Comment #24 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 198170 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198170&action=edit Bug 42418: Add Koha::Schema::Result::Checkin DBIC Result class for the new checkins table. Includes relationships to items, borrowers, branches, desks, old_issues, branchtransfers, reserves, recalls, borrower_debarments, and return_claims. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 --- Comment #25 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 198171 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198171&action=edit Bug 42418: Add Koha::Checkin and Koha::Checkins classes Koha::Object-based classes for the checkins table, with tests. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 --- Comment #26 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 198172 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198172&action=edit Bug 42418: DBIC schema changes [DO NOT PUSH] Auto-generated has_many relationships added to related Result classes for the new checkins table. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 --- Comment #27 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 198173 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198173&action=edit Bug 42418: Wire Koha::Checkin into AddReturn Creates a Koha::Checkin record during AddReturn, capturing the checkin context (item, staff, library, desk, checkout, transfer, hold, recall, restriction, claim). The checkin_id is passed to the after_circ_action hook payload for plugin consumption. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 --- Comment #28 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- (In reply to Kyle M Hall (khall) from comment #17)
This looks to be a solid concept to me! It will definitely be useful to know when an item was scanned in with or without a return being processed.
The conversation has drifted a bit, so here are my two bits; If we created a new table for all of the $table/old_$table combos, we could we could recreate the existing current/old tables as views to avoid the need to rewrite reports. It looks like DBIC also supports views, but I haven't looked into it deeply.
Thanks Kyle, I think that's a good idea actually. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 Kyle M Hall (khall) <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA --- Comment #29 from Kyle M Hall (khall) <kyle@bywatersolutions.com> --- prove t/db_dependent/Circulation.t t/db_dependent/Circulation.t .. 30/87 # Transfer limit: Holding -> Return # Transfer limit: Return -> Home # Transfer limit: Return -> Holding # Looks like you planned 3 tests but ran 2. # Failed test 'AddReturn + TransferLimits' # at t/db_dependent/Circulation.t line 3217. Broken FK constraint # Failed test 'no warnings' # at /usr/share/perl/5.36/Test/Builder.pm line 193. # There were 1 warning(s) # Previous test 2 'Item cannot be returned if there is a transfer to do (item is floating)' # DBIx::Class::Storage::DBI::_dbh_execute(): DBI Exception: DBD::mysql::st execute failed: Cannot add or update a child row: a foreign key constraint fails (`koha_kohadev`.`checkins`, CONSTRAINT `checkins_ibfk_2` FOREIGN KEY (`user_id`) REFERENCES `borrowers` (`borrowernumber`) ON DELETE SET NULL ON UPDATE CASCADE) at /kohadevbox/koha/Koha/Object.pm line 174 # at /kohadevbox/koha/Koha/Object.pm line 178. # Koha::Object::catch {...} (DBIx::Class::Exception=HASH(0xaaaad545efd8)) called at /usr/share/perl5/Try/Tiny.pm line 123 # Try::Tiny::try(CODE(0xaaaad5327910), Try::Tiny::Catch=REF(0xaaaad57ae988)) called at /kohadevbox/koha/Koha/Object.pm line 183 # Koha::Object::store(Koha::Checkin=HASH(0xaaaacf983888)) called at /kohadevbox/koha/C4/Circulation.pm line 2842 # C4::Circulation::AddReturn("rDenWVmihu00aHTJS", "UihEz4gC") called at t/db_dependent/Circulation.t line 3215 # main::__ANON__() called at /usr/share/perl/5.36/Test/Builder.pm line 374 # eval {...} called at /usr/share/perl/5.36/Test/Builder.pm line 374 # Test::Builder::subtest(Test::Builder=HASH(0xaaaac96e2c20), "AddReturn + TransferLimits", CODE(0xaaaad5381bd8)) called at /usr/share/perl/5.36/Test/More.pm line 809 # Test::More::subtest("AddReturn + TransferLimits", CODE(0xaaaad5381bd8)) called at t/db_dependent/Circulation.t line 3217 # # Looks like your test exited with 11 just after 34. t/db_dependent/Circulation.t .. Dubious, test returned 11 (wstat 2816, 0xb00) Failed 55/87 subtests Test Summary Report ------------------- t/db_dependent/Circulation.t (Wstat: 2816 (exited 11) Tests: 34 Failed: 2) Failed tests: 33-34 Non-zero exit status: 11 Parse errors: Bad plan. You planned 87 tests but ran 34. Files=1, Tests=34, 10 wallclock secs ( 0.03 usr 0.01 sys + 7.42 cusr 1.34 csys = 8.80 CPU) Result: FAIL -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 --- Comment #30 from Kyle M Hall (khall) <kyle@bywatersolutions.com> --- INSERT INTO checkins (item_id, user_id, library_id, timestamp, checkout_id) SELECT oi.itemnumber, oi.issuer_id, COALESCE(oi.checkin_library, oi.branchcode), oi.returndate, oi.issue_id FROM old_issues oi WHERE oi.returndate IS NOT NULL AND oi.itemnumber IS NOT NULL Can fail because check_library and branchcode are both allowed to be null. May add an "AND ( oi.checkin_library IS NOT NULL OR oi.branchcode IS NOT NULL )"? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 --- Comment #31 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- (In reply to Kyle M Hall (khall) from comment #29)
# DBIx::Class::Storage::DBI::_dbh_execute(): DBI Exception: DBD::mysql::st execute failed: Cannot add or update a child row: a foreign key constraint fails (`koha_kohadev`.`checkins`, CONSTRAINT `checkins_ibfk_2` FOREIGN KEY (`user_id`) REFERENCES `borrowers` (`borrowernumber`) ON DELETE SET NULL ON UPDATE CASCADE) at /kohadevbox/koha/Koha/Object.pm line 174
That's because the mocked userenv (I reused) is pointing to a non-existent patron. I'll provide a fix, but the fallback to `borrowernumber=51` (the default) is a ticking bomb I'd say. Gonna file a separate (follow-up) bug report to make `mock_userenv` create a patron if not passed. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #198169|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #198170|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #198171|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #198172|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #198173|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 --- Comment #32 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 198555 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198555&action=edit Bug 42418: DB changes Adds the checkins table to track individual checkin events with references to the item, staff member, library, desk, checkout, transfer, hold, recall, restriction, and return claim involved. Includes backfill of historical checkin records from old_issues. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 --- Comment #33 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 198556 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198556&action=edit Bug 42418: Add Koha::Schema::Result::Checkin DBIC Result class for the new checkins table. Includes relationships to items, borrowers, branches, desks, old_issues, branchtransfers, reserves, recalls, borrower_debarments, and return_claims. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 --- Comment #34 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 198557 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198557&action=edit Bug 42418: Add Koha::Checkin and Koha::Checkins classes Koha::Object-based classes for the checkins table, with tests. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 --- Comment #35 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 198558 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198558&action=edit Bug 42418: DBIC schema changes [DO NOT PUSH] Auto-generated has_many relationships added to related Result classes for the new checkins table. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 --- Comment #36 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 198559 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198559&action=edit Bug 42418: Wire Koha::Checkin into AddReturn Creates a Koha::Checkin record during AddReturn, capturing the checkin context (item, staff, library, desk, checkout, transfer, hold, recall, restriction, claim). The checkin_id is passed to the after_circ_action hook payload for plugin consumption. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 --- Comment #37 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 198560 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198560&action=edit Bug 42418: (follow-up) Fix tests using invalid mock_userenv The checkins table has a FK on user_id pointing to borrowers. Several mock_userenv calls only passed branchcode, which defaults borrowernumber to the hardcoded value 51 — a non-existent patron. This causes FK violations when AddReturn creates a checkin record. This patch fixes the affected calls in Circulation.t and Branch.t by passing a valid patron to mock_userenv. It also adjusts the stock rotation transfer test to account for the new 5th return value from AddReturn (the Koha::Checkin object), and adds tests for the three checkin record user_id scenarios (valid patron, missing number, explicit undef number). A global fix to mock_userenv (auto-generating a patron when none is passed) would address the ~120 remaining calls across the test suite, but the risk surface warrants a separate bug. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 --- Comment #38 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 198561 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198561&action=edit Bug 42418: (follow-up) Skip old_issues with no library on backfill Both checkin_library and branchcode are nullable in old_issues. The COALESCE can return NULL, which violates the NOT NULL constraint on checkins.library_id. Filter out those rows. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |andrew@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@gmail.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks|24401 | Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24401 [Bug 24401] Add POST /checkins endpoint with availability check and confirmation flow -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 --- Comment #39 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 198632 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198632&action=edit Bug 42418: (follow-up) Fix Patrons.t for checkin record creation AddReturn now creates a checkin record which requires a valid library_id. The anonymize_last_borrowers subtests call AddReturn without a userenv set, causing a NOT NULL constraint violation. This adds mock_userenv before the AddReturn calls. Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove t/db_dependent/Koha/Patrons.t => SUCCESS: All 52 tests pass! 3. Sign off :-D -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 David Nind <david@davidnind.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=42418 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #198555|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #198556|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #198557|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #198558|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #198559|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #198560|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #198561|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #198632|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 --- Comment #40 from David Nind <david@davidnind.com> --- Created attachment 198677 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198677&action=edit Bug 42418: DB changes Adds the checkins table to track individual checkin events with references to the item, staff member, library, desk, checkout, transfer, hold, recall, restriction, and return claim involved. Includes backfill of historical checkin records from old_issues. Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 --- Comment #41 from David Nind <david@davidnind.com> --- Created attachment 198678 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198678&action=edit Bug 42418: Add Koha::Schema::Result::Checkin DBIC Result class for the new checkins table. Includes relationships to items, borrowers, branches, desks, old_issues, branchtransfers, reserves, recalls, borrower_debarments, and return_claims. Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 --- Comment #42 from David Nind <david@davidnind.com> --- Created attachment 198679 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198679&action=edit Bug 42418: Add Koha::Checkin and Koha::Checkins classes Koha::Object-based classes for the checkins table, with tests. Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 --- Comment #43 from David Nind <david@davidnind.com> --- Created attachment 198680 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198680&action=edit Bug 42418: DBIC schema changes [DO NOT PUSH] Auto-generated has_many relationships added to related Result classes for the new checkins table. Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 --- Comment #44 from David Nind <david@davidnind.com> --- Created attachment 198681 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198681&action=edit Bug 42418: Wire Koha::Checkin into AddReturn Creates a Koha::Checkin record during AddReturn, capturing the checkin context (item, staff, library, desk, checkout, transfer, hold, recall, restriction, claim). The checkin_id is passed to the after_circ_action hook payload for plugin consumption. Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 --- Comment #45 from David Nind <david@davidnind.com> --- Created attachment 198682 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198682&action=edit Bug 42418: (follow-up) Fix tests using invalid mock_userenv The checkins table has a FK on user_id pointing to borrowers. Several mock_userenv calls only passed branchcode, which defaults borrowernumber to the hardcoded value 51 — a non-existent patron. This causes FK violations when AddReturn creates a checkin record. This patch fixes the affected calls in Circulation.t and Branch.t by passing a valid patron to mock_userenv. It also adjusts the stock rotation transfer test to account for the new 5th return value from AddReturn (the Koha::Checkin object), and adds tests for the three checkin record user_id scenarios (valid patron, missing number, explicit undef number). A global fix to mock_userenv (auto-generating a patron when none is passed) would address the ~120 remaining calls across the test suite, but the risk surface warrants a separate bug. Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 --- Comment #46 from David Nind <david@davidnind.com> --- Created attachment 198683 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198683&action=edit Bug 42418: (follow-up) Skip old_issues with no library on backfill Both checkin_library and branchcode are nullable in old_issues. The COALESCE can return NULL, which violates the NOT NULL constraint on checkins.library_id. Filter out those rows. Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 --- Comment #47 from David Nind <david@davidnind.com> --- Created attachment 198684 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198684&action=edit Bug 42418: (follow-up) Fix Patrons.t for checkin record creation AddReturn now creates a checkin record which requires a valid library_id. The anonymize_last_borrowers subtests call AddReturn without a userenv set, causing a NOT NULL constraint violation. This adds mock_userenv before the AddReturn calls. Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove t/db_dependent/Koha/Patrons.t => SUCCESS: All 52 tests pass! 3. Sign off :-D Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 --- Comment #48 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 201487 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201487&action=edit Bug 42418: (follow-up) Always create Koha::Checkin and attach messages This patch ensures every barcode scan at the checkin desk creates a Koha::Checkin record, even when the return is blocked. Previously, the BlockedWithdrawn and Wrongbranch early-return paths skipped creating the record, losing the audit trail. Changes: - Create Koha::Checkin record once, before blocker checks - BlockedWithdrawn and Wrongbranch early returns now include the record - Add _attach_messages_to_checkin() to translate AddReturn's legacy $messages hashref into Koha::Object::Message instances on the checkin object, using lower_snake_case coded values - Call _attach_messages_to_checkin at all return points This is backwards compatible - existing callers still receive the $messages hashref. The object_messages are additive and ignored by callers that don't read them. Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove t/db_dependent/api/v1/checkins.t => SUCCESS: Tests pass 3. Sign off :-D -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #198677|0 |1 is obsolete| | Attachment #198678|0 |1 is obsolete| | Attachment #198679|0 |1 is obsolete| | Attachment #198680|0 |1 is obsolete| | Attachment #198681|0 |1 is obsolete| | Attachment #198682|0 |1 is obsolete| | Attachment #198683|0 |1 is obsolete| | Attachment #198684|0 |1 is obsolete| | Attachment #201487|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 --- Comment #49 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 201930 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201930&action=edit Bug 42418: DB changes Adds the checkins table to track individual checkin events with references to the item, staff member, library, desk, checkout, transfer, hold, recall, restriction, and return claim involved. Includes backfill of historical checkin records from old_issues. Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 --- Comment #50 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 201931 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201931&action=edit Bug 42418: Add Koha::Schema::Result::Checkin DBIC Result class for the new checkins table. Includes relationships to items, borrowers, branches, desks, old_issues, branchtransfers, reserves, recalls, borrower_debarments, and return_claims. Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 --- Comment #51 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 201932 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201932&action=edit Bug 42418: Add Koha::Checkin and Koha::Checkins classes Koha::Object-based classes for the checkins table, with tests. Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 --- Comment #52 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 201933 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201933&action=edit Bug 42418: DBIC schema changes [DO NOT PUSH] Auto-generated has_many relationships added to related Result classes for the new checkins table. Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 --- Comment #53 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 201934 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201934&action=edit Bug 42418: Wire Koha::Checkin into AddReturn Creates a Koha::Checkin record during AddReturn, capturing the checkin context (item, staff, library, desk, checkout, transfer, hold, recall, restriction, claim). The checkin_id is passed to the after_circ_action hook payload for plugin consumption. Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 --- Comment #54 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 201935 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201935&action=edit Bug 42418: (follow-up) Fix tests using invalid mock_userenv The checkins table has a FK on user_id pointing to borrowers. Several mock_userenv calls only passed branchcode, which defaults borrowernumber to the hardcoded value 51 — a non-existent patron. This causes FK violations when AddReturn creates a checkin record. This patch fixes the affected calls in Circulation.t and Branch.t by passing a valid patron to mock_userenv. It also adjusts the stock rotation transfer test to account for the new 5th return value from AddReturn (the Koha::Checkin object), and adds tests for the three checkin record user_id scenarios (valid patron, missing number, explicit undef number). A global fix to mock_userenv (auto-generating a patron when none is passed) would address the ~120 remaining calls across the test suite, but the risk surface warrants a separate bug. Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 --- Comment #55 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 201936 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201936&action=edit Bug 42418: (follow-up) Skip old_issues with no library on backfill Both checkin_library and branchcode are nullable in old_issues. The COALESCE can return NULL, which violates the NOT NULL constraint on checkins.library_id. Filter out those rows. Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 --- Comment #56 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 201937 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201937&action=edit Bug 42418: (follow-up) Fix Patrons.t for checkin record creation AddReturn now creates a checkin record which requires a valid library_id. The anonymize_last_borrowers subtests call AddReturn without a userenv set, causing a NOT NULL constraint violation. This adds mock_userenv before the AddReturn calls. Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove t/db_dependent/Koha/Patrons.t => SUCCESS: All 52 tests pass! 3. Sign off :-D Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #201930|0 |1 is obsolete| | Attachment #201931|0 |1 is obsolete| | Attachment #201932|0 |1 is obsolete| | Attachment #201933|0 |1 is obsolete| | Attachment #201934|0 |1 is obsolete| | Attachment #201935|0 |1 is obsolete| | Attachment #201936|0 |1 is obsolete| | Attachment #201937|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 --- Comment #57 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 201943 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201943&action=edit Bug 42418: DB changes Adds the checkins table to track individual checkin events with references to the item, staff member, library, desk, checkout, transfer, hold, recall, restriction, and return claim involved. Includes backfill of historical checkin records from old_issues. Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 --- Comment #58 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 201944 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201944&action=edit Bug 42418: Add Koha::Schema::Result::Checkin DBIC Result class for the new checkins table. Includes relationships to items, borrowers, branches, desks, old_issues, branchtransfers, reserves, recalls, borrower_debarments, and return_claims. Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 --- Comment #59 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 201945 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201945&action=edit Bug 42418: Add Koha::Checkin and Koha::Checkins classes Koha::Object-based classes for the checkins table, with tests. Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 --- Comment #60 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 201946 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201946&action=edit Bug 42418: DBIC schema changes [DO NOT PUSH] Auto-generated has_many relationships added to related Result classes for the new checkins table. Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 --- Comment #61 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 201947 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201947&action=edit Bug 42418: Wire Koha::Checkin into AddReturn Creates a Koha::Checkin record during AddReturn, capturing the checkin context (item, staff, library, desk, checkout, transfer, hold, recall, restriction, claim). The checkin_id is passed to the after_circ_action hook payload for plugin consumption. Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 --- Comment #62 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 201948 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201948&action=edit Bug 42418: (follow-up) Fix tests using invalid mock_userenv The checkins table has a FK on user_id pointing to borrowers. Several mock_userenv calls only passed branchcode, which defaults borrowernumber to the hardcoded value 51 — a non-existent patron. This causes FK violations when AddReturn creates a checkin record. This patch fixes the affected calls in Circulation.t and Branch.t by passing a valid patron to mock_userenv. It also adjusts the stock rotation transfer test to account for the new 5th return value from AddReturn (the Koha::Checkin object), and adds tests for the three checkin record user_id scenarios (valid patron, missing number, explicit undef number). A global fix to mock_userenv (auto-generating a patron when none is passed) would address the ~120 remaining calls across the test suite, but the risk surface warrants a separate bug. Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 --- Comment #63 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 201949 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201949&action=edit Bug 42418: (follow-up) Skip old_issues with no library on backfill Both checkin_library and branchcode are nullable in old_issues. The COALESCE can return NULL, which violates the NOT NULL constraint on checkins.library_id. Filter out those rows. Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 --- Comment #64 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 201950 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201950&action=edit Bug 42418: (follow-up) Fix Patrons.t for checkin record creation AddReturn now creates a checkin record which requires a valid library_id. The anonymize_last_borrowers subtests call AddReturn without a userenv set, causing a NOT NULL constraint violation. This adds mock_userenv before the AddReturn calls. Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove t/db_dependent/Koha/Patrons.t => SUCCESS: All 52 tests pass! 3. Sign off :-D Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #201943|0 |1 is obsolete| | Attachment #201944|0 |1 is obsolete| | Attachment #201945|0 |1 is obsolete| | Attachment #201946|0 |1 is obsolete| | Attachment #201947|0 |1 is obsolete| | Attachment #201948|0 |1 is obsolete| | Attachment #201949|0 |1 is obsolete| | Attachment #201950|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 --- Comment #65 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 204153 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=204153&action=edit Bug 42418: DB changes Adds the checkins table to track individual checkin events with references to the item, staff member, library, desk, checkout, transfer, hold, recall, restriction, and return claim involved. Includes backfill of historical checkin records from old_issues. Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Tomás 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=42418 --- Comment #66 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 204154 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=204154&action=edit Bug 42418: Add Koha::Schema::Result::Checkin DBIC Result class for the new checkins table. Includes relationships to items, borrowers, branches, desks, old_issues, branchtransfers, reserves, recalls, borrower_debarments, and return_claims. Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Tomás 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=42418 --- Comment #67 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 204155 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=204155&action=edit Bug 42418: Add Koha::Checkin and Koha::Checkins classes Koha::Object-based classes for the checkins table, with tests. Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Tomás 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=42418 --- Comment #68 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 204156 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=204156&action=edit Bug 42418: DBIC schema changes [DO NOT PUSH] Auto-generated has_many relationships added to related Result classes for the new checkins table. Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Tomás 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=42418 --- Comment #69 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 204157 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=204157&action=edit Bug 42418: Wire Koha::Checkin into AddReturn Creates a Koha::Checkin record during AddReturn, capturing the checkin context (item, staff, library, desk, checkout, transfer, hold, recall, restriction, claim). The checkin_id is passed to the after_circ_action hook payload for plugin consumption. Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Tomás 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=42418 --- Comment #70 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 204158 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=204158&action=edit Bug 42418: (follow-up) Fix tests using invalid mock_userenv The checkins table has a FK on user_id pointing to borrowers. Several mock_userenv calls only passed branchcode, which defaults borrowernumber to the hardcoded value 51 — a non-existent patron. This causes FK violations when AddReturn creates a checkin record. This patch fixes the affected calls in Circulation.t and Branch.t by passing a valid patron to mock_userenv. It also adjusts the stock rotation transfer test to account for the new 5th return value from AddReturn (the Koha::Checkin object), and adds tests for the three checkin record user_id scenarios (valid patron, missing number, explicit undef number). A global fix to mock_userenv (auto-generating a patron when none is passed) would address the ~120 remaining calls across the test suite, but the risk surface warrants a separate bug. Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Tomás 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=42418 --- Comment #71 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 204159 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=204159&action=edit Bug 42418: (follow-up) Skip old_issues with no library on backfill Both checkin_library and branchcode are nullable in old_issues. The COALESCE can return NULL, which violates the NOT NULL constraint on checkins.library_id. Filter out those rows. Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Tomás 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=42418 --- Comment #72 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 204160 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=204160&action=edit Bug 42418: (follow-up) Fix Patrons.t for checkin record creation AddReturn now creates a checkin record which requires a valid library_id. The anonymize_last_borrowers subtests call AddReturn without a userenv set, causing a NOT NULL constraint violation. This adds mock_userenv before the AddReturn calls. Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove t/db_dependent/Koha/Patrons.t => SUCCESS: All 52 tests pass! 3. Sign off :-D Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Tomás 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=42418 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA QA Contact|testopia@bugs.koha-communit |martin.renvoize@openfifth.c |y.org |o.uk -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #204153|0 |1 is obsolete| | Attachment #204154|0 |1 is obsolete| | Attachment #204155|0 |1 is obsolete| | Attachment #204156|0 |1 is obsolete| | Attachment #204157|0 |1 is obsolete| | Attachment #204158|0 |1 is obsolete| | Attachment #204159|0 |1 is obsolete| | Attachment #204160|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 --- Comment #73 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 204904 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=204904&action=edit Bug 42418: DB changes Adds the checkins table to track individual checkin events with references to the item, staff member, library, desk, checkout, transfer, hold, recall, restriction, and return claim involved. Includes backfill of historical checkin records from old_issues. Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 --- Comment #74 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 204905 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=204905&action=edit Bug 42418: Add Koha::Schema::Result::Checkin DBIC Result class for the new checkins table. Includes relationships to items, borrowers, branches, desks, old_issues, branchtransfers, reserves, recalls, borrower_debarments, and return_claims. Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 --- Comment #75 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 204906 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=204906&action=edit Bug 42418: Add Koha::Checkin and Koha::Checkins classes Koha::Object-based classes for the checkins table, with tests. Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 --- Comment #76 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 204907 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=204907&action=edit Bug 42418: DBIC schema changes [DO NOT PUSH] Auto-generated has_many relationships added to related Result classes for the new checkins table. Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 --- Comment #77 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 204908 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=204908&action=edit Bug 42418: Wire Koha::Checkin into AddReturn Creates a Koha::Checkin record during AddReturn, capturing the checkin context (item, staff, library, desk, checkout, transfer, hold, recall, restriction, claim). The checkin_id is passed to the after_circ_action hook payload for plugin consumption. Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 --- Comment #78 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 204909 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=204909&action=edit Bug 42418: (follow-up) Fix tests using invalid mock_userenv The checkins table has a FK on user_id pointing to borrowers. Several mock_userenv calls only passed branchcode, which defaults borrowernumber to the hardcoded value 51 — a non-existent patron. This causes FK violations when AddReturn creates a checkin record. This patch fixes the affected calls in Circulation.t and Branch.t by passing a valid patron to mock_userenv. It also adjusts the stock rotation transfer test to account for the new 5th return value from AddReturn (the Koha::Checkin object), and adds tests for the three checkin record user_id scenarios (valid patron, missing number, explicit undef number). A global fix to mock_userenv (auto-generating a patron when none is passed) would address the ~120 remaining calls across the test suite, but the risk surface warrants a separate bug. Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 --- Comment #79 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 204910 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=204910&action=edit Bug 42418: (follow-up) Skip old_issues with no library on backfill Both checkin_library and branchcode are nullable in old_issues. The COALESCE can return NULL, which violates the NOT NULL constraint on checkins.library_id. Filter out those rows. Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 --- Comment #80 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 204911 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=204911&action=edit Bug 42418: (follow-up) Fix Patrons.t for checkin record creation AddReturn now creates a checkin record which requires a valid library_id. The anonymize_last_borrowers subtests call AddReturn without a userenv set, causing a NOT NULL constraint violation. This adds mock_userenv before the AddReturn calls. Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove t/db_dependent/Koha/Patrons.t => SUCCESS: All 52 tests pass! 3. Sign off :-D Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 --- Comment #81 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 204912 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=204912&action=edit Bug 42418: (QA follow-up) Keep AddReturn message-content assertions in sync with the new return arity Wiring Koha::Checkin into AddReturn changed its return list from 4 to 5 elements. Three is_deeply assertions on the full return list broke as a result and were weakened down to bare is($doreturn, 0, ...)/ single-key checks instead of being updated for the new arity - losing their ability to catch corruption of $messages content. Update them to slice to the first four elements instead (matching the pattern already used elsewhere in this file for the same reason), and add a Koha::Checkin isa_ok check on the 5th element where a checkin record is expected. Test plan: 1. prove t/db_dependent/Circulation.t => SUCCESS: 90 tests pass. Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 --- Comment #82 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 204913 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=204913&action=edit Bug 42418: (QA follow-up) Fix DataCorrupted return arity and checkin timestamp population Two fixes to how AddReturn manages the Koha::Checkin audit record it creates for every scan: - The checkin record's DB-computed defaults (e.g. timestamp, which defaults to current_timestamp()) were never refreshed from storage after the initial INSERT, so the in-memory object kept holding the literal SQL placeholder DBIx::Class leaves behind instead of the real value. Add ->discard_changes after ->store to fix this. - AddReturn's DataCorrupted error path (fired when MarkIssueReturned dies) still returned the pre-Bug-42418 4-tuple instead of including $checkin_record, and never attached its own messages to it. This crashed the REST controller's to_api($checkin) call on undef, masking the real error behind a generic 500 and leaving the audit row without any explanation of what happened. Add a 'data_corrupted' message case to _attach_messages_to_checkin for this. Test plan: 1. prove t/db_dependent/Circulation.t t/db_dependent/Koha/Checkin.t => SUCCESS: all pass. Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 --- Comment #83 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 204914 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=204914&action=edit Bug 42418: (QA follow-up) Add full coverage for _attach_messages_to_checkin _attach_messages_to_checkin (moved back to C4::Circulation in the previous follow-up) had no coverage at all, direct or indirect - existing tests only ever assert on AddReturn's legacy $messages hashref, never on the Koha::Checkin object_messages this function actually produces. It's a large function (~185 lines, 27 recognised message keys), so cover it directly rather than trying to drive all 27 through real AddReturn scenarios, following the existing precedent in this file of calling C4::Circulation private subs directly in tests (_GetCircControlBranch, _FixOverduesOnReturn). Test plan: 1. prove t/db_dependent/Circulation.t => SUCCESS: 92 tests pass, all 27 message keys exercised. Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #204904|0 |1 is obsolete| | Attachment #204905|0 |1 is obsolete| | Attachment #204906|0 |1 is obsolete| | Attachment #204907|0 |1 is obsolete| | Attachment #204908|0 |1 is obsolete| | Attachment #204909|0 |1 is obsolete| | Attachment #204910|0 |1 is obsolete| | Attachment #204911|0 |1 is obsolete| | Attachment #204912|0 |1 is obsolete| | Attachment #204913|0 |1 is obsolete| | Attachment #204914|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 --- Comment #84 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 205060 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=205060&action=edit Bug 42418: DB changes Adds the checkins table to track individual checkin events with references to the item, staff member, library, desk, checkout, transfer, hold, recall, restriction, and return claim involved. Includes backfill of historical checkin records from old_issues. Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 --- Comment #85 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 205061 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=205061&action=edit Bug 42418: Add Koha::Schema::Result::Checkin DBIC Result class for the new checkins table. Includes relationships to items, borrowers, branches, desks, old_issues, branchtransfers, reserves, recalls, borrower_debarments, and return_claims. Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 --- Comment #86 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 205062 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=205062&action=edit Bug 42418: Add Koha::Checkin and Koha::Checkins classes Koha::Object-based classes for the checkins table, with tests. Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 --- Comment #87 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 205063 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=205063&action=edit Bug 42418: DBIC schema changes [DO NOT PUSH] Auto-generated has_many relationships added to related Result classes for the new checkins table. Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 --- Comment #88 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 205064 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=205064&action=edit Bug 42418: Wire Koha::Checkin into AddReturn Creates a Koha::Checkin record during AddReturn, capturing the checkin context (item, staff, library, desk, checkout, transfer, hold, recall, restriction, claim). The checkin_id is passed to the after_circ_action hook payload for plugin consumption. Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 --- Comment #89 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 205065 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=205065&action=edit Bug 42418: (follow-up) Fix tests using invalid mock_userenv The checkins table has a FK on user_id pointing to borrowers. Several mock_userenv calls only passed branchcode, which defaults borrowernumber to the hardcoded value 51 — a non-existent patron. This causes FK violations when AddReturn creates a checkin record. This patch fixes the affected calls in Circulation.t and Branch.t by passing a valid patron to mock_userenv. It also adjusts the stock rotation transfer test to account for the new 5th return value from AddReturn (the Koha::Checkin object), and adds tests for the three checkin record user_id scenarios (valid patron, missing number, explicit undef number). A global fix to mock_userenv (auto-generating a patron when none is passed) would address the ~120 remaining calls across the test suite, but the risk surface warrants a separate bug. Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 --- Comment #90 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 205066 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=205066&action=edit Bug 42418: (follow-up) Skip old_issues with no library on backfill Both checkin_library and branchcode are nullable in old_issues. The COALESCE can return NULL, which violates the NOT NULL constraint on checkins.library_id. Filter out those rows. Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 --- Comment #91 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 205067 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=205067&action=edit Bug 42418: (follow-up) Fix Patrons.t for checkin record creation AddReturn now creates a checkin record which requires a valid library_id. The anonymize_last_borrowers subtests call AddReturn without a userenv set, causing a NOT NULL constraint violation. This adds mock_userenv before the AddReturn calls. Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove t/db_dependent/Koha/Patrons.t => SUCCESS: All 52 tests pass! 3. Sign off :-D Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42418 --- Comment #92 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 205068 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=205068&action=edit Bug 42418: (QA follow-up) Keep AddReturn message-content assertions in sync with the new return arity Wiring Koha::Checkin into AddReturn changed its return list from 4 to 5 elements. Three is_deeply assertions on the full return list broke as a result and were weakened down to bare is($doreturn, 0, ...)/ single-key checks instead of being updated for the new arity - losing their ability to catch corruption of $messages content. Update them to slice to the first four elements instead (matching the pattern already used elsewhere in this file for the same reason), and add a Koha::Checkin isa_ok check on the 5th element where a checkin record is expected. Test plan: 1. prove t/db_dependent/Circulation.t => SUCCESS: 90 tests pass. Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> Signed-off-by: Tomás 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=42418 --- Comment #93 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 205069 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=205069&action=edit Bug 42418: (QA follow-up) Fix DataCorrupted return arity and checkin timestamp population Two fixes to how AddReturn manages the Koha::Checkin audit record it creates for every scan: - The checkin record's DB-computed defaults (e.g. timestamp, which defaults to current_timestamp()) were never refreshed from storage after the initial INSERT, so the in-memory object kept holding the literal SQL placeholder DBIx::Class leaves behind instead of the real value. Add ->discard_changes after ->store to fix this. - AddReturn's DataCorrupted error path (fired when MarkIssueReturned dies) still returned the pre-Bug-42418 4-tuple instead of including $checkin_record, and never attached its own messages to it. This crashed the REST controller's to_api($checkin) call on undef, masking the real error behind a generic 500 and leaving the audit row without any explanation of what happened. Add a 'data_corrupted' message case to _attach_messages_to_checkin for this. Test plan: 1. prove t/db_dependent/Circulation.t t/db_dependent/Koha/Checkin.t => SUCCESS: all pass. Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> Signed-off-by: Tomás 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=42418 --- Comment #94 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 205070 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=205070&action=edit Bug 42418: (QA follow-up) Add full coverage for _attach_messages_to_checkin _attach_messages_to_checkin (moved back to C4::Circulation in the previous follow-up) had no coverage at all, direct or indirect - existing tests only ever assert on AddReturn's legacy $messages hashref, never on the Koha::Checkin object_messages this function actually produces. It's a large function (~185 lines, 27 recognised message keys), so cover it directly rather than trying to drive all 27 through real AddReturn scenarios, following the existing precedent in this file of calling C4::Circulation private subs directly in tests (_GetCircControlBranch, _FixOverduesOnReturn). Test plan: 1. prove t/db_dependent/Circulation.t => SUCCESS: 92 tests pass, all 27 message keys exercised. Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> Signed-off-by: Tomás 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=42418 Bug 42418 depends on bug 41728, which changed state. Bug 41728 Summary: Add `Koha::Item::Availability::Checkin` to centralize logic https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41728 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