Prior to bug 22008 we were defaulting to a manager_id of '0' wherever we found a userenv was not set. This roughly equated to signifying that an action had taken place via a script of cronjob... however I'm not sure I saw this behaviour documented anywhere. With bug 22008 adding constraints to manager_id it highlighted this behaviour and raised the question as to how we should overcome it. I believe the clearest approach to identifying 'where' an action has taken place would be to add an interface field to the accountlines table and set it for any action.
Created attachment 87132 [details] [review] Bug 22600: Add 'interface' to accountlines This patch adds the 'interface' field to the accountlines table and updates all Koha::Object routines and calls to use it. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment on attachment 87132 [details] [review] Bug 22600: Add 'interface' to accountlines Review of attachment 87132 [details] [review]: ----------------------------------------------------------------- ::: C4/Overdues.pm @@ +599,5 @@ > description => $desc, > note => undef, > user_id => undef, > + interface => C4::Context->interface, > + library_id => undef, #FIXME: Should we grab the checkout or circ-control branch here perhaps? On the first view, I think if the Context is set then we should grab it from context, if not, stay with undef ::: C4/SIP/ILS/Transaction/FeePayment.pm @@ +28,4 @@ > our $debug = 0; > > my %fields = (); > +C4::Context->interface('opac'); Should be 'SIP' I think ::: installer/data/mysql/atomicupdate/bug_22600.perl @@ +2,5 @@ > + > +$DBversion = 'XXX'; # will be replaced by the RM > +if ( CheckVersion($DBversion) ) { > + > + if ( !column_exists( 'accountlines', 'status' ) ) { should be 'interface' instead of 'status' @@ +7,5 @@ > + $dbh->do( > + qq{ > + ALTER TABLE `accountlines` > + ADD > + `interface` varchar(16) DEFAULT 'script' 'script' default is weird here @@ +35,5 @@ > + manager_id != borrowernumber; > + }); > + > + SetVersion($DBversion); > + print "Upgrade to $DBversion done (Bug 22518 - Fix accounttype 'O' to 'FU')\n"; Not right description I think ;)
Created attachment 87133 [details] [review] Bug 22600: Add 'interface' to accountlines This patch adds the 'interface' field to the accountlines table and updates all Koha::Object routines and calls to use it. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 87134 [details] [review] Bug 22600: Add 'api' to interface types and set appropriately This patch adds 'api' as a valid interface and sets it appropriately for both the new REST api and previous /svc/ api's. Handling to keep the interface of a logged in session is included such that if the OPAC or Intranet use the API's internally via a Cookie we will maintain the interface throughout the session. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 87137 [details] [review] Bug 22600: Add 'interface' to accountlines This patch adds the 'interface' field to the accountlines table and updates all Koha::Object routines and calls to use it. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 87138 [details] [review] Bug 22600: Add 'api' to interface types and set appropriately This patch adds 'api' as a valid interface and sets it appropriately for both the new REST api and previous /svc/ api's. Handling to keep the interface of a logged in session is included such that if the OPAC or Intranet use the API's internally via a Cookie we will maintain the interface throughout the session. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 87139 [details] [review] Bug 22600: Add 'cron' to interface types and set appropriately This patch adds 'cron' as a valid interface and sets it appropriately for existing cron scripts. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Just started testing, but found: prove t/db_dependent/Koha/Account.t Field 'interface' doesn't have a default value
Created attachment 87140 [details] [review] Bug 22600: Set 'commandline' interface appropriately This patch change Koha::Cron to be a more generic Koha::Script class and update all commanline driven scripts to use it. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 87189 [details] [review] Bug 22600: Add 'interface' to accountlines This patch adds the 'interface' field to the accountlines table and updates all Koha::Object routines and calls to use it. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 87190 [details] [review] Bug 22600: Add 'api' to interface types and set appropriately This patch adds 'api' as a valid interface and sets it appropriately for both the new REST api and previous /svc/ api's. Handling to keep the interface of a logged in session is included such that if the OPAC or Intranet use the API's internally via a Cookie we will maintain the interface throughout the session. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 87191 [details] [review] Bug 22600: Add 'cron' to interface types and set appropriately This patch adds 'cron' as a valid interface and sets it appropriately for existing cron scripts. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 87192 [details] [review] Bug 22600: Set 'commandline' interface appropriately This patch change Koha::Cron to be a more generic Koha::Script class and update all commanline driven scripts to use it. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 87193 [details] [review] Bug 22600: Update Tests adding 'interface' as needed
Hopefully that most recent patchset should address the test failure and your other comments.. back to NSO
Created attachment 87515 [details] [review] Bug 22600: Add 'interface' to accountlines This patch adds the 'interface' field to the accountlines table and updates all Koha::Object routines and calls to use it. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 87516 [details] [review] Bug 22600: Add 'api' to interface types and set appropriately This patch adds 'api' as a valid interface and sets it appropriately for both the new REST api and previous /svc/ api's. Handling to keep the interface of a logged in session is included such that if the OPAC or Intranet use the API's internally via a Cookie we will maintain the interface throughout the session. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 87517 [details] [review] Bug 22600: Add 'cron' to interface types and set appropriately This patch adds 'cron' as a valid interface and sets it appropriately for existing cron scripts. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 87518 [details] [review] Bug 22600: Set 'commandline' interface appropriately This patch change Koha::Cron to be a more generic Koha::Script class and update all commanline driven scripts to use it. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 87519 [details] [review] Bug 22600: Update Tests adding 'interface' as needed Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 87611 [details] [review] Bug 22600: Add 'interface' to accountlines This patch adds the 'interface' field to the accountlines table and updates all Koha::Object routines and calls to use it. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 87612 [details] [review] Bug 22600: Add 'api' to interface types and set appropriately This patch adds 'api' as a valid interface and sets it appropriately for both the new REST api and previous /svc/ api's. Handling to keep the interface of a logged in session is included such that if the OPAC or Intranet use the API's internally via a Cookie we will maintain the interface throughout the session. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 87613 [details] [review] Bug 22600: Add 'cron' to interface types and set appropriately This patch adds 'cron' as a valid interface and sets it appropriately for existing cron scripts. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 87614 [details] [review] Bug 22600: Set 'commandline' interface appropriately This patch change Koha::Cron to be a more generic Koha::Script class and update all commanline driven scripts to use it. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 87615 [details] [review] Bug 22600: Update Tests adding 'interface' as needed Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 87616 [details] [review] Bug 22600: (QA follow-up) Raise an exception on missing interface param This patch makes add_credit and add_debit raise a Koha::Exceptions::MissingParameter exception if the 'interface' parameter is ommited. The database will fail to generate the line anyways in strict mode, and we better handle it gracefuly. Bonus: fixed the TODOs in the tests. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Thanks for the follow-up, looks good to me :)
Awesome work all! Pushed to master for 19.05
Enhancement will not be backported to 18.11.x series.