+1 - We have multiple reports where this would be really useful.

+1 this would be extremely helpful!

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

Created attachment 160880 [details] [review]
Bug 35746: add multiselect to report dropdowns

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.

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')

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?

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.

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

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

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>

*** Bug 16002 has been marked as a duplicate of this bug. ***

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

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.

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>

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?

(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

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.

(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!

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

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

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>

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>

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!

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!)

Created attachment 165276 [details] [review]
Bug 35746: (follow-up) Fix invalid TT syntax inside html tag

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!

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!

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>

Created attachment 165334 [details] [review]
Bug 35746: (follow-up) Fix invalid TT syntax inside html tag

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>

Created attachment 165345 [details] [review]
Bug 35746: (follow-up) Fix invalid tt in html tag

(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.

Thx for the follow-up! I'll be back asap.

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!

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>

Created attachment 165660 [details] [review]
Bug 35746: (follow-up) Fix invalid tt in html tag

(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!

Applies now!

Pushed for 24.05!

Well done everyone, thank you!

Not backported to 23.11.x