The ISSN search field is already in master.
This patch depends on Bug 5343 (DB field aqorders.subscriptionid).

Patch does not apply.  Please rebase and resubmit.

The patch doesn't apply cleanly against master.
There is a merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/acqui/invoices.tt.
So I don't sign off the match, and I pass it to "failed QA".

The applying (against master 031) of the patch is OK.
I've created some invoices for subscriptions' orders and some invoices not.
When the checkbox "Show only subscriptions" is not checked, the patch works well and all the invoices are shown in the invoices table.
On the contrary, when the checkbox "Show only subscriptions" is checked, no invoice is shown in the invoices table.
Is it OK?
This is the part of the patch about selecting only subscription' invoices:
resultst.fnFilter( "[0-9]+", 0, true )
Can I create the situation with "[0-9+]" and see the list of the subscriptions' invoices shown? Thanks.
When I see an invoice (from column "Invoice no." of the invoices table) and the invoice is not a subscription' one, I see "Invoice details" with a following table full of dates ("Summary" "Publisher" etc).
When I see an invoice (from column "Invoice no." of the invoices table) and the invoice is a subscription' one, I see "Invoice details" followed by "No orders yet". On the contrary the order is present in bd: it is listed by Orders-search' results table of Acquisitions, and is linked to the subscripted serial.

(In reply to Paola Rossi from comment #7)
Paola, I cannot reproduce your problem but I found a typo in the patch. Could your retest please?
The regex should match the subscription id in the first column of the table. Datatables uses it for filtering on subscriptions.

Rebasing invoices.tt is neeeded.
Against master 3.13.00.032:
Applying: Bug 7288: Allow to display only subscriptions in the invoices table
Using index info to reconstruct a base tree...
Falling back to patching base and 3-way merge...
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/acqui/invoices.tt
CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/acqui/invoices.tt
Auto-merging C4/Acquisition.pm
Failed to merge in the changes.
Patch failed at 0001 Bug 7288: Allow to display only subscriptions in the invoices table

So I pass the patch to failed QA status.

Created attachment 23815 [details] [review]
[PASSED QA] Bug 7288: Allow to display only subscriptions in the invoices table

This patch adds a checkbox "Show only subscriptions" in the invoices
table.
If this checkbox is checked, a filter is added on the table in order to
show only subscription invoices

Signed-off-by: Paola Rossi <paola.rossi@cineca.it>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
To test:
- Test in a database with multiple existing invoices
- Create an order from a subscription, close basket, receive
- Test that the result table of the invoice search shows
  the new checkbox and that it works correctly

Passes all tests and QA script.
Also passes t/db_dependent/Acquisition.t and t/db_dependent/Acquisition/*.

Comment on attachment 23815 [details] [review]
[PASSED QA] Bug 7288: Allow to display only subscriptions in the invoices table

Review of attachment 23815 [details] [review]:
-----------------------------------------------------------------

::: C4/Acquisition.pm
@@ +2504,4 @@
>      my $dbh = C4::Context->dbh;
>      my $query = qq{
>          SELECT aqinvoices.*, aqbooksellers.name AS suppliername,
> +          aqorders.subscriptionid,

This does not handle the case where a single invoice has more than one order with a subscription very well.  In particular, only one subscription would (arbitrarily) be displayed.

If the goal is to be able to link to all subscriptions on a given invoice, it's not sufficient to display just one.  If the goal is more simply to just be able to filter the invoice search for invoices that have one or more subscriptions, then it would be better calculate a Boolean flag.

Either way, as the patch stands now it is likely to cause confusion for libraries that have invoices that cover multiple subscriptions -- which is very, very common in the US, at least.

Failing QA.  When you resubmit, I would also appreciate if you wrote a unit test for the change in the core routine.

Created attachment 26065 [details] [review]
Bug 7288: Set a boolean if the invoice if linked to subscriptions

If an invoice is linked to subscription, we need to set a boolean to
true in order to filter them in the interface.

(In reply to Galen Charlton from comment #14)
> Failing QA.  When you resubmit, I would also appreciate if you wrote a unit
> test for the change in the core routine.

It is quite difficult to provide good unit tests here. To test if an invoice is linked to subscription, we should create a subscription (so a budget, a frequency, etc.).

Sorry Galen,

do we need to write an extra unit test or the code is OK to be pushed in master ?

Zeno Tajoli

Pushed to master, along with a follow-up that adds a direct unit test.

Thanks, Jonathan!

I choose to not apply to 3.14.x so I set as resolved.