Bug 34431

Summary: Distinguish between status and status alias in ILL UI
Product: Koha Reporter: Pedro Amorim <pedro.amorim>
Component: ILLAssignee: Pedro Amorim <pedro.amorim>
Status: Needs documenting --- QA Contact: Tomás Cohen Arazi (tcohen) <tomascohen>
Severity: enhancement    
Priority: P5 - low CC: fridolin.somers, jonathan.field, magnus, pedro.amorim, tomascohen
Version: Main   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30271
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
24.05.00
Circulation function:
Bug Depends on: 34223    
Bug Blocks: 36105    
Attachments: Bug 34431: Add 'type' to existing_statuses method
Bug 34431: Distinguish status from status_alias in filter dropdown
Bug 34431: Distinguish status from status_alias in table column
Bug 34431: Update tests
Bug 34431: Allow for x-koha-embed in ill/backends endpoint
Bug 34431: Show both status and status_alias - if exists - in table column
Bug 34431: Add a status_alias filter input, if any status_alias exist
Bug 34431: Update tests
Bug 34431: Add status and status_alias filters to clear
Bug 34431: Add 'type' to existing_statuses method
Bug 34431: Allow for x-koha-embed in ill/backends endpoint
Bug 34431: Show both status and status_alias - if exists - in table column
Bug 34431: Add a status_alias filter input, if any status_alias exist
Bug 34431: Update tests
Bug 34431: Add status and status_alias filters to clear
Bug 34431: (QA follow-up) Fix duplicate backend options
Bug 34431: Add 'type' to existing_statuses method
Bug 34431: Allow for x-koha-embed in ill/backends endpoint
Bug 34431: Show both status and status_alias - if exists - in table column
Bug 34431: Add a status_alias filter input, if any status_alias exist
Bug 34431: Update tests
Bug 34431: Add status and status_alias filters to clear
Bug 34431: (QA follow-up) Fix duplicate backend options
Bug 34431: Add 'type' to existing_statuses method
Bug 34431: Allow for x-koha-embed in ill/backends endpoint
Bug 34431: Show both status and status_alias - if exists - in table column
Bug 34431: Add a status_alias filter input, if any status_alias exist
Bug 34431: Update tests
Bug 34431: Add status and status_alias filters to clear
Bug 34431: (QA follow-up) Fix duplicate backend options

Description Pedro Amorim 2023-07-27 11:12:08 UTC
At the moment, we're concating status and status_alias in the ILL left filters.
We also have our own custom logic of what to present in the status table column, i.e. if a status_alias exists, we show that over status.

This may cause confusing as some users are filtering for status, but table is showing status_alias.

We need to find a way to make it obvious which status a request is at, and which status_alias (if any) is at.
Comment 1 Pedro Amorim 2023-07-28 10:23:53 UTC
Created attachment 154034 [details] [review]
Bug 34431: Add 'type' to existing_statuses method
Comment 2 Pedro Amorim 2023-07-28 10:23:55 UTC
Created attachment 154035 [details] [review]
Bug 34431: Distinguish status from status_alias in filter dropdown

Add a new optgroup in status dropdown
Comment 3 Pedro Amorim 2023-07-28 10:23:58 UTC
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
Comment 4 Pedro Amorim 2023-07-28 10:24:00 UTC
Created attachment 154037 [details] [review]
Bug 34431: Update tests
Comment 5 Pedro Amorim 2023-07-28 10:44:10 UTC
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
Comment 6 Pedro Amorim 2023-08-04 10:18:51 UTC
I'm going to implement this differently.
Comment 7 Pedro Amorim 2023-08-04 16:07:35 UTC
Created attachment 154289 [details] [review]
Bug 34431: Allow for x-koha-embed in ill/backends endpoint
Comment 8 Pedro Amorim 2023-08-04 16:07:38 UTC
Created attachment 154290 [details] [review]
Bug 34431: Show both status and status_alias - if exists - in table column
Comment 9 Pedro Amorim 2023-08-04 16:07:41 UTC
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.
Comment 10 Pedro Amorim 2023-08-04 16:07:44 UTC
Created attachment 154292 [details] [review]
Bug 34431: Update tests
Comment 11 Pedro Amorim 2023-08-07 10:06:01 UTC
Created attachment 154301 [details] [review]
Bug 34431: Add status and status_alias filters to clear
Comment 12 Pedro Amorim 2023-08-14 15:55:31 UTC
Created attachment 154405 [details] [review]
Bug 34431: Add 'type' to existing_statuses method

Signed-off-by: Stephen Graham <s.graham4@herts.ac.uk>
Comment 13 Pedro Amorim 2023-08-14 15:55:34 UTC
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>
Comment 14 Pedro Amorim 2023-08-14 15:55:37 UTC
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>
Comment 15 Pedro Amorim 2023-08-14 15:55:40 UTC
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>
Comment 16 Pedro Amorim 2023-08-14 15:55:42 UTC
Created attachment 154409 [details] [review]
Bug 34431: Update tests

Signed-off-by: Stephen Graham <s.graham4@herts.ac.uk>
Comment 17 Pedro Amorim 2023-08-14 15:55:45 UTC
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>
Comment 18 Pedro Amorim 2023-08-31 08:57:30 UTC
Created attachment 154999 [details] [review]
Bug 34431: (QA follow-up) Fix duplicate backend options

Prevent adding backend option if the option already exists
Comment 19 Pedro Amorim 2023-10-30 09:06:58 UTC
*** Bug 33685 has been marked as a duplicate of this bug. ***
Comment 20 Tomás Cohen Arazi (tcohen) 2024-01-17 12:33:13 UTC
Please rebase and put back at SO. Thanks
Comment 21 Pedro Amorim 2024-01-17 13:32:14 UTC
Created attachment 161104 [details] [review]
Bug 34431: Add 'type' to existing_statuses method

Signed-off-by: Stephen Graham <s.graham4@herts.ac.uk>
Comment 22 Pedro Amorim 2024-01-17 13:32:16 UTC
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>
Comment 23 Pedro Amorim 2024-01-17 13:32:19 UTC
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>
Comment 24 Pedro Amorim 2024-01-17 13:32:22 UTC
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>
Comment 25 Pedro Amorim 2024-01-17 13:32:24 UTC
Created attachment 161108 [details] [review]
Bug 34431: Update tests

Signed-off-by: Stephen Graham <s.graham4@herts.ac.uk>
Comment 26 Pedro Amorim 2024-01-17 13:32:27 UTC
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>
Comment 27 Pedro Amorim 2024-01-17 13:32:30 UTC
Created attachment 161110 [details] [review]
Bug 34431: (QA follow-up) Fix duplicate backend options

Prevent adding backend option if the option already exists
Comment 28 Tomás Cohen Arazi (tcohen) 2024-01-18 12:58:49 UTC
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>
Comment 29 Tomás Cohen Arazi (tcohen) 2024-01-18 12:58:52 UTC
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>
Comment 30 Tomás Cohen Arazi (tcohen) 2024-01-18 12:58:55 UTC
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>
Comment 31 Tomás Cohen Arazi (tcohen) 2024-01-18 12:58:58 UTC
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>
Comment 32 Tomás Cohen Arazi (tcohen) 2024-01-18 12:59:01 UTC
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>
Comment 33 Tomás Cohen Arazi (tcohen) 2024-01-18 12:59:04 UTC
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>
Comment 34 Tomás Cohen Arazi (tcohen) 2024-01-18 12:59:07 UTC
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>
Comment 35 Katrin Fischer 2024-03-22 13:27:50 UTC
> 2) Pet a cat

Love! But I didn't find a cat to pet :(
Comment 36 Katrin Fischer 2024-03-22 13:31:49 UTC
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?
Comment 37 Katrin Fischer 2024-03-22 14:09:09 UTC
Pushed for 24.05!

Well done everyone, thank you!
Comment 38 Pedro Amorim 2024-03-25 10:12:05 UTC
(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/
Comment 39 Katrin Fischer 2024-03-25 16:17:58 UTC
> > 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)
Comment 40 Pedro Amorim 2024-03-25 16:53:36 UTC
(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.
Comment 41 Katrin Fischer 2024-03-25 19:52:48 UTC
(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!
Comment 42 Fridolin Somers 2024-05-23 07:35:07 UTC
Not backported to 23.11.x
Comment 43 Katrin Fischer 2024-05-26 19:02:25 UTC
(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?
Comment 44 Pedro Amorim 2024-05-27 09:34:48 UTC
(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.
Comment 45 Katrin Fischer 2024-05-27 10:04:39 UTC
(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!