Bug 35746 - Multiple selections for parameters used in the IN function
Summary: Multiple selections for parameters used in the IN function
Status: Needs documenting
Alias: None
Product: Koha
Classification: Unclassified
Component: Reports (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Brendan Lawlor
QA Contact: Testopia
URL:
Keywords:
: 16002 (view as bug list)
Depends on:
Blocks: 35856 37077
  Show dependency treegraph
 
Reported: 2024-01-09 16:26 UTC by Christopher Brannon
Modified: 2024-06-12 13:43 UTC (History)
13 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
This enhancement adds the ability for report runtime parameters to allow selecting multiple options from the list of authorized values. **Usage**: `WHERE branchcode IN <<Select branches|branches:in>>`
Version(s) released in:
24.05.00


Attachments
Bug 35746: add multiselect to report dropdowns (3.51 KB, patch)
2024-01-11 16:50 UTC, Brendan Lawlor
Details | Diff | Splinter Review
Bug 35746: add multiselect to reports dropdowns (4.62 KB, patch)
2024-01-12 20:33 UTC, Brendan Lawlor
Details | Diff | Splinter Review
Bug 35746: Add multiselect to report dropdowns (7.78 KB, patch)
2024-01-19 21:48 UTC, Brendan Lawlor
Details | Diff | Splinter Review
Bug 35746: Add multiselect to report dropdowns (7.78 KB, patch)
2024-01-22 15:17 UTC, Brendan Lawlor
Details | Diff | Splinter Review
Bug 35746: Add multiselect to report dropdowns (7.84 KB, patch)
2024-01-22 15:22 UTC, Biblibre Sandboxes
Details | Diff | Splinter Review
Bug 35746: add multiselect to report dropdowns (7.09 KB, patch)
2024-02-20 16:15 UTC, Brendan Lawlor
Details | Diff | Splinter Review
Bug 35746: add multiselect to report dropdowns (7.16 KB, patch)
2024-02-20 16:32 UTC, Caroline Cyr La Rose
Details | Diff | Splinter Review
Bug 35746: add multiselect to report dropdowns (7.26 KB, patch)
2024-03-25 19:30 UTC, Brendan Lawlor
Details | Diff | Splinter Review
Bug 35746: add multiselect to report dropdowns (7.33 KB, patch)
2024-03-26 13:22 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 35746: add multiselect to report dropdowns (7.38 KB, patch)
2024-03-26 13:31 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 35746: (follow-up) Fix invalid TT syntax inside html tag (1.58 KB, patch)
2024-04-22 12:39 UTC, Brendan Lawlor
Details | Diff | Splinter Review
Bug 35746: add multiselect to report dropdowns (7.43 KB, patch)
2024-04-22 17:06 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 35746: (follow-up) Fix invalid TT syntax inside html tag (1.63 KB, patch)
2024-04-22 17:06 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 35746: (follow-up) Fix invalid tt in html tag (2.54 KB, patch)
2024-04-22 18:46 UTC, Brendan Lawlor
Details | Diff | Splinter Review
Bug 35746: add multiselect to report dropdowns (7.43 KB, patch)
2024-04-26 15:11 UTC, Brendan Lawlor
Details | Diff | Splinter Review
Bug 35746: (follow-up) Fix invalid tt in html tag (2.53 KB, patch)
2024-04-26 15:11 UTC, Brendan Lawlor
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Christopher Brannon 2024-01-09 16:26:18 UTC
It would be great if when a dropdown selection is used in something like an IN statement in WHERE, that Koha would allow you to select multiple options.

So, for example:

WHERE branchcode IN (<<branches>>)
Comment 1 Barbara Johnson 2024-01-09 16:35:27 UTC
+1 - We have multiple reports where this would be really useful.
Comment 2 Rebecca Coert 2024-01-09 17:56:25 UTC
+1 this would be extremely helpful!
Comment 3 Brendan Lawlor 2024-01-10 19:43:41 UTC
This is a great idea.

I think the report syntax could be similar to when reports include the all option.

<<Select branches| branches:in>>

Then if the 'in' parameter is included guided_reports_start.tt would render that element something like
<select name="sql_params" tabindex="1" id="sql_params_Homebranch" multiple>

so the user could select multiple options.

Then when the report is run the SQL would be somehow generated as a comma separated list of the options the user selected.


For example:
WHERE branchcode IN (<<Select branches|branches:in>>)
would become
WHERE branchcode IN ('opt1', 'opt2', 'opt3')

I think this bug contains some clues to how to implement this:
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=93555&action=diff
Comment 4 Brendan Lawlor 2024-01-11 16:50:14 UTC
Created attachment 160880 [details] [review]
Bug 35746: add multiselect to report dropdowns
Comment 5 Brendan Lawlor 2024-01-11 17:05:19 UTC
Somehow my test plan did not get attached to this patch.

This is a work in progress patch that adds a new syntax to reports for multiselect similar to adding <<Item type|itemtypes:all>>

Currently the patch adds the syntax to the report allowing you to select multiple authorized values when you run the report. The multiselected values are passed as multiple url parameters, but those parameters will need to be parsed and formatted for the query to work. I think this part could be done using either javascript to override the default functionality of the multiselect to send a single parameter, or done in the perl somewhere. 

Test Plan:
1. Try writing a report with the new syntax ie.
   WHERE itemtype in (<<Select item types|itemtypes:in>>)
2. Save the report and notice there is an error
3. Apply patch
4. Write a report with the new syntax
   select * 
   from items 
   where 
   itype in ( <<Select item types|itemtypes:in>> )
5. Note the report now saves without error
6. Run the report
7. Note that you can now select multiple parameters in the dropdown
8. Click 'Run the report'
9. Note the report fails to run
10. Check the url to see the multiselected parameters are included

TODO: Make the multi selected params insert into the SQL as a comma separated list of strings.
Comment 6 Brendan Lawlor 2024-01-12 20:33:39 UTC
Created attachment 160995 [details] [review]
Bug 35746: add multiselect to reports dropdowns

This is a work in progress patch to add multiselect to report dropdowns.

Test Plan:
1. Create an SQL report with with new syntax for multi select
    SELECT *
    FROM branches
    WHERE branchcode in (<<Select branches|branches:in>>)
2. Save the report
3. Note that you get the error
   The authorized value category (branches:in) you selected does not exist.
4. Apply patch and repeat steps 1 and 2
5. Note the report saves successfully
6. Run the report
7. Note the select dropdown is now a multiselect
8. Run the report with multiple selections
9. Note that the multiselect choices are in the url like:
    guided_reports.pl?reports=2&phase=Run+this+report&param_name=Select+branches%7Cbranches%3Ain&sql_params=CPL&sql_params=FFL&sql_params=FPL
10. Click show SQL and note that only the first parameter gets inserted intot he query like:
WHERE branchcode in ('CPL')

TODO: Format the multiselected parameterss as a comma separated list to generate valid SQL like:
    WHERE branch in ('CPL', 'FFL', 'FPL')
Comment 7 Emily Lamancusa 2024-01-17 15:50:55 UTC
Multi-select in reports would be awesome!

This bug adding List parameters might be a good reference for processing the multi-select parameters: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27380 (The list-specific code is currently in Koha/Report.pm, lines 172-183 / near the end of prep_report()) I'm not sure if a list passes the parameters in exactly the same way as a multi-select drop-down, but hopefully that's a helpful starting point?
Comment 8 Brendan Lawlor 2024-01-17 20:44:00 UTC
Thanks for the helpful link Emily, I hadn't found that part of the code yet.

It's a good example to show how to split an input string into an array and then wrap the elements in quotes and separate them by commas.
Comment 9 Brendan Lawlor 2024-01-19 21:48:54 UTC
Created attachment 161224 [details] [review]
Bug 35746: Add multiselect to report dropdowns

This work in progress patch adds the ability to use multi select in report dropdowns.

It uses syntax similar to using a list and when using the :all option
WHERE i.homebranch in <<Select libraries|brnaches:in>>

Currently this works but fails qa scripts because of some forbidden regex

Test plan:
1. Create an SQL report with with new syntax for multi select
    SELECT
    i.homebranch,
    count(*)
    FROM items i
    WHERE i.homebranch in <<<<Select libraries|brnaches:in>>
    GROUP BY i.homebranch
2. Save the report
3. Note that you get the error
    The authorized value category (branches:in) you selected does not exist.
4. Apply the patch and repeat steps 1 and 2
5. Note the report saves successfully
6. Run the report
7. Note the select dropdown is now a multiselect
8. Run the report with multiple selections
10. Click show SQL and note that multi select parameters get inserted into the query like:
     WHERE i.homebranch in ('CPL', 'FFL', 'FPL')
11. Test other types of queries with multiple multi selects and lists etc
Comment 10 Brendan Lawlor 2024-01-22 15:17:14 UTC
Created attachment 161247 [details] [review]
Bug 35746: Add multiselect to report dropdowns

This work in progress patch adds the ability to use multi select in report dropdowns.

It uses syntax similar to using a list and when using the :all option
WHERE i.homebranch in <<Select libraries|brnaches:in>>

Currently this works but fails qa scripts because of some forbidden regex

Test plan:
1. Create an SQL report with with new syntax for multi select
    SELECT
    i.homebranch,
    count(*)
    FROM items i
    WHERE i.homebranch in <<Select libraries|branches:in>>
    GROUP BY i.homebranch
2. Save the report
3. Note that you get the error
    The authorized value category (branches:in) you selected does not exist.
4. Apply the patch and repeat steps 1 and 2
5. Note the report saves successfully
6. Run the report
7. Note the select dropdown is now a multiselect
8. Run the report with multiple selections
10. Click show SQL and note that multi select parameters get inserted into the query like:
     WHERE i.homebranch in ('CPL', 'FFL', 'FPL')
11. Test other types of queries with multiple multi selects and lists etc
Comment 11 Biblibre Sandboxes 2024-01-22 15:22:22 UTC
Created attachment 161248 [details] [review]
Bug 35746: Add multiselect to report dropdowns

This work in progress patch adds the ability to use multi select in report dropdowns.

It uses syntax similar to using a list and when using the :all option
WHERE i.homebranch in <<Select libraries|brnaches:in>>

Currently this works but fails qa scripts because of some forbidden regex

Test plan:
1. Create an SQL report with with new syntax for multi select
    SELECT
    i.homebranch,
    count(*)
    FROM items i
    WHERE i.homebranch in <<<<Select libraries|brnaches:in>>
    GROUP BY i.homebranch
2. Save the report
3. Note that you get the error
    The authorized value category (branches:in) you selected does not exist.
4. Apply the patch and repeat steps 1 and 2
5. Note the report saves successfully
6. Run the report
7. Note the select dropdown is now a multiselect
8. Run the report with multiple selections
10. Click show SQL and note that multi select parameters get inserted into the query like:
     WHERE i.homebranch in ('CPL', 'FFL', 'FPL')
11. Test other types of queries with multiple multi selects and lists etc

Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org>
Comment 12 Andrew Fuerste-Henry 2024-01-22 15:36:44 UTC
*** Bug 16002 has been marked as a duplicate of this bug. ***
Comment 13 Brendan Lawlor 2024-02-20 16:15:54 UTC
Created attachment 162300 [details] [review]
Bug 35746: add multiselect to report dropdowns

This patch adds the ability to use multi select in report dropdowns.

It uses syntax similar to using a list and when using the :all option
WHERE i.homebranch in <<Select libraries|brnaches:in>>

Test plan:
1. Create an SQL report with with new syntax for multi select
    SELECT
    i.homebranch,
    count(*)
    FROM items i
    WHERE i.homebranch in <<Select libraries|branches:in>>
    GROUP BY i.homebranch
2. Save the report
3. Note that you get the error
    The authorized value category (branches:in) you selected does not exist.
4. Apply the patch and repeat steps 1 and 2
5. Note the report saves successfully
6. Run the report
7. Note the select dropdown is now a multiselect
8. Run the report with multiple selections
10. Click show SQL and note that multi select parameters get inserted into the query like:
     WHERE i.homebranch in ('CPL', 'FFL', 'FPL')
11. Test other types of queries with multiple multi selects and lists etc

Sponsored-by: CLAMS
Comment 14 Brendan Lawlor 2024-02-20 16:22:54 UTC
I just fixed the regex and perltidy issues so this patch now passes the QA scripts. 

Also in the work in progress patch I had deleted a block of code in C4/Reports/Guided.pm, because it didn't seem to be doing anything, but this time I left it in there just in case it was important for something else.
Comment 15 Caroline Cyr La Rose 2024-02-20 16:32:41 UTC
Created attachment 162303 [details] [review]
Bug 35746: add multiselect to report dropdowns

This patch adds the ability to use multi select in report dropdowns.

It uses syntax similar to using a list and when using the :all option
WHERE i.homebranch in <<Select libraries|brnaches:in>>

Test plan:
1. Create an SQL report with with new syntax for multi select
    SELECT
    i.homebranch,
    count(*)
    FROM items i
    WHERE i.homebranch in <<Select libraries|branches:in>>
    GROUP BY i.homebranch
2. Save the report
3. Note that you get the error
    The authorized value category (branches:in) you selected does not exist.
4. Apply the patch and repeat steps 1 and 2
5. Note the report saves successfully
6. Run the report
7. Note the select dropdown is now a multiselect
8. Run the report with multiple selections
10. Click show SQL and note that multi select parameters get inserted into the query like:
     WHERE i.homebranch in ('CPL', 'FFL', 'FPL')
11. Test other types of queries with multiple multi selects and lists etc

Sponsored-by: CLAMS
Signed-off-by: Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com>
Comment 16 Caroline Cyr La Rose 2024-02-20 16:34:40 UTC
Works as advertised, very cool!

Probably out of scope, but it would be nice to have a way to add "in" with the parameter button. Maybe a checkbox in the pop up?
Comment 17 Andrew Fuerste-Henry 2024-02-20 16:59:22 UTC
(In reply to Caroline Cyr La Rose from comment #16)
> Works as advertised, very cool!
> 
> Probably out of scope, but it would be nice to have a way to add "in" with
> the parameter button. Maybe a checkbox in the pop up?

That's here! https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35856
Comment 18 Caroline Cyr La Rose 2024-02-20 17:16:27 UTC
I just tested it again on KTD and it gives me a weird error.

For the same report, my local installation returns

SELECT
    i.homebranch,
    count(*)
    FROM items i
    WHERE i.homebranch in ('CPL','MPL')
    GROUP BY i.homebranch /* saved_sql.id: 16 */

And KTD returns

SELECT
    i.homebranch,
    count(*)
    FROM items i
    WHERE i.homebranch in 'CPL\r\nMPL'
    GROUP BY i.homebranch /* saved_sql.id: 1 */

I'm not used to KTD yet, so I don't know what causes the difference. But it definitely works on my local Koha.
Comment 19 Caroline Cyr La Rose 2024-02-20 17:29:21 UTC
(In reply to Caroline Cyr La Rose from comment #18)
> I just tested it again on KTD and it gives me a weird error.
> 
> For the same report, my local installation returns
> 
> SELECT
>     i.homebranch,
>     count(*)
>     FROM items i
>     WHERE i.homebranch in ('CPL','MPL')
>     GROUP BY i.homebranch /* saved_sql.id: 16 */
> 
> And KTD returns
> 
> SELECT
>     i.homebranch,
>     count(*)
>     FROM items i
>     WHERE i.homebranch in 'CPL\r\nMPL'
>     GROUP BY i.homebranch /* saved_sql.id: 1 */
> 
> I'm not used to KTD yet, so I don't know what causes the difference. But it
> definitely works on my local Koha.

I saw that it was the parentheses. My local Koha adds parentheses, but KTD does not.
(In reply to Caroline Cyr La Rose from comment #18)
> I just tested it again on KTD and it gives me a weird error.
> 
> For the same report, my local installation returns
> 
> SELECT
>     i.homebranch,
>     count(*)
>     FROM items i
>     WHERE i.homebranch in ('CPL','MPL')
>     GROUP BY i.homebranch /* saved_sql.id: 16 */
> 
> And KTD returns
> 
> SELECT
>     i.homebranch,
>     count(*)
>     FROM items i
>     WHERE i.homebranch in 'CPL\r\nMPL'
>     GROUP BY i.homebranch /* saved_sql.id: 1 */
> 
> I'm not used to KTD yet, so I don't know what causes the difference. But it
> definitely works on my local Koha.

Nevermind, I restarted KTD and it's fine! Sorry about that!
Comment 20 Brendan Lawlor 2024-02-20 18:03:52 UTC
Thanks for testing Caroline!

Originally I envisioned that you would write it with parentheses something like:
WHERE i.itemtype IN (<<Select item types|itemtypes:in>>)

but it piggybacks on the code that implemented the list run time parameter so you have to write it the same way without parentheses
Comment 21 Brendan Lawlor 2024-03-25 19:30:19 UTC
Created attachment 163875 [details] [review]
Bug 35746: add multiselect to report dropdowns

Rebased for changes to guided_reports_start.tt

This patch adds the ability to use multi select in report dropdowns.

It uses syntax similar to using a list and when using the :all option
WHERE i.homebranch in <<Select libraries|brnaches:in>>

Test plan:
1. Create an SQL report with with new syntax for multi select
    SELECT
    i.homebranch,
    count(*)
    FROM items i
    WHERE i.homebranch in <<Select libraries|branches:in>>
    GROUP BY i.homebranch
2. Save the report
3. Note that you get the error
    The authorized value category (branches:in) you selected does not exist.
4. Apply the patch and repeat steps 1 and 2
5. Note the report saves successfully
6. Run the report
7. Note the select dropdown is now a multiselect
8. Run the report with multiple selections
10. Click show SQL and note that multi select parameters get inserted into the query like:
     WHERE i.homebranch in ('CPL', 'FFL', 'FPL')
11. Test other types of queries with multiple multi selects and lists etc

Sponsored-by: CLAMS
Comment 22 Andrew Fuerste-Henry 2024-03-26 13:22:38 UTC
Created attachment 163920 [details] [review]
Bug 35746: add multiselect to report dropdowns

Rebased for changes to guided_reports_start.tt

This patch adds the ability to use multi select in report dropdowns.

It uses syntax similar to using a list and when using the :all option
WHERE i.homebranch in <<Select libraries|brnaches:in>>

Test plan:
1. Create an SQL report with with new syntax for multi select
    SELECT
    i.homebranch,
    count(*)
    FROM items i
    WHERE i.homebranch in <<Select libraries|branches:in>>
    GROUP BY i.homebranch
2. Save the report
3. Note that you get the error
    The authorized value category (branches:in) you selected does not exist.
4. Apply the patch and repeat steps 1 and 2
5. Note the report saves successfully
6. Run the report
7. Note the select dropdown is now a multiselect
8. Run the report with multiple selections
10. Click show SQL and note that multi select parameters get inserted into the query like:
     WHERE i.homebranch in ('CPL', 'FFL', 'FPL')
11. Test other types of queries with multiple multi selects and lists etc

Sponsored-by: CLAMS
Signed-off-by: Andrew Fuerste Henry <andrewfh@dubcolib.org>
Comment 23 Nick Clemens (kidclamp) 2024-03-26 13:31:12 UTC
Created attachment 163921 [details] [review]
Bug 35746: add multiselect to report dropdowns

Rebased for changes to guided_reports_start.tt

This patch adds the ability to use multi select in report dropdowns.

It uses syntax similar to using a list and when using the :all option
WHERE i.homebranch in <<Select libraries|brnaches:in>>

Test plan:
1. Create an SQL report with with new syntax for multi select
    SELECT
    i.homebranch,
    count(*)
    FROM items i
    WHERE i.homebranch in <<Select libraries|branches:in>>
    GROUP BY i.homebranch
2. Save the report
3. Note that you get the error
    The authorized value category (branches:in) you selected does not exist.
4. Apply the patch and repeat steps 1 and 2
5. Note the report saves successfully
6. Run the report
7. Note the select dropdown is now a multiselect
8. Run the report with multiple selections
10. Click show SQL and note that multi select parameters get inserted into the query like:
     WHERE i.homebranch in ('CPL', 'FFL', 'FPL')
11. Test other types of queries with multiple multi selects and lists etc

Sponsored-by: CLAMS

Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 24 Nick Clemens (kidclamp) 2024-03-26 13:32:23 UTC
I played around to see if we could do the same thing without JS, but the way we get the params makes it tricky to use the multi-select directly. This works well, is easy to read, excellent work!
Comment 25 Katrin Fischer 2024-04-22 06:39:30 UTC
Nice work! 

We have a small issue with a failing test:

xt/tt_valid.t .................... 1/3 
#   Failed test 'TT syntax: not using TT directive within HTML tag'
#   at xt/tt_valid.t line 91.
# Files list: 
# intranet-tmpl/prog/en/modules/reports/guided_reports_start.tt: 879
# Looks like you failed 1 test of 3.
xt/tt_valid.t .................... Dubious, test returned 1 (wstat 256, 0x100)

Can you please fix? 

I wonder if we could also add it to the runtime parameters menu somehow for some advertising? (see bug 35856 maybe for reference, also: this is for a separate bug and not a blocker!)
Comment 26 Brendan Lawlor 2024-04-22 12:39:22 UTC
Created attachment 165276 [details] [review]
Bug 35746: (follow-up) Fix invalid TT syntax inside html tag
Comment 27 Brendan Lawlor 2024-04-22 12:48:37 UTC
Thanks for pointing out where to run that test. I had only ran the qa test before.

I fixed the invalid TT syntax inside the html tag so it now passes:
$ ./xt/tt_valid.t
1..3
ok 1 - TT syntax: not using TT directive within HTML tag
ok 2 - <body> tag with id and class attributes
ok 3 - Token received same as original put on stack

Bug 35856 includes a hint for this in the runtime parameter modal too. Should I change the status of that bug from Blocked back to Signed off?

Thanks Katrin!
Comment 28 Lucas Gass 2024-04-22 17:05:18 UTC
This changes the :all option into from a single select dropdown to a multi one. I don't think that is necessarily bad, could be handy actually!
Comment 29 Lucas Gass 2024-04-22 17:06:14 UTC
Created attachment 165333 [details] [review]
Bug 35746: add multiselect to report dropdowns

Rebased for changes to guided_reports_start.tt

This patch adds the ability to use multi select in report dropdowns.

It uses syntax similar to using a list and when using the :all option
WHERE i.homebranch in <<Select libraries|brnaches:in>>

Test plan:
1. Create an SQL report with with new syntax for multi select
    SELECT
    i.homebranch,
    count(*)
    FROM items i
    WHERE i.homebranch in <<Select libraries|branches:in>>
    GROUP BY i.homebranch
2. Save the report
3. Note that you get the error
    The authorized value category (branches:in) you selected does not exist.
4. Apply the patch and repeat steps 1 and 2
5. Note the report saves successfully
6. Run the report
7. Note the select dropdown is now a multiselect
8. Run the report with multiple selections
10. Click show SQL and note that multi select parameters get inserted into the query like:
     WHERE i.homebranch in ('CPL', 'FFL', 'FPL')
11. Test other types of queries with multiple multi selects and lists etc

Sponsored-by: CLAMS

Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Comment 30 Lucas Gass 2024-04-22 17:06:16 UTC
Created attachment 165334 [details] [review]
Bug 35746: (follow-up) Fix invalid TT syntax inside html tag

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Comment 31 Brendan Lawlor 2024-04-22 18:46:04 UTC
Created attachment 165345 [details] [review]
Bug 35746: (follow-up) Fix invalid tt in html tag
Comment 32 Brendan Lawlor 2024-04-22 19:04:19 UTC
(In reply to Lucas Gass from comment #28)
> This changes the :all option into from a single select dropdown to a multi
> one. I don't think that is necessarily bad, could be handy actually!

Good catch Lucas!

I think that this should not happen because it would change how current reports work. There could be cases where you only want a parameter to be single choice or the all option but not multi select.

The original patch relied on the value of the template variable always being put in the html tag as either the attribute 'multiple' or as an empty string for single selects. My understanding is that causes a problem with translations.

I changed one conditional so that only selects that should be multi will get the multiple attribute and it doesn't have invalid TT inside the select tag so it passes the test too.
Comment 33 Katrin Fischer 2024-04-23 06:14:48 UTC
Thx for the follow-up! I'll be back asap.
Comment 34 Katrin Fischer 2024-04-26 14:48:21 UTC
I am sorry, I have trouble with the second patch:

git bz apply 35746

Bug 35746 - Multiple selections for parameters used in the IN function

165333 - Bug 35746: add multiselect to report dropdowns
165345 - Bug 35746: (follow-up) Fix invalid tt in html tag

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 35746: add multiselect to report dropdowns
Applying: Bug 35746: (follow-up) Fix invalid tt in html tag
error: sha1 information is lacking or useless (koha-tmpl/intranet-tmpl/prog/en/modules/reports/guided_reports_start.tt).
error: could not build fake ancestor
Patch failed at 0001 Bug 35746: (follow-up) Fix invalid tt in html tag


Can you please rebase and re-attach both of those patches? If you need help please let us know!
Comment 35 Brendan Lawlor 2024-04-26 15:11:33 UTC
Created attachment 165659 [details] [review]
Bug 35746: add multiselect to report dropdowns

Rebased for changes to guided_reports_start.tt

This patch adds the ability to use multi select in report dropdowns.

It uses syntax similar to using a list and when using the :all option
WHERE i.homebranch in <<Select libraries|brnaches:in>>

Test plan:
1. Create an SQL report with with new syntax for multi select
    SELECT
    i.homebranch,
    count(*)
    FROM items i
    WHERE i.homebranch in <<Select libraries|branches:in>>
    GROUP BY i.homebranch
2. Save the report
3. Note that you get the error
    The authorized value category (branches:in) you selected does not exist.
4. Apply the patch and repeat steps 1 and 2
5. Note the report saves successfully
6. Run the report
7. Note the select dropdown is now a multiselect
8. Run the report with multiple selections
10. Click show SQL and note that multi select parameters get inserted into the query like:
     WHERE i.homebranch in ('CPL', 'FFL', 'FPL')
11. Test other types of queries with multiple multi selects and lists etc

Sponsored-by: CLAMS

Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Comment 36 Brendan Lawlor 2024-04-26 15:11:36 UTC
Created attachment 165660 [details] [review]
Bug 35746: (follow-up) Fix invalid tt in html tag
Comment 37 Brendan Lawlor 2024-04-26 15:14:55 UTC
(In reply to Katrin Fischer from comment #34)
> Can you please rebase and re-attach both of those patches? If you need help
> please let us know!

I think maybe the problem was that I submitted the follow up by itself. Hopefully I did it the right way this time.

Thanks!
Comment 38 Katrin Fischer 2024-04-26 15:57:32 UTC
Applies now!
Comment 39 Katrin Fischer 2024-04-26 16:07:11 UTC
Pushed for 24.05!

Well done everyone, thank you!
Comment 40 Fridolin Somers 2024-05-24 08:51:25 UTC
Not backported to 23.11.x