|
Description
Pedro Amorim
2024-02-16 16:22:50 UTC
Created attachment 162277 [details] [review] Bug 36118: [DONT PUSH] patch to test patron notice only Created attachment 162278 [details] [review] Bug 36118: Send patron object to template Created attachment 162279 [details] [review] Bug 36118: Update status and send notice core actions logging 1) Enable ILLlog sys pref 2) Enable ILLModule and install 'FreeForm', run: bash <(curl -s https://raw.githubusercontent.com/ammopt/koha-ill-dev/master/start-ill-dev.sh) 3) Create a new FreeForm request, visit: <INTRA_URL>/cgi-bin/koha/ill/ill-requests.pl?method=create&backend=FreeForm 4) Pick a type, insert '42' on cardnumber and pick a library, hit 'Create' 5) On the next screen, click 'Confirm request' and confirm. 6) Click 'ILL request log', notice the entry is there but has no information about who did the logged action. 7) Apply patches. Repeat steps 3) to 6). Notice the logged action now has information about the user who did the action. 8) Extra: Click 'Send notice to patron' and confirm that action is also logged with patron information. To my knowledge, these are the only 2 actions logged by core Logger. Backends logging their own actions that want patron information need to add it themselves following this pattern. Created attachment 162585 [details] [review] Bug 36118: Send patron object to template Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 162586 [details] [review] Bug 36118: Update status and send notice core actions logging 1) Enable ILLlog sys pref 2) Enable ILLModule and install 'FreeForm', run: bash <(curl -s https://raw.githubusercontent.com/ammopt/koha-ill-dev/master/start-ill-dev.sh) 3) Create a new FreeForm request, visit: <INTRA_URL>/cgi-bin/koha/ill/ill-requests.pl?method=create&backend=FreeForm 4) Pick a type, insert '42' on cardnumber and pick a library, hit 'Create' 5) On the next screen, click 'Confirm request' and confirm. 6) Click 'ILL request log', notice the entry is there but has no information about who did the logged action. 7) Apply patches. Repeat steps 3) to 6). Notice the logged action now has information about the user who did the action. 8) Extra: Click 'Send notice to patron' and confirm that action is also logged with patron information. To my knowledge, these are the only 2 actions logged by core Logger. Backends logging their own actions that want patron information need to add it themselves following this pattern. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 162587 [details] [review] Bug 36118: Tidy changed code block Simple tidy of the codeblock we added a line to so we get through the tidy requirements of the QA script. Nice improvement. Signing off > 4) Pick a type, insert '42' on cardnumber and pick a library, hit 'Create' picking a type results in error 403 > Programming error - No CSRF token passed for POST > http://localhost:8081/intranet/ill/ill-requests.pl?method=create&backend=FreeForm > (referer: > http://localhost:8081/cgi-bin/koha/ill/ill-requests.pl?method=create&backend=FreeForm) Is that already reported? It doesn't seem to be in Bug 36195 's pad and bug 34478 's dependencies. So can't test for now. Code looks good at least :) Created attachment 163178 [details] [review] Bug 36118: [DONT PUSH] patch to test patron notice only Created attachment 163179 [details] [review] Bug 36118: Send patron object to template Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 163180 [details] [review] Bug 36118: Update status and send notice core actions logging 1) Enable ILLlog sys pref 2) Enable ILLModule and install 'FreeForm', run: bash <(curl -s https://raw.githubusercontent.com/ammopt/koha-ill-dev/master/start-ill-dev.sh) 3) Create a new FreeForm request, visit: <INTRA_URL>/cgi-bin/koha/ill/ill-requests.pl?method=create&backend=FreeForm 4) You need to checkout FreeForm's branch compatible with the CSRF work: cd /kohadevbox/koha/Koha/Illbackends/FreeForm git checkout b_36243 koha-plack --restart kohadev 5) Pick a type, insert '42' on cardnumber and pick a library, hit 'Create' 6) On the next screen, click 'Confirm request' and confirm. 7) Click 'ILL request log', notice the entry is there but has no information about who did the logged action. 8) Apply patches. Repeat steps 3) to 6). Notice the logged action now has information about the user who did the action. 9) Extra: Click 'Send notice to patron' and confirm that action is also logged with patron information. To my knowledge, these are the only 2 actions logged by core Logger. Backends logging their own actions that want patron information need to add it themselves following this pattern. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 163181 [details] [review] Bug 36118: Tidy changed code block Simple tidy of the codeblock we added a line to so we get through the tidy requirements of the QA script. (In reply to Victor Grousset/tuxayo from comment #8) > > 4) Pick a type, insert '42' on cardnumber and pick a library, hit 'Create' > > picking a type results in error 403 > > > Programming error - No CSRF token passed for POST > > http://localhost:8081/intranet/ill/ill-requests.pl?method=create&backend=FreeForm > > (referer: > > http://localhost:8081/cgi-bin/koha/ill/ill-requests.pl?method=create&backend=FreeForm) > > Is that already reported? It doesn't seem to be in Bug 36195 's pad and bug > 34478 's dependencies. > > So can't test for now. > Code looks good at least :) Thanks Victor, the original test plan was missing a step, I've since added it: 4) You need to checkout FreeForm's branch compatible with the CSRF work: cd /kohadevbox/koha/Koha/Illbackends/FreeForm git checkout b_36243 koha-plack --restart kohadev Created attachment 163283 [details] [review] Bug 36118: [DONT PUSH] patch to test patron notice only Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Created attachment 163284 [details] [review] Bug 36118: Send patron object to template Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Created attachment 163285 [details] [review] Bug 36118: Update status and send notice core actions logging 1) Enable ILLlog sys pref 2) Enable ILLModule and install 'FreeForm', run: bash <(curl -s https://raw.githubusercontent.com/ammopt/koha-ill-dev/master/start-ill-dev.sh) 3) Create a new FreeForm request, visit: <INTRA_URL>/cgi-bin/koha/ill/ill-requests.pl?method=create&backend=FreeForm 4) You need to checkout FreeForm's branch compatible with the CSRF work: cd /kohadevbox/koha/Koha/Illbackends/FreeForm git checkout b_36243 koha-plack --restart kohadev 5) Pick a type, insert '42' on cardnumber and pick a library, hit 'Create' 6) On the next screen, click 'Confirm request' and confirm. 7) Click 'ILL request log', notice the entry is there but has no information about who did the logged action. 8) Apply patches. Repeat steps 3) to 6). Notice the logged action now has information about the user who did the action. 9) Extra: Click 'Send notice to patron' and confirm that action is also logged with patron information. To my knowledge, these are the only 2 actions logged by core Logger. Backends logging their own actions that want patron information need to add it themselves following this pattern. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Created attachment 163286 [details] [review] Bug 36118: Tidy changed code block Simple tidy of the codeblock we added a line to so we get through the tidy requirements of the QA script. Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> > 7) Click 'ILL request log', notice the entry is there but has no information about who did the logged action. Testing notes: need to have this commit applied "Bug 36118: [DONT PUSH] patch to test patron notice only" Works, makes sense, QA script happy, code looks good, passing QA :) Created attachment 164181 [details] [review] Bug 36118: [DONT PUSH] patch to test patron notice only Created attachment 164182 [details] [review] Bug 36118: Send patron object to template Created attachment 164183 [details] [review] Bug 36118: Preparation: Move action logs tt methods This moves action logs related methods to its own .inc file in order to be reusable Created attachment 164184 [details] [review] Bug 36118: Move common ILL log parts to single ill-request-log.inc file Created attachment 164185 [details] [review] Bug 36118: Update status and send notice core actions logging 1) Enable ILLlog sys pref 2) Enable ILLModule and install 'FreeForm', run: bash <(curl -s https://raw.githubusercontent.com/ammopt/koha-ill-dev/master/start-ill-dev.sh) 3) Create a new FreeForm request, visit: <INTRA_URL>/cgi-bin/koha/ill/ill-requests.pl?method=create&backend=FreeForm 4) You need to checkout FreeForm's branch compatible with the CSRF work: cd /kohadevbox/koha/Koha/Illbackends/FreeForm git checkout b_36243 koha-plack --restart kohadev 5) Pick a type, insert '42' on cardnumber and pick a library, hit 'Create' 6) On the next screen, click 'Confirm request' and confirm. 7) Click 'ILL request log', notice the entry is there but has no information about who did the logged action. 8) Apply patches. Repeat steps 3) to 6). Notice the logged action now has information about the user who did the action. 9) Extra: Click 'Send notice to patron' and confirm that action is also logged with patron information. To my knowledge, these are the only 2 actions logged by core Logger. Backends logging their own actions that want patron information need to add it themselves following this pattern. Hi guys, Martin and Victor, thanks for SO and QA. Implementing this for another backend, I found some flaws in the original implementation: - patron_notice.tt and status_change.tt were not DRY. - The previous implementation did not consider the possibility of a log entry not having a patron (it may have been added by command-line or cron). The new implementation considers this and displays accordingly. I've revisited it and since this had not yet been pushed I'm resubmitting here. The test plan is the same. Hi Pedro, better to add the Sponsored by to the bug, see here: https://wiki.koha-community.org/wiki/Commit_messages#Sponsor_line And also this here from Martin ;) https://gitlab.com/koha-community/qa-test-tools/-/merge_requests/67 It will look better and can be counted by the release script for the sponsorships I believe. Created attachment 164398 [details] [review] Bug 36118: [DONT PUSH] patch to test patron notice only Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Created attachment 164399 [details] [review] Bug 36118: Send patron object to template Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Created attachment 164400 [details] [review] Bug 36118: Preparation: Move action logs tt methods This moves action logs related methods to its own .inc file in order to be reusable Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Created attachment 164401 [details] [review] Bug 36118: Move common ILL log parts to single ill-request-log.inc file Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Created attachment 164402 [details] [review] Bug 36118: Update status and send notice core actions logging 1) Enable ILLlog sys pref 2) Enable ILLModule and install 'FreeForm', run: bash <(curl -s https://raw.githubusercontent.com/ammopt/koha-ill-dev/master/start-ill-dev.sh) 3) Create a new FreeForm request, visit: <INTRA_URL>/cgi-bin/koha/ill/ill-requests.pl?method=create&backend=FreeForm 4) You need to checkout FreeForm's branch compatible with the CSRF work: cd /kohadevbox/koha/Koha/Illbackends/FreeForm git checkout b_36243 koha-plack --restart kohadev 5) Pick a type, insert '42' on cardnumber and pick a library, hit 'Create' 6) On the next screen, click 'Confirm request' and confirm. 7) Click 'ILL request log', notice the entry is there but has no information about who did the logged action. 8) Apply patches. Repeat steps 3) to 6). Notice the logged action now has information about the user who did the action. 9) Extra: Click 'Send notice to patron' and confirm that action is also logged with patron information. To my knowledge, these are the only 2 actions logged by core Logger. Backends logging their own actions that want patron information need to add it themselves following this pattern. Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> It works :)
---
> [% content | $raw %]
Where is the `content` variable from?
----
Review note: here is how to see the diff with the moved parts colored as such, thus leaving less lines to analyze in details.
git diff upstream/master --color-moved --color-moved-ws=allow-indentation-change
Also, there are some parts with alignment changes that are trivial to read with this:
git diff --color-words=. upstream/master
Thank you, Victor. The content variable contains the HTML within the WRAPPER directive: https://template-toolkit.org/docs/manual/Directives.html#section_WRAPPER Created attachment 164405 [details] [review] Bug 36118: [DONT PUSH] patch to test patron notice only Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Created attachment 164406 [details] [review] Bug 36118: Send patron object to template Sponsored-by: UKHSA - UK Health Security Agency Sponsored-by: PTFS Europe Ltd Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Created attachment 164407 [details] [review] Bug 36118: Preparation: Move action logs tt methods This moves action logs related methods to its own .inc file in order to be reusable Sponsored-by: UKHSA - UK Health Security Agency Sponsored-by: PTFS Europe Ltd Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Created attachment 164408 [details] [review] Bug 36118: Move common ILL log parts to single ill-request-log.inc file Sponsored-by: UKHSA - UK Health Security Agency Sponsored-by: PTFS Europe Ltd Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Created attachment 164409 [details] [review] Bug 36118: Update status and send notice core actions logging 1) Enable ILLlog sys pref 2) Enable ILLModule and install 'FreeForm', run: bash <(curl -s https://raw.githubusercontent.com/ammopt/koha-ill-dev/master/start-ill-dev.sh) 3) Create a new FreeForm request, visit: <INTRA_URL>/cgi-bin/koha/ill/ill-requests.pl?method=create&backend=FreeForm 4) You need to checkout FreeForm's branch compatible with the CSRF work: cd /kohadevbox/koha/Koha/Illbackends/FreeForm git checkout b_36243 koha-plack --restart kohadev 5) Pick a type, insert '42' on cardnumber and pick a library, hit 'Create' 6) On the next screen, click 'Confirm request' and confirm. 7) Click 'ILL request log', notice the entry is there but has no information about who did the logged action. 8) Apply patches. Repeat steps 3) to 6). Notice the logged action now has information about the user who did the action. 9) Extra: Click 'Send notice to patron' and confirm that action is also logged with patron information. To my knowledge, these are the only 2 actions logged by core Logger. Backends logging their own actions that want patron information need to add it themselves following this pattern. Sponsored-by: UKHSA - UK Health Security Agency Sponsored-by: PTFS Europe Ltd Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> (In reply to Pedro Amorim from comment #33) > The content variable contains the HTML within the WRAPPER directive: > https://template-toolkit.org/docs/manual/Directives.html#section_WRAPPER Oh, right! Thanks. Works, makes sense, QA script happy, code looks good, count my QA stamp in :) Needs a signoff, looks a bit too much to directly pass QA. Is it already in production/pre-production? with other people (librarian or colleagues) having used the enhancement? Created attachment 170450 [details] [review] Bug 36118: [DONT PUSH] patch to test patron notice only Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Created attachment 170451 [details] [review] Bug 36118: Send patron object to template Sponsored-by: UKHSA - UK Health Security Agency Sponsored-by: PTFS Europe Ltd Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Created attachment 170452 [details] [review] Bug 36118: Preparation: Move action logs tt methods This moves action logs related methods to its own .inc file in order to be reusable Sponsored-by: UKHSA - UK Health Security Agency Sponsored-by: PTFS Europe Ltd Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Created attachment 170453 [details] [review] Bug 36118: Move common ILL log parts to single ill-request-log.inc file Sponsored-by: UKHSA - UK Health Security Agency Sponsored-by: PTFS Europe Ltd Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Created attachment 170454 [details] [review] Bug 36118: Update status and send notice core actions logging 1) Enable ILLlog sys pref 2) Enable ILLModule and install 'FreeForm', run: bash <(curl -s https://raw.githubusercontent.com/ammopt/koha-ill-dev/master/start-ill-dev.sh) 3) Create a new FreeForm request, visit: <INTRA_URL>/cgi-bin/koha/ill/ill-requests.pl?method=create&backend=FreeForm 4) You need to checkout FreeForm's branch compatible with 24.05: cd /kohadevbox/koha/Koha/Illbackends/FreeForm git checkout 24.05.x koha-plack --restart kohadev 5) Pick a type, insert '42' on cardnumber and pick a library, hit 'Create' 6) On the next screen, click 'Confirm request' and confirm. 7) Click 'ILL request log', notice the entry is there but has no information about who did the logged action. 8) Apply patches. Repeat steps 3) to 6). Notice the logged action now has information about the user who did the action. 9) Extra: Click 'Send notice to patron' and confirm that action is also logged with patron information. To my knowledge, these are the only 2 actions logged by core Logger. Backends logging their own actions that want patron information need to add it themselves following this pattern. Sponsored-by: UKHSA - UK Health Security Agency Sponsored-by: PTFS Europe Ltd Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Created attachment 171372 [details] [review] [23.11] Bug 36118: Send patron object to template Sponsored-by: UKHSA - UK Health Security Agency Sponsored-by: PTFS Europe Ltd Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Created attachment 171373 [details] [review] [23.11] Bug 36118: Preparation: Move action logs tt methods This moves action logs related methods to its own .inc file in order to be reusable Sponsored-by: UKHSA - UK Health Security Agency Sponsored-by: PTFS Europe Ltd Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Created attachment 171374 [details] [review] [23.11] Bug 36118: Move common ILL log parts to single ill-request-log.inc file Sponsored-by: UKHSA - UK Health Security Agency Sponsored-by: PTFS Europe Ltd Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Created attachment 171375 [details] [review] [23.11] Bug 36118: Update status and send notice core actions logging 1) Enable ILLlog sys pref 2) Enable ILLModule and install 'FreeForm', run: bash <(curl -s https://raw.githubusercontent.com/ammopt/koha-ill-dev/master/start-ill-dev.sh) 3) Create a new FreeForm request, visit: <INTRA_URL>/cgi-bin/koha/ill/ill-requests.pl?method=create&backend=FreeForm 4) You need to checkout FreeForm's branch compatible with 24.05: cd /kohadevbox/koha/Koha/Illbackends/FreeForm git checkout 24.05.x koha-plack --restart kohadev 5) Pick a type, insert '42' on cardnumber and pick a library, hit 'Create' 6) On the next screen, click 'Confirm request' and confirm. 7) Click 'ILL request log', notice the entry is there but has no information about who did the logged action. 8) Apply patches. Repeat steps 3) to 6). Notice the logged action now has information about the user who did the action. 9) Extra: Click 'Send notice to patron' and confirm that action is also logged with patron information. To my knowledge, these are the only 2 actions logged by core Logger. Backends logging their own actions that want patron information need to add it themselves following this pattern. Sponsored-by: UKHSA - UK Health Security Agency Sponsored-by: PTFS Europe Ltd Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Created attachment 172785 [details] [review] Bug 36118: [DONT PUSH] patch to test patron notice only Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Created attachment 172786 [details] [review] Bug 36118: Send patron object to template Sponsored-by: UKHSA - UK Health Security Agency Sponsored-by: PTFS Europe Ltd Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Jeremy Evans <Jeremy.Evans@ukhsa.gov.uk> Created attachment 172787 [details] [review] Bug 36118: Preparation: Move action logs tt methods This moves action logs related methods to its own .inc file in order to be reusable Sponsored-by: UKHSA - UK Health Security Agency Sponsored-by: PTFS Europe Ltd Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Jeremy Evans <Jeremy.Evans@ukhsa.gov.uk> Created attachment 172788 [details] [review] Bug 36118: Move common ILL log parts to single ill-request-log.inc file Sponsored-by: UKHSA - UK Health Security Agency Sponsored-by: PTFS Europe Ltd Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Jeremy Evans <Jeremy.Evans@ukhsa.gov.uk> Created attachment 172789 [details] [review] Bug 36118: Update status and send notice core actions logging 1) Enable ILLlog sys pref 2) Enable ILLModule and install 'FreeForm', run: bash <(curl -s https://raw.githubusercontent.com/ammopt/koha-ill-dev/master/start-ill-dev.sh) 3) Create a new FreeForm request, visit: <INTRA_URL>/cgi-bin/koha/ill/ill-requests.pl?method=create&backend=FreeForm 4) You need to checkout FreeForm's branch compatible with 24.05: cd /kohadevbox/koha/Koha/Illbackends/FreeForm git checkout 24.05.x koha-plack --restart kohadev 5) Pick a type, insert '42' on cardnumber and pick a library, hit 'Create' 6) On the next screen, click 'Confirm request' and confirm. 7) Click 'ILL request log', notice the entry is there but has no information about who did the logged action. 8) Apply patches. Repeat steps 3) to 6). Notice the logged action now has information about the user who did the action. 9) Extra: Click 'Send notice to patron' and confirm that action is also logged with patron information. To my knowledge, these are the only 2 actions logged by core Logger. Backends logging their own actions that want patron information need to add it themselves following this pattern. Sponsored-by: UKHSA - UK Health Security Agency Sponsored-by: PTFS Europe Ltd Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Jeremy Evans <Jeremy.Evans@ukhsa.gov.uk> (In reply to Victor Grousset/tuxayo from comment #39) > Needs a signoff, looks a bit too much to directly pass QA. > Is it already in production/pre-production? with other people (librarian or > colleagues) having used the enhancement? Hi Victor. Yes, we have been using it in production and I've now added the sign-off from Jeremy Evans. Thanks, that was the last missing piece. No code change since my last test on Comment 39 QA script still happy. 1) This is untranslatable: +[% WRAPPER ill_request_log action_label = "Patron notice sent: " %] +[% WRAPPER ill_request_log action_label = "Status changed" %] ... The rough guideline for TT is that the parser will parce everything that is inside an HTML element. We can also get some attributes like button texts etc. from HTML elements. But in this here there is no indicator for the translation script that this is a display text. Please fix! Hope you are not going to hate me for it, but please rebase on top of bug 37103. I hope as this is only template changes it won't give to much trouble. Created attachment 173023 [details] [review] Bug 36118: (QA follow-up): Make labels translatable Also move the space+colon from 'Patron notice sent: ' label to ill-request-log.inc template Adjust status_change.tt 'From' capitalized because for status changed this is now: $ Status changed: From "New request" to "Requested" Instead of the previous: $ Status changed from "New request" to "Requested" Created attachment 173024 [details] [review] Bug 36118: [DONT PUSH] patch to test patron notice only Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Created attachment 173025 [details] [review] Bug 36118: Send patron object to template Sponsored-by: UKHSA - UK Health Security Agency Sponsored-by: PTFS Europe Ltd Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Jeremy Evans <Jeremy.Evans@ukhsa.gov.uk> Created attachment 173026 [details] [review] Bug 36118: Preparation: Move action logs tt methods This moves action logs related methods to its own .inc file in order to be reusable Sponsored-by: UKHSA - UK Health Security Agency Sponsored-by: PTFS Europe Ltd Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Jeremy Evans <Jeremy.Evans@ukhsa.gov.uk> Created attachment 173027 [details] [review] Bug 36118: Move common ILL log parts to single ill-request-log.inc file Sponsored-by: UKHSA - UK Health Security Agency Sponsored-by: PTFS Europe Ltd Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Jeremy Evans <Jeremy.Evans@ukhsa.gov.uk> Created attachment 173028 [details] [review] Bug 36118: Update status and send notice core actions logging 1) Enable ILLlog sys pref 2) Enable ILLModule and install 'FreeForm', run: bash <(curl -s https://raw.githubusercontent.com/ammopt/koha-ill-dev/master/start-ill-dev.sh) 3) Create a new FreeForm request, visit: <INTRA_URL>/cgi-bin/koha/ill/ill-requests.pl?method=create&backend=FreeForm 4) You need to checkout FreeForm's branch compatible with 24.05: cd /kohadevbox/koha/Koha/Illbackends/FreeForm git checkout 24.05.x koha-plack --restart kohadev 5) Pick a type, insert '42' on cardnumber and pick a library, hit 'Create' 6) On the next screen, click 'Confirm request' and confirm. 7) Click 'ILL request log', notice the entry is there but has no information about who did the logged action. 8) Apply patches. Repeat steps 3) to 6). Notice the logged action now has information about the user who did the action. 9) Extra: Click 'Send notice to patron' and confirm that action is also logged with patron information. To my knowledge, these are the only 2 actions logged by core Logger. Backends logging their own actions that want patron information need to add it themselves following this pattern. Sponsored-by: UKHSA - UK Health Security Agency Sponsored-by: PTFS Europe Ltd Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Jeremy Evans <Jeremy.Evans@ukhsa.gov.uk> Created attachment 173029 [details] [review] Bug 36118: (QA follow-up): Make labels translatable Also move the space+colon from 'Patron notice sent: ' label to ill-request-log.inc template Adjust status_change.tt 'From' capitalized because for status changed this is now: $ Status changed: From "New request" to "Requested" Instead of the previous: $ Status changed from "New request" to "Requested" Created attachment 173030 [details] [review] Bug 36118: [DONT PUSH] patch to test patron notice only Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Created attachment 173031 [details] [review] Bug 36118: Send patron object to template Sponsored-by: UKHSA - UK Health Security Agency Sponsored-by: PTFS Europe Ltd Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Jeremy Evans <Jeremy.Evans@ukhsa.gov.uk> Created attachment 173032 [details] [review] Bug 36118: Preparation: Move action logs tt methods This moves action logs related methods to its own .inc file in order to be reusable Sponsored-by: UKHSA - UK Health Security Agency Sponsored-by: PTFS Europe Ltd Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Jeremy Evans <Jeremy.Evans@ukhsa.gov.uk> Created attachment 173033 [details] [review] Bug 36118: Move common ILL log parts to single ill-request-log.inc file Sponsored-by: UKHSA - UK Health Security Agency Sponsored-by: PTFS Europe Ltd Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Jeremy Evans <Jeremy.Evans@ukhsa.gov.uk> Created attachment 173034 [details] [review] Bug 36118: Update status and send notice core actions logging 1) Enable ILLlog sys pref 2) Enable ILLModule and install 'FreeForm', run: bash <(curl -s https://raw.githubusercontent.com/ammopt/koha-ill-dev/master/start-ill-dev.sh) 3) Create a new FreeForm request, visit: <INTRA_URL>/cgi-bin/koha/ill/ill-requests.pl?method=create&backend=FreeForm 4) You need to checkout FreeForm's branch compatible with 24.05: cd /kohadevbox/koha/Koha/Illbackends/FreeForm git checkout 24.05.x koha-plack --restart kohadev 5) Pick a type, insert '42' on cardnumber and pick a library, hit 'Create' 6) On the next screen, click 'Confirm request' and confirm. 7) Click 'ILL request log', notice the entry is there but has no information about who did the logged action. 8) Apply patches. Repeat steps 3) to 6). Notice the logged action now has information about the user who did the action. 9) Extra: Click 'Send notice to patron' and confirm that action is also logged with patron information. To my knowledge, these are the only 2 actions logged by core Logger. Backends logging their own actions that want patron information need to add it themselves following this pattern. Sponsored-by: UKHSA - UK Health Security Agency Sponsored-by: PTFS Europe Ltd Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Jeremy Evans <Jeremy.Evans@ukhsa.gov.uk> Created attachment 173035 [details] [review] Bug 36118: (QA follow-up): Make labels translatable Also move the space+colon from 'Patron notice sent: ' label to ill-request-log.inc template Adjust status_change.tt 'From' capitalized because for status changed this is now: $ Status changed: From "New request" to "Requested" Instead of the previous: $ Status changed from "New request" to "Requested" Created attachment 173036 [details] [review] Bug 36118: [DONT PUSH] patch to test patron notice only Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Created attachment 173037 [details] [review] Bug 36118: Send patron object to template Sponsored-by: UKHSA - UK Health Security Agency Sponsored-by: PTFS Europe Ltd Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Jeremy Evans <Jeremy.Evans@ukhsa.gov.uk> Created attachment 173038 [details] [review] Bug 36118: Preparation: Move action logs tt methods This moves action logs related methods to its own .inc file in order to be reusable Sponsored-by: UKHSA - UK Health Security Agency Sponsored-by: PTFS Europe Ltd Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Jeremy Evans <Jeremy.Evans@ukhsa.gov.uk> Created attachment 173039 [details] [review] Bug 36118: Move common ILL log parts to single ill-request-log.inc file Sponsored-by: UKHSA - UK Health Security Agency Sponsored-by: PTFS Europe Ltd Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Jeremy Evans <Jeremy.Evans@ukhsa.gov.uk> Created attachment 173040 [details] [review] Bug 36118: Update status and send notice core actions logging 1) Enable ILLlog sys pref 2) Enable ILLModule and install 'FreeForm', run: bash <(curl -s https://raw.githubusercontent.com/ammopt/koha-ill-dev/master/start-ill-dev.sh) 3) Create a new FreeForm request, visit: <INTRA_URL>/cgi-bin/koha/ill/ill-requests.pl?method=create&backend=FreeForm 4) You need to checkout FreeForm's branch compatible with 24.05: cd /kohadevbox/koha/Koha/Illbackends/FreeForm git checkout 24.05.x koha-plack --restart kohadev 5) Pick a type, insert '42' on cardnumber and pick a library, hit 'Create' 6) On the next screen, click 'Confirm request' and confirm. 7) Click 'ILL request log', notice the entry is there but has no information about who did the logged action. 8) Apply patches. Repeat steps 3) to 6). Notice the logged action now has information about the user who did the action. 9) Extra: Click 'Send notice to patron' and confirm that action is also logged with patron information. To my knowledge, these are the only 2 actions logged by core Logger. Backends logging their own actions that want patron information need to add it themselves following this pattern. Sponsored-by: UKHSA - UK Health Security Agency Sponsored-by: PTFS Europe Ltd Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Jeremy Evans <Jeremy.Evans@ukhsa.gov.uk> Created attachment 173041 [details] [review] Bug 36118: (QA follow-up): Make labels translatable Also move the space+colon from 'Patron notice sent: ' label to ill-request-log.inc template Adjust status_change.tt 'From' capitalized because for status changed this is now: $ Status changed: From "New request" to "Requested" Instead of the previous: $ Status changed from "New request" to "Requested" (In reply to Katrin Fischer from comment #57) > Hope you are not going to hate me for it, but please rebase on top of bug > 37103. > > I hope as this is only template changes it won't give to much trouble. Can you please do a quick rebase? I will get back to it ASAP. Applies nicely for me. Please note to apply interactively and comment out any [23.11] patch. That might have been it, will re-try. (In reply to Katrin Fischer from comment #79) > That might have been it, will re-try. Yep, that was the issue. I think we will need a follow-up then for 23.11 too - leaving with additional_work_needed. Pushed for 24.11! Well done everyone, thank you! Enhancement, no 24.05.x backport |