[Bug 21401] New: Account offsets should save the transacting library
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21401 Bug ID: 21401 Summary: Account offsets should save the transacting library Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: koha-bugs@lists.koha-community.org Reporter: josef.moravec@gmail.com QA Contact: testopia@bugs.koha-community.org -- 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=21401 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |black23@gmail.com, | |rbit@rbit.cz Assignee|koha-bugs@lists.koha-commun |josef.moravec@gmail.com |ity.org | -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21401 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21401 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21401 --- Comment #1 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 79272 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=79272&action=edit Bug 21401: Add column transaction_library to table account_offsets -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21401 --- Comment #2 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 79273 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=79273&action=edit Bug 21401: Update schema -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21401 --- Comment #3 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 79274 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=79274&action=edit Bug 21401: Add tests for transaction_library -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21401 --- Comment #4 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 79275 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=79275&action=edit Bug 21401: Take transaction library from userenv when not defined Test plan: 1) Apply the patch 2) Run database update 3) prove t/db_dependent/Account.t -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21401 Michal Denar <black23@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #79275|0 |1 is obsolete| | --- Comment #5 from Michal Denar <black23@gmail.com> --- Created attachment 79284 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=79284&action=edit Bug 21401: Take transaction library from userenv when not defined Test plan: 1) Apply the patch 2) Run database update 3) prove t/db_dependent/Account.t Signed-off-by: Michal Denar <black23@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21401 Michal Denar <black23@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21401 Michal Denar <black23@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #79272|0 |1 is obsolete| | Attachment #79273|0 |1 is obsolete| | Attachment #79274|0 |1 is obsolete| | Attachment #79284|0 |1 is obsolete| | --- Comment #6 from Michal Denar <black23@gmail.com> --- Created attachment 79285 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=79285&action=edit Bug 21401: Add column transaction_library to table account_offsets Signed-off-by: Michal Denare <black23@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21401 --- Comment #7 from Michal Denar <black23@gmail.com> --- Created attachment 79286 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=79286&action=edit Bug 21401: Update schema Signed-off-by: Your Full Name <your_email> Signed-off-by: Your Full Name <your_email> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21401 --- Comment #8 from Michal Denar <black23@gmail.com> --- Created attachment 79287 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=79287&action=edit Bug 21401: Add tests for transaction_library Signed-off-by: Your Full Name <your_email> Signed-off-by: Your Full Name <your_email> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21401 --- Comment #9 from Michal Denar <black23@gmail.com> --- Created attachment 79288 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=79288&action=edit Bug 21401: Take transaction library from userenv when not defined Test plan: 1) Apply the patch 2) Run database update 3) prove t/db_dependent/Account.t Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Your Full Name <your_email> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21401 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@bugs.koha-c | |ommunity.org --- Comment #10 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- - Use cases? - Maybe we should use ON DELETE SET NULL instead of cascade? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21401 --- Comment #11 from Josef Moravec <josef.moravec@gmail.com> --- (In reply to Jonathan Druart from comment #10)
- Use cases?
Statistics - we need reports for payment, classified by branch and type of fee, account_offsets table do the needed linking between fine and its payment, we just need to add a branch information
- Maybe we should use ON DELETE SET NULL instead of cascade?
Good point, I'll fix it. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21401 --- Comment #12 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 79688 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=79688&action=edit Bug 21401: (follow-up) on update set null instead of cascade on transaction library -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21401 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |martin.renvoize@ptfs-europe | |.com QA Contact|testopia@bugs.koha-communit |martin.renvoize@ptfs-europe |y.org |.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21401 --- Comment #13 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- A few points. 1) This information is actually already stored in the statistics table (though I'll happily admit that's a pain and not especially useful for querying against) 2) It feels confusing to have this at the account_offsets level (which often has both debit_id and credit_id), it feels like it would be better placed in accountlines to me. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21401 --- Comment #14 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- also, as much as I personally don't like it.. you'd be best to conform the the guidelines for your new field name: https://wiki.koha-community.org/wiki/Koha_Objects#Database_Table_Conventions - Every foreign key should have the same name as the primary key it links to when possible ( exceptions would be multiple FKs to the same table, and where a descriptor is better e.g. widgets.borrowernumber = widgets.managing_borrowernumber ) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21401 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |ASSIGNED --- Comment #15 from Josef Moravec <josef.moravec@gmail.com> --- (In reply to Martin Renvoize from comment #13)
A few points.
1) This information is actually already stored in the statistics table (though I'll happily admit that's a pain and not especially useful for querying against)
That's the reason I wanted to add the branch to accounts
2) It feels confusing to have this at the account_offsets level (which often has both debit_id and credit_id), it feels like it would be better placed in accountlines to me.
You are right, I'll change this to accountlines, it is not clear if the library is for credit or debit and it could be different library... -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21401 --- Comment #16 from Josef Moravec <josef.moravec@gmail.com> --- (In reply to Martin Renvoize from comment #14)
also, as much as I personally don't like it.. you'd be best to conform the the guidelines for your new field name:
https://wiki.koha-community.org/wiki/Koha_Objects#Database_Table_Conventions - Every foreign key should have the same name as the primary key it links to when possible ( exceptions would be multiple FKs to the same table, and where a descriptor is better e.g. widgets.borrowernumber = widgets.managing_borrowernumber )
True, I'll do it this way. Thanks for comments! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21401 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21401 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #79285|0 |1 is obsolete| | Attachment #79286|0 |1 is obsolete| | Attachment #79287|0 |1 is obsolete| | Attachment #79288|0 |1 is obsolete| | Attachment #79688|0 |1 is obsolete| | --- Comment #17 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 80355 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=80355&action=edit Bug 21401: Add column transaction_library to table account_offsets -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21401 --- Comment #18 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 80356 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=80356&action=edit Bug 21401: DO NOT PUSH - schema changes -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21401 --- Comment #19 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 80357 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=80357&action=edit Bug 21401: Add tests for transaction_library -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21401 --- Comment #20 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 80358 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=80358&action=edit Bug 21401: Take transaction library from userenv when not defined Test plan: 1) Apply the patch 2) Run database update 3) prove t/db_dependent/Account.t -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21401 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |21538 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21538 [Bug 21538] Show ilbrary on account page -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21401 --- Comment #21 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Sorry Josef, I think this also needs some minor updates to Koha::Account.. in particular near the bottom of the pay method, but I've not delved further yet. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21401 --- Comment #22 from Josef Moravec <josef.moravec@gmail.com> --- I look at Koha::Account, and it does make sense to add it do log, do you have anything else in mind? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21401 --- Comment #23 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- I think we should leave the FinesLog stuff in (for backwards compatibility - perhaps we can create a deprecation plan for removing this in the future). Both the 'pay' and 'add_credit' methods need minor adjustments to account for the new branchcode recording. These are the main routines that should, in theory, be used throughout Koha to work with the accountlines and account_offset tables for payments and credits. (though I'd leave in your adaptation to 'new' in accountlines as it's a good catchall too). Have you considered the debits case at all? With you're overriden 'new' method in accountlines you'll be recording branchcodes with a fallback to userenv, but I have not yet walked the codepath backwards to check that fine creation etc would ever actually pass through a branchcode (or whether userenv will be set and make sense). All worth having a quick look at. Please don't be put off.. I do like this enhancement :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21401 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |tomascohen@gmail.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21401 --- Comment #24 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Oooh.. also.. we'de need library_id adding to the existing api endpoints (especially aiming for this release as the endpoint will 'go live' with 18.11 for the first time) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21401 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |ASSIGNED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21401 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=19066 --- Comment #25 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Same as bug 19066. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21401 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |19066 --- Comment #26 from Josef Moravec <josef.moravec@gmail.com> --- I will rework this on top of 19066 - would like to add foreign key - api changes Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19066 [Bug 19066] Add branchcode to accountlines -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21401 Bug 21401 depends on bug 19066, which changed state. Bug 19066 Summary: Add branchcode to accountlines https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19066 What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to Master |RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21401 --- Comment #27 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Was there any further work you wanted to undertake here Josef.. or did it all end up folded into bug 19066? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21401 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |RESOLVED Resolution|--- |WONTFIX --- Comment #28 from Josef Moravec <josef.moravec@gmail.com> --- (In reply to Martin Renvoize from comment #27)
Was there any further work you wanted to undertake here Josef.. or did it all end up folded into bug 19066?
For me, it is covered by bug 19066, so I am not going to continue on this. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21401 --- Comment #29 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Brill, thanks for the update Josef. 😀 -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org