Bug 37050 - Add select2 to SQL report multi select
Summary: Add select2 to SQL report multi select
Status: Signed Off
Alias: None
Product: Koha
Classification: Unclassified
Component: Reports (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement
Assignee: Brendan Lawlor
QA Contact: Pedro Amorim
URL:
Keywords:
: 37886 (view as bug list)
Depends on:
Blocks:
 
Reported: 2024-06-06 19:45 UTC by Brendan Lawlor
Modified: 2025-03-26 16:21 UTC (History)
3 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:


Attachments
Bug 37050: Add select2 to SQL reports multi select (4.56 KB, patch)
2024-06-06 20:07 UTC, Brendan Lawlor
Details | Diff | Splinter Review
Run report screen (52.75 KB, image/png)
2024-06-12 13:45 UTC, Pedro Amorim
Details
Bug 37050: Add select2 to SQL reports multi select (4.62 KB, patch)
2024-06-18 12:57 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 37050: (follow-up) CSS and configuration tweak (1.93 KB, patch)
2024-06-18 12:57 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 37050: Add select2 to SQL reports multi select (4.67 KB, patch)
2024-06-18 13:06 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 37050: (follow-up) CSS and configuration tweak (1.99 KB, patch)
2024-06-18 13:06 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 37050: Add select2 to SQL reports multi select (4.60 KB, patch)
2024-07-26 15:28 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 37050: (follow-up) CSS and configuration tweak (1.93 KB, patch)
2024-07-26 15:28 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 37050: Add select2 to SQL reports multi select (4.66 KB, patch)
2024-07-29 10:01 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 37050: (follow-up) CSS and configuration tweak (2.07 KB, patch)
2024-07-29 10:01 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 37050: Add select2 to SQL reports multi select (3.80 KB, patch)
2024-09-11 15:03 UTC, Brendan Lawlor
Details | Diff | Splinter Review
Bug 37050: (follow-up) CSS and configuration tweak (2.07 KB, patch)
2024-09-11 15:03 UTC, Brendan Lawlor
Details | Diff | Splinter Review
Bug 37050: (follow-up) Add select2 to SQL reports multiselect (3.47 KB, patch)
2024-09-11 15:03 UTC, Brendan Lawlor
Details | Diff | Splinter Review
Bug 37050: Add select2 to SQL reports multi select (3.86 KB, patch)
2024-09-25 21:58 UTC, Biblibre Sandboxes
Details | Diff | Splinter Review
Bug 37050: (follow-up) CSS and configuration tweak (2.13 KB, patch)
2024-09-25 21:58 UTC, Biblibre Sandboxes
Details | Diff | Splinter Review
Bug 37050: (follow-up) Add select2 to SQL reports multiselect (3.52 KB, patch)
2024-09-25 21:58 UTC, Biblibre Sandboxes
Details | Diff | Splinter Review
Bug 37050: Add select2 to SQL reports multiselect (5.43 KB, patch)
2025-01-29 20:32 UTC, Brendan Lawlor
Details | Diff | Splinter Review
Bug 37050: Add select2 to SQL reports multiselect (6.11 KB, patch)
2025-03-24 18:16 UTC, Brendan Lawlor
Details | Diff | Splinter Review
Bug 37050: Add select2 to SQL reports multiselect (6.17 KB, patch)
2025-03-26 16:20 UTC, Biblibre Sandboxes
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Brendan Lawlor 2024-06-06 19:45:52 UTC
Bug 36545 adds select2 to improve the user experience when making multiple selections in item search.

Bug 35746 adds the ability to reports to make multiple selections for SQL parameters.

It would be nice for consistency and ux for the report multiselect to use select2.
Comment 1 Brendan Lawlor 2024-06-06 20:07:29 UTC Comment hidden (obsolete)
Comment 2 Pedro Amorim 2024-06-12 13:45:10 UTC
Created attachment 167659 [details]
Run report screen

Something seems wrong with the UI. This is what I'm seeing even after yarn css:build.
Comment 3 Brendan Lawlor 2024-06-12 13:55:43 UTC
Thanks for looking at this Pedro!

I think there is some css needed to make the little chips appear fit to the value and in line, like they do in the new item search, but I haven't been able to look much into it yet.
Comment 4 Owen Leonard 2024-06-18 12:57:26 UTC Comment hidden (obsolete)
Comment 5 Owen Leonard 2024-06-18 12:57:28 UTC Comment hidden (obsolete)
Comment 6 Owen Leonard 2024-06-18 13:01:40 UTC
(In reply to Brendan Lawlor from comment #3)
> I think there is some css needed to make the little chips appear fit

It was necessary to override some of the CSS inherited from fieldset.rows li.
Comment 7 Pedro Amorim 2024-06-18 13:06:23 UTC Comment hidden (obsolete)
Comment 8 Pedro Amorim 2024-06-18 13:06:26 UTC Comment hidden (obsolete)
Comment 9 Owen Leonard 2024-07-26 15:28:06 UTC Comment hidden (obsolete)
Comment 10 Owen Leonard 2024-07-26 15:28:08 UTC Comment hidden (obsolete)
Comment 11 Owen Leonard 2024-07-26 15:28:23 UTC
Rebased.
Comment 12 Pedro Amorim 2024-07-29 10:01:30 UTC Comment hidden (obsolete)
Comment 13 Pedro Amorim 2024-07-29 10:01:33 UTC Comment hidden (obsolete)
Comment 14 Katrin Fischer 2024-08-23 17:00:12 UTC
I am unsure about the small conflict here. It's about this line:

<option value="null" hidden></option>

Previously changed by bug 37077. 

Can you please have a look and rebase yourself since you wrote the other patch too?
Comment 15 Brendan Lawlor 2024-09-10 20:38:34 UTC
*** Bug 37886 has been marked as a duplicate of this bug. ***
Comment 16 Brendan Lawlor 2024-09-11 15:03:52 UTC Comment hidden (obsolete)
Comment 17 Brendan Lawlor 2024-09-11 15:03:54 UTC Comment hidden (obsolete)
Comment 18 Brendan Lawlor 2024-09-11 15:03:56 UTC Comment hidden (obsolete)
Comment 19 Biblibre Sandboxes 2024-09-25 21:58:04 UTC
Created attachment 171957 [details] [review]
Bug 37050: Add select2 to SQL reports multi select

This patch adds select 2 to multi-select parameter fields in
SQL reports. If no selections are made for a multi-select
all authorised values will be used by default. This allows reports
to be written where a multi select parameter is optional.

To test:
1. Apply patch
2. Go to Reports and create a report from SQL
   select *
   from items
   where
   homebranch in <<Libraries|branches:in>>
   and
   itype in <<Item type|itemtypes:in>>
3. Run the report making multiple selections for each parameter
    and making single selections for each parameter
4. Run the report making multiple selections for one parameter and no
   selections in another parameter
5. Check that the results are as you would expect
6. On the results page click 'Show SQL code'
7. Confirm that the codes are included according to your selections
8. Note that when no selections are made for a parameter all codes are
   used by defaut

Sponsored-by: CLAMS
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>
Comment 20 Biblibre Sandboxes 2024-09-25 21:58:07 UTC
Created attachment 171958 [details] [review]
Bug 37050: (follow-up) CSS and configuration tweak

This follow-up tweaks the Select2 CSS so that the selected choices don't
display with 100% width.

The patch also removes the "closeOnSelect: false" configuration from
Select2. In discussing this option for Bug 36545 more people preferred
the default behavior and I think we should be consistent.

PA amended commit message: `yarn css:build` is required to test these
CSS changes

Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>
Comment 21 Biblibre Sandboxes 2024-09-25 21:58:09 UTC
Created attachment 171959 [details] [review]
Bug 37050: (follow-up) Add select2 to SQL reports multiselect

Rebase for changes made on bug 37077
This patch removes some javascript in favor of an update to the template.
The default value for multi select parameter's is a newline separated list
so that report results are not limited when no selections are made.

Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>
Comment 22 Pedro Amorim 2024-10-29 12:07:10 UTC
Taking another look here
Comment 23 Pedro Amorim 2024-10-29 12:26:28 UTC
Observations:
It feels a bit weird to allow for an empty selection (causing an empty box with an x), and have that empty option behave the same as if no option was selected at all. Should we just not show an empty option in this case? The user can deselect any selections and submit it empty anyway.

Found the following error:
A) Follow steps 1 and 2 of the test plan, submit the empty form -> all good
B) Follow steps 1 and 2 of the test plan, pick a single library, then remove, submit the empty form -> BOOM

This may have been caused by bug 35746 and not the patches here.

For the reasons above I'm not comfortable putting this back to PQA, but I don't think they're strong enough reasons to FQA either (1 is an observation, the other a bug that I don't relates to patches here).

@Brendan, what do you think of the comments above?
Comment 24 Brendan Lawlor 2024-10-30 15:54:27 UTC
Hi Pedro, thanks for finding these issues!

(In reply to Pedro Amorim from comment #23)
> Observations:
> It feels a bit weird to allow for an empty selection (causing an empty box
> with an x), and have that empty option behave the same as if no option was
> selected at all. Should we just not show an empty option in this case? The
> user can deselect any selections and submit it empty anyway.

I hadn't noticed that the hidden select option is selectable in the select2 dropdown. I agree this should be fixed. It seems to be that select2 is not respecting the hidden property on that first option for some reason.

> Found the following error:
> A) Follow steps 1 and 2 of the test plan, submit the empty form -> all good
> B) Follow steps 1 and 2 of the test plan, pick a single library, then
> remove, submit the empty form -> BOOM

The template fills all the codes into the hidden option and then every time the user updates selections the value of the hidden option is updated. This error is happening because of an edge case when everything is deselected the value is becoming an empty string when it should revert to a newline delimited list of all codes.

I will come back to this and submit a new patch as soon as I can. Thanks!
Comment 25 Katrin Fischer 2024-11-21 14:26:21 UTC
@Pedro and @Brendan: Hi there, reviewing "additional_work_needed" - can you please review and file new bugs if needed?
Comment 26 Brendan Lawlor 2024-11-21 14:48:09 UTC
(In reply to Katrin Fischer from comment #25)
> @Pedro and @Brendan: Hi there, reviewing "additional_work_needed" - can you
> please review and file new bugs if needed?


Hi Katrin!

I added "additional_work_needed" to remind me to come back to this one. Should I have just set it to Failed QA instead?

I intend to work on this but haven't had the time recently.
Comment 27 Katrin Fischer 2024-11-21 14:49:45 UTC
(In reply to Brendan Lawlor from comment #26)
> (In reply to Katrin Fischer from comment #25)
> > @Pedro and @Brendan: Hi there, reviewing "additional_work_needed" - can you
> > please review and file new bugs if needed?
> 
> 
> Hi Katrin!
> 
> I added "additional_work_needed" to remind me to come back to this one.
> Should I have just set it to Failed QA instead?
> 
> I intend to work on this but haven't had the time recently.

Hah! That was my mistake, we usually use the additional_work_needed for bugs already pushed that still need something done. Failed QA or back to Assigned is fine if you need to work on it still.
Comment 28 Brendan Lawlor 2025-01-29 20:32:38 UTC
Created attachment 177320 [details] [review]
Bug 37050: Add select2 to SQL reports multiselect

This patch adds select 2 to multi-select parameter fields in
SQL reports. If no selections are made for a multi-select
all authorised values will be used by default. This allows reports
to be written where a multi select parameter is optional.

To test:
1. Apply patch
2. Go to Reports and create a report from SQL
   select *
   from items
   where
   homebranch in <<Libraries|branches:in>>
   and
   itype in <<Item type|itemtypes:in>>
3. Run the report making multiple selections for each parameter
    and making single selections for each parameter
4. Run the report making multiple selections for one parameter and no
   selections in another parameter
5. Check that the results are as you would expect
6. On the results page click 'Show SQL code'
7. Confirm that the codes are included according to your selections
8. Note that when no selections are made for a parameter all codes are
   used by defaut

Sponsored-by: CLAMS
Comment 29 Brendan Lawlor 2025-03-24 18:16:50 UTC
Created attachment 179669 [details] [review]
Bug 37050: Add select2 to SQL reports multiselect

This patch adds select 2 to multi-select parameter fields in
SQL reports. If no selections are made for a multi-select
all authorised values will be used by default. This allows reports
to be written where a multi select parameter is optional.

To test:
1. Apply patch
2. Go to Reports and create a report from SQL
   select *
   from items
   where
   homebranch in <<Libraries|branches:in>>
   and
   itype in <<Item type|itemtypes:in>>
3. Run the report making multiple selections for each parameter
    and making single selections for each parameter
4. Run the report making multiple selections for one parameter and no
   selections in another parameter
5. Check that the results are as you would expect
6. On the results page click 'Show SQL code'
7. Confirm that the codes are included according to your selections
8. Note that when no selections are made for a parameter all codes are
   used by defaut

Sponsored-by: CLAMS
Comment 30 Brendan Lawlor 2025-03-24 18:18:16 UTC
rebased for main
Comment 31 Biblibre Sandboxes 2025-03-26 16:20:30 UTC
Created attachment 179728 [details] [review]
Bug 37050: Add select2 to SQL reports multiselect

This patch adds select 2 to multi-select parameter fields in
SQL reports. If no selections are made for a multi-select
all authorised values will be used by default. This allows reports
to be written where a multi select parameter is optional.

To test:
1. Apply patch
2. Go to Reports and create a report from SQL
   select *
   from items
   where
   homebranch in <<Libraries|branches:in>>
   and
   itype in <<Item type|itemtypes:in>>
3. Run the report making multiple selections for each parameter
    and making single selections for each parameter
4. Run the report making multiple selections for one parameter and no
   selections in another parameter
5. Check that the results are as you would expect
6. On the results page click 'Show SQL code'
7. Confirm that the codes are included according to your selections
8. Note that when no selections are made for a parameter all codes are
   used by defaut

Sponsored-by: CLAMS
Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>
Comment 32 Michaela Sieber 2025-03-26 16:21:37 UTC
Ready for Pedro ;-)