Having branch transfers in the action logs would help to more fully show the cycle of holds that require transfers.
Created attachment 144543 [details] [review] Bug 32034: Add item transfers to action logs Having branch transfers in the action logs would help to more fully show the cycle of holds that require transfers. Test Plan: 1) Apply this patch 2) Run updatedatabase.pl 3) Restart all the things! 4) Enable the new syspref LogTransfers 5) Create and finish some transfers, note the new logs in the log viewer!
This works, but there's a typo that causes the module to get written as "TRANFERS" rather than "TRANSFERS" (missing the 1st S). I tried to edit it and sign off on the amended patch, but I did something wrong and git yelled at me.
Created attachment 144581 [details] [review] Bug 32034: Add item transfers to action logs
(In reply to Andrew Fuerste-Henry from comment #2) > This works, but there's a typo that causes the module to get written as > "TRANFERS" rather than "TRANSFERS" (missing the 1st S). I tried to edit it > and sign off on the amended patch, but I did something wrong and git yelled > at me. Good catch! Fixed!
Created attachment 144582 [details] [review] Bug 32366: (bug 30460 follow-up) Add tests Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org>
I'm not sure what happened when I tried to sign off on this before, but I can confirm this patch doesn't apply. I get this error in the provision logs: TASK [Apply bug 32034 via git-bz in docker container] ************************** fatal: [localhost -> koha-boo]: FAILED! => {"changed": true, "cmd": "cd /kohadevbox/koha && yes | git bz apply 32034", "delta": "0:00:05.882738", "end": "2023-01-09 15:54:40.137836", "msg": "non-zero return code", "rc": 1, "start": "2023-01-09 15:54:34.255098", "stderr": "Traceback (most recent call last):\n File \"/usr/bin/git-bz\", line 2716, in <module>\n applied = do_apply(bug_ref)\n File \"/usr/bin/git-bz\", line 1791, in do_apply\n print \"%d - %s\" % (patch.attach_id, patch.description)\nUnicodeEncodeError: 'ascii' codec can't encode character u'\\xa0' in position 12: ordinal not in range(128)", "stderr_lines": ["Traceback (most recent call last):", " File \"/usr/bin/git-bz\", line 2716, in <module>", " applied = do_apply(bug_ref)", " File \"/usr/bin/git-bz\", line 1791, in do_apply", " print \"%d - %s\" % (patch.attach_id, patch.description)", "UnicodeEncodeError: 'ascii' codec can't encode character u'\\xa0' in position 12: ordinal not in range(128)"], "stdout": "\nBug 32034 - Library branch transfers should be in the action logs", "stdout_lines": ["", "Bug 32034 - Library branch transfers should be in the action logs"]}
(In reply to Andrew Fuerste-Henry from comment #6) > I'm not sure what happened when I tried to sign off on this before, but I > can confirm this patch doesn't apply. I get this error in the provision logs: Ack! The unicode error again! I was able to go into your sandbox and apply the bug using the koha-qa alias. No errors. I suppose that means ansible doesn't like some unicode character in the file or something. In any case, I created a sandbox for you with this patch applied: https://sandbox.bywatersolutions.com/ so you should be good to go!
* Commit title does not start with 'Bug XXXXX: ' - ce32863a1e
(In reply to Marcel de Rooy from comment #8) > * Commit title does not start with 'Bug XXXXX: ' - ce32863a1e Fixed but a strange one..
(In reply to Marcel de Rooy from comment #9) > (In reply to Marcel de Rooy from comment #8) > > * Commit title does not start with 'Bug XXXXX: ' - ce32863a1e > > Fixed but a strange one.. [PATCH] =?utf-8?q?Bug=C2=A032034:=20Add=20item=20transfers=20to?= =?utf-8?q?=20action=20logs?=
Viewing history and commit message, this never got a signoff. Changing status.
Wanted to sign off on this as several partners are very interested in it, but looks like it hit the same encoding error as when Andrew tested: TASK [Apply bug 32034 via git-bz in docker container] ************************** fatal: [localhost -> koha-transferlog]: FAILED! => {"changed": true, "cmd": "koha-shell -c 'cd /kohadevbox/koha && yes | git bz apply 32034' transferlog", "delta": "0:00:03.408310", "end": "2023-03-03 21:17:20.388399", "msg": "non-zero return code", "rc": 1, "start": "2023-03-03 21:17:16.980089", "stderr": "Traceback (most recent call last):\n File \"/usr/bin/git-bz\", line 2716, in <module>\n applied = do_apply(bug_ref)\n File \"/usr/bin/git-bz\", line 1791, in do_apply\n print \"%d - %s\" % (patch.attach_id, patch.description)\nUnicodeEncodeError: 'ascii' codec can't encode character u'\\xa0' in position 12: ordinal not in range(128)", "stderr_lines": ["Traceback (most recent call last):", " File \"/usr/bin/git-bz\", line 2716, in <module>", " applied = do_apply(bug_ref)", " File \"/usr/bin/git-bz\", line 1791, in do_apply", " print \"%d - %s\" % (patch.attach_id, patch.description)", "UnicodeEncodeError: 'ascii' codec can't encode character u'\\xa0' in position 12: ordinal not in range(128)"], "stdout": "\nBug 32034 - Library branch transfers should be in the action logs", "stdout_lines": ["", "Bug 32034 - Library branch transfers should be in the action logs"]}
Taking it from Kyle to help with it.
Created attachment 180991 [details] [review] Bug 32034: Add new syspref 'TransferLog' Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 180992 [details] [review] Bug 32034: Add item transfers to action logs Having branch transfers in the action logs would help to more fully show the cycle of holds that require transfers. Test Plan: 1) Apply this patch 2) Run updatedatabase.pl 3) Restart all the things! 4) Enable the new syspref LogTransfers 5) Create and finish some transfers, note the new logs in the log viewer! Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 180993 [details] [review] Bug 32034: Add more tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Ok, I redid the changes and attributed them to Kyle. Basically the `viewlog.tt` template had some portions moved to an include so it would never apply. Test counts, the mega-tidy action got in the middle too, etc. I moved the syspref setting to the 'new style' and on its own commit. I kept Kyle's tests on the second commit, but added my own tests on a follow-up. On writing tests I noticed a couple bugs! I fixed them inline on Kyle's patch: * When you create the object and \'NOW()' is passed, it makes the JSON processing explode (and is not useful either!) because it cannot process a scalar reference. So I made it force re-reading from the DB. * The `$active` variable was always 'in_storage' because it was read after calling `$self->SUPER::store()`. I did this instead of failing, with Kyle's permission because he won't be able to come back soon. I tested the feature and it works as described/expected. QA script happy, besides a false negative message about sysprefs.sql.