Description
Pedro Amorim
2023-07-27 11:12:08 UTC
Created attachment 154034 [details] [review] Bug 34431: Add 'type' to existing_statuses method Created attachment 154035 [details] [review] Bug 34431: Distinguish status from status_alias in filter dropdown Add a new optgroup in status dropdown Created attachment 154036 [details] [review] Bug 34431: Distinguish status from status_alias in table column status now always displays. status_alias shows after in italic bold if exists Created attachment 154037 [details] [review] Bug 34431: Update tests The following command will (hopefully) reset your ILL data and create 10k fake ILL requests (run this in DEV KTD only). 1) BEFORE APPLYING PATCH, on an empty k-t-d, run: bash <(curl -s https://raw.githubusercontent.com/ammopt/koha-ill-dev/master/start-ill-dev-data.sh) 2) Pet a cat 3) Visit /cgi-bin/koha/admin/authorised_values.pl add search for "alias", click on ILL_STATUS_ALIAS 4) Click on the "New authorized value for ILL_STATUS_ALIAS" button and add "alias_code" in authorized value and "Alias Description" in Description 5) Visit /cgi-bin/koha/ill/ill-requests.pl and click "Manage request" on a FreeForm request, click "Edit request" 6) Edit the Status and select "Alias Description", click "Submit" 7) Visit /cgi-bin/koha/ill/ill-requests.pl and select FreeForm on the "Backend:" left-side filter 8) Notice how "Status Description" status_alias is now there, mixed with all the other "real" statuses. Click on it and filter on it. Confirm it works 9) Notice the table displays "Status Description" on the "Status" table column 10) Apply patch 11) Repeat 7) Notice how "Status Description" now shows under a new optgroup "Status aliases" 12) Notice the table nows displays whatever status the request has, followed by the "Status Description" status_alias in italic bold I'm going to implement this differently. Created attachment 154289 [details] [review] Bug 34431: Allow for x-koha-embed in ill/backends endpoint Created attachment 154290 [details] [review] Bug 34431: Show both status and status_alias - if exists - in table column Created attachment 154291 [details] [review] Bug 34431: Add a status_alias filter input, if any status_alias exist This patch also makes it so that the status input filter is enabled by default, and shows all existing statuses by default, or all existing statuses for a specific backend, if and when selected. Created attachment 154292 [details] [review] Bug 34431: Update tests Created attachment 154301 [details] [review] Bug 34431: Add status and status_alias filters to clear Created attachment 154405 [details] [review] Bug 34431: Add 'type' to existing_statuses method Signed-off-by: Stephen Graham <s.graham4@herts.ac.uk> Created attachment 154406 [details] [review] Bug 34431: Allow for x-koha-embed in ill/backends endpoint Signed-off-by: Stephen Graham <s.graham4@herts.ac.uk> Created attachment 154407 [details] [review] Bug 34431: Show both status and status_alias - if exists - in table column Signed-off-by: Stephen Graham <s.graham4@herts.ac.uk> Created attachment 154408 [details] [review] Bug 34431: Add a status_alias filter input, if any status_alias exist This patch also makes it so that the status input filter is enabled by default, and shows all existing statuses by default, or all existing statuses for a specific backend, if and when selected. Signed-off-by: Stephen Graham <s.graham4@herts.ac.uk> Created attachment 154409 [details] [review] Bug 34431: Update tests Signed-off-by: Stephen Graham <s.graham4@herts.ac.uk> Created attachment 154410 [details] [review] Bug 34431: Add status and status_alias filters to clear Signed-off-by: Stephen Graham <s.graham4@herts.ac.uk> Created attachment 154999 [details] [review] Bug 34431: (QA follow-up) Fix duplicate backend options Prevent adding backend option if the option already exists *** Bug 33685 has been marked as a duplicate of this bug. *** Please rebase and put back at SO. Thanks Created attachment 161104 [details] [review] Bug 34431: Add 'type' to existing_statuses method Signed-off-by: Stephen Graham <s.graham4@herts.ac.uk> Created attachment 161105 [details] [review] Bug 34431: Allow for x-koha-embed in ill/backends endpoint Signed-off-by: Stephen Graham <s.graham4@herts.ac.uk> Created attachment 161106 [details] [review] Bug 34431: Show both status and status_alias - if exists - in table column Signed-off-by: Stephen Graham <s.graham4@herts.ac.uk> Created attachment 161107 [details] [review] Bug 34431: Add a status_alias filter input, if any status_alias exist This patch also makes it so that the status input filter is enabled by default, and shows all existing statuses by default, or all existing statuses for a specific backend, if and when selected. Signed-off-by: Stephen Graham <s.graham4@herts.ac.uk> Created attachment 161108 [details] [review] Bug 34431: Update tests Signed-off-by: Stephen Graham <s.graham4@herts.ac.uk> Created attachment 161109 [details] [review] Bug 34431: Add status and status_alias filters to clear Signed-off-by: Stephen Graham <s.graham4@herts.ac.uk> Created attachment 161110 [details] [review] Bug 34431: (QA follow-up) Fix duplicate backend options Prevent adding backend option if the option already exists Created attachment 161129 [details] [review] Bug 34431: Add 'type' to existing_statuses method Signed-off-by: Stephen Graham <s.graham4@herts.ac.uk> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 161130 [details] [review] Bug 34431: Allow for x-koha-embed in ill/backends endpoint Signed-off-by: Stephen Graham <s.graham4@herts.ac.uk> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 161131 [details] [review] Bug 34431: Show both status and status_alias - if exists - in table column Signed-off-by: Stephen Graham <s.graham4@herts.ac.uk> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 161132 [details] [review] Bug 34431: Add a status_alias filter input, if any status_alias exist This patch also makes it so that the status input filter is enabled by default, and shows all existing statuses by default, or all existing statuses for a specific backend, if and when selected. Signed-off-by: Stephen Graham <s.graham4@herts.ac.uk> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 161133 [details] [review] Bug 34431: Update tests Signed-off-by: Stephen Graham <s.graham4@herts.ac.uk> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 161134 [details] [review] Bug 34431: Add status and status_alias filters to clear Signed-off-by: Stephen Graham <s.graham4@herts.ac.uk> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 161135 [details] [review] Bug 34431: (QA follow-up) Fix duplicate backend options Prevent adding backend option if the option already exists Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> > 2) Pet a cat
Love! But I didn't find a cat to pet :(
Hm, just leaving a note (unrelated to this patch) : I can no longer create a FreeForm request in the GUI, I thought we had fixed them all, but maybe not? 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) Or it's the backend that requires fixing? Pushed for 24.05! Well done everyone, thank you! (In reply to Katrin Fischer from comment #36) > Hm, just leaving a note (unrelated to this patch) : I can no longer create a > FreeForm request in the GUI, I thought we had fixed them all, but maybe not? > > 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) > > Or it's the backend that requires fixing? Hi Katrin, thanks for noticiing. FreeForm on a Koha running after the CSRF work requires the use of branch b_36243 https://github.com/PTFS-Europe/koha-ill-freeform/commits/b_36243/ > > Or it's the backend that requires fixing?
>
> Hi Katrin, thanks for noticiing.
> FreeForm on a Koha running after the CSRF work requires the use of branch
> b_36243
> https://github.com/PTFS-Europe/koha-ill-freeform/commits/b_36243/
That makes sense. Do we have some kind of plan on how to handle this post/close to release? Maybe a note on README or using the version settings in the plugin info? (not sure if we are really checking for these so far)
(In reply to Katrin Fischer from comment #39) > That makes sense. Do we have some kind of plan on how to handle this > post/close to release? Maybe a note on README or using the version settings > in the plugin info? (not sure if we are really checking for these so far) If bug 35570 is pushed in time for 24.05 this becomes a non-issue I believe, as that is planned to supersede FreeForm. As for other backends, I've tried my best to address compatibility through get_op_param_deprecation in Illrequest.pm but csrf_token needs to exist in the POST form anyway, which means some update work will also be required unless I'm missing some other alternative. ILL backend maintainer's will need to maintain their ILL backends, as it's also the case with regular plugins. (In reply to Pedro Amorim from comment #40) > (In reply to Katrin Fischer from comment #39) > > That makes sense. Do we have some kind of plan on how to handle this > > post/close to release? Maybe a note on README or using the version settings > > in the plugin info? (not sure if we are really checking for these so far) > > If bug 35570 is pushed in time for 24.05 this becomes a non-issue I believe, > as that is planned to supersede FreeForm. > > As for other backends, I've tried my best to address compatibility through > get_op_param_deprecation in Illrequest.pm but csrf_token needs to exist in > the POST form anyway, which means some update work will also be required > unless I'm missing some other alternative. ILL backend maintainer's will > need to maintain their ILL backends, as it's also the case with regular > plugins. Thx for clarifying! Not backported to 23.11.x (In reply to Pedro Amorim from comment #40) > (In reply to Katrin Fischer from comment #39) > > That makes sense. Do we have some kind of plan on how to handle this > > post/close to release? Maybe a note on README or using the version settings > > in the plugin info? (not sure if we are really checking for these so far) > > If bug 35570 is pushed in time for 24.05 this becomes a non-issue I believe, > as that is planned to supersede FreeForm. Hm, we didn't manage to push it in time - anything we need to do here now? (In reply to Katrin Fischer from comment #43) > Hm, we didn't manage to push it in time - anything we need to do here now? I don't think so, no. I've edited the README.md at FreeForm: https://github.com/PTFS-Europe/koha-ill-freeform There's a new branch 24.05.x containing the CSRF compatibility work as well as the structural changes from bug 35581 compatibility work. (In reply to Pedro Amorim from comment #44) > (In reply to Katrin Fischer from comment #43) > > Hm, we didn't manage to push it in time - anything we need to do here now? > > I don't think so, no. > I've edited the README.md at FreeForm: > https://github.com/PTFS-Europe/koha-ill-freeform > > There's a new branch 24.05.x containing the CSRF compatibility work as well > as the structural changes from bug 35581 compatibility work. Thanks! |