Bug 22773 - Bulk Close invoices and Filter invoice view (open/closed)
Summary: Bulk Close invoices and Filter invoice view (open/closed)
Status: Signed Off
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Devinim
QA Contact: Testopia
URL:
Keywords: rel_20_11_target
Depends on:
Blocks:
 
Reported: 2019-04-25 15:24 UTC by Joy Nelson
Modified: 2020-04-15 16:21 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 22773: Bulk Close invoices and Filter invoice view (open/closed) (9.94 KB, patch)
2019-10-01 09:54 UTC, Devinim
Details | Diff | Splinter Review
Bug 22773: Bulk Close invoices and Filter invoice view (open/closed) (9.99 KB, patch)
2019-10-01 14:23 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 22773: (follow-up) Fix capitalization (4.25 KB, patch)
2019-10-02 12:39 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 22773: Message appearing if any invoice selected (3.90 KB, patch)
2019-10-03 16:01 UTC, Devinim
Details | Diff | Splinter Review
Bug 22773: (follow-up) Recommended changes are made on datatable and butons (4.54 KB, patch)
2019-10-18 13:54 UTC, Devinim
Details | Diff | Splinter Review
Bug 22773: The deprecated plugin is removed (4.13 KB, patch)
2019-11-12 10:03 UTC, Devinim
Details | Diff | Splinter Review
Bug 22773: Bulk Close invoices and Filter invoice view (open/closed) (10.05 KB, patch)
2020-01-07 15:39 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 22773: (follow-up) Fix capitalization (4.31 KB, patch)
2020-01-07 15:39 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 22773: Message appearing if any invoice selected (3.96 KB, patch)
2020-01-07 15:40 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 22773: (follow-up) Recommended changes are made on datatable and butons (4.60 KB, patch)
2020-01-07 15:40 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 22773: The deprecated plugin is removed (4.19 KB, patch)
2020-01-07 15:40 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 22773: Bulk Close invoices and Filter invoice view (open/closed) (10.05 KB, patch)
2020-02-11 22:28 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 22773: (follow-up) Fix capitalization (4.31 KB, patch)
2020-02-11 22:28 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 22773: Message appearing if any invoice selected (3.96 KB, patch)
2020-02-11 22:29 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 22773: (follow-up) Recommended changes are made on datatable and butons (4.67 KB, patch)
2020-02-11 22:29 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 22773: The deprecated plugin is removed (4.27 KB, patch)
2020-02-11 22:29 UTC, ByWater Sandboxes
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Joy Nelson 2019-04-25 15:24:28 UTC
The invoice screen shows all invoices and is not filtered by open and closed.  We propose to:
1. Create a closed tab on the invoices view.

2. Move all closed invoices to a closed tab on the invoices view.  All open invoices would remain in the open tab until closed. 
This would provide the filtering needed and clean up the display for users.

3. Allow users to select multiple invoices on the Open tab and then provide a button on the screen to 'close selected invoices' in bulk.
Comment 1 Devinim 2019-10-01 09:54:28 UTC
Created attachment 93357 [details] [review]
Bug 22773: Bulk Close invoices and Filter invoice view (open/closed)

Test plan:
1)Verify that close tab occurse for closed invoices.
2)Try to select bulk invoices in open invoices tab and press button "Close selected invoices". Verify that your selected invoices moved to close invoices tab. 
3)Try to select bulk invoices in close invoices tab and press button "Reopen selected invoices". Verify that your selected invoices moved to open invoices tab.
4)Check also when you check on "Show only subscriptions" checkbox you may see subscribed invoices in all tabs.
Comment 2 ByWater Sandboxes 2019-10-01 14:23:59 UTC
Created attachment 93375 [details] [review]
Bug 22773: Bulk Close invoices and Filter invoice view (open/closed)

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Comment 3 Katrin Fischer 2019-10-02 12:29:34 UTC
This works really well, but can please do a minor follow-up?

- Capitalization on the tab names and the new button is wrong: only the first word should begin with a capital letter (attaching a follow-up for this)
- The buttons don't align - we are missing: margin: .5em 0 0; on the second button. They should also be using: btn btn-default classes instead of submit.
- The alignment on the 'closed inovices' table seems a bit off. It's much smaller than my other table in testing and aligns 'center'. The data is almost indentical, so I think it's not that.
- It would be nice if there is a message when you don't check anything and click on the button (we do that in other places, like when selecting records for merging). (not blocker)

Please keep in touch - labelling this as 19.11 candidate for now.
Comment 4 Katrin Fischer 2019-10-02 12:39:51 UTC
Created attachment 93448 [details] [review]
Bug 22773: (follow-up) Fix capitalization
Comment 5 Devinim 2019-10-03 16:01:37 UTC
Created attachment 93660 [details] [review]
Bug 22773: Message appearing if any invoice selected
Comment 6 Owen Leonard 2019-10-03 18:36:21 UTC
(In reply to Katrin Fischer from comment #3)

> - The buttons don't align - we are missing: margin: .5em 0 0; on the second
> button.

Actually the right fix would be to remove the lin in staff-global.scss for "#merge" (currently line 3488).

> They should also be using: btn btn-default classes instead of submit.

This still needs to be fixed.

> - The alignment on the 'closed inovices' table seems a bit off. It's much
> smaller than my other table in testing and aligns 'center'.

I suggest adding "autoWidth: false" to the DataTable configuration.

By the way, I mentioned in conversation with Katrin on IRC that the dataTables.fnFilterAll.js should be unnecessary. The source of the plugin itself says it is deprecated:

" * DataTables 1.10+ provides this ability through its new API, which is able to
 * to control multiple tables at a time.
 * `$('.dataTable').DataTable().search( ... )` for example will apply the same
 * filter to all tables on the page. The new API should be used in preference
 * to this older method if at all possible."

However I wasn't able to get this method to work on both tables, so it appears the plugin is required for now.
Comment 7 Devinim 2019-10-18 13:54:22 UTC
Created attachment 94411 [details] [review]
Bug 22773: (follow-up) Recommended changes are made on datatable and butons
Comment 8 Jonathan Druart 2019-10-22 11:13:30 UTC
1. Not sure the tabs are the best choice here. We could have had 2 links to filter by open/close invoices. It would have made the code easier :)
Not blocker.

2. Also not sure why we need this plugin. Cannot we retrieve the instances of DataTable and filter them individually?

3. Should not we update the number of invoices displayed in each tab?
"Open invoices (X)"
Comment 9 Katrin Fischer 2019-10-28 07:07:41 UTC
(In reply to Jonathan Druart from comment #8)
> 1. Not sure the tabs are the best choice here. We could have had 2 links to
> filter by open/close invoices. It would have made the code easier :)
> Not blocker.
> 
> 2. Also not sure why we need this plugin. Cannot we retrieve the instances
> of DataTable and filter them individually?
> 
> 3. Should not we update the number of invoices displayed in each tab?
> "Open invoices (X)"

About 1) - we have something very similar for the closed and open subscriptions. Both are searched, but displayed in different tabs, so the pattern already exists. Not sure how it works, but maybe worth taking a look?
Comment 10 Katrin Fischer 2019-11-06 10:22:00 UTC
Hi all,

as there are some concerns about the datatables plugin used, could it be an option to see how it was solved for the subscription (separate tabs for active and closed subscriptions) and if this could be a way to avoid the plugin?

If it turns out that we can't do without the plugin for now, we could still maybe add it with a FIXME.

Would that be agreeable?

Then this leaves updating the number displayed on the bug (comment#8).
Comment 11 Martin Renvoize 2019-11-06 10:41:51 UTC
I don't mind whether we use tabs or a single table and filter.. there's precedence for both.

I really don't like the idea of introducing a new dependency that is already officially deprecated, so if there's any way to work around that then I'd be onboard with it.

Hope that helps guide this.. it's a nice feature and would be good to get moving again.
Comment 12 Devinim 2019-11-12 10:03:41 UTC
Created attachment 95290 [details] [review]
Bug 22773: The deprecated plugin is removed

We used the deprecated plugin (fnFilterAll) to benefit "Show only subscriptions" checkbox property.
In this new patch we removed fnFilterAll plugin and we put "Show only subscriptions" checkbox in each tab to be able to show checked subscriptions.
Comment 13 Jonathan Druart 2019-12-05 10:44:36 UTC
It's not clear with 2 tabs. Sorry to insist but:

1. Do not select anything, click "close selected invoices"
=> You get a warning "nothing selected"
Now tick an open invoice, and click "open selected invoices"
=> You do not get the warning

2. Click "Show only subscriptions" label does not select the checkbox. Missing id.

3. You can select subscriptions from the 2 tabs then click "Merge selected invoices". It's expected but as the button is inside the tab (so 2 buttons) it's not obvious.

Not blocker as other QAers agreed with the tab, but insufficient for me to PQA.
Comment 14 ByWater Sandboxes 2020-01-07 15:39:56 UTC
Created attachment 96906 [details] [review]
Bug 22773: Bulk Close invoices and Filter invoice view (open/closed)

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
Comment 15 ByWater Sandboxes 2020-01-07 15:39:59 UTC
Created attachment 96907 [details] [review]
Bug 22773: (follow-up) Fix capitalization

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
Comment 16 ByWater Sandboxes 2020-01-07 15:40:01 UTC
Created attachment 96908 [details] [review]
Bug 22773: Message appearing if any invoice selected

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
Comment 17 ByWater Sandboxes 2020-01-07 15:40:04 UTC
Created attachment 96909 [details] [review]
Bug 22773: (follow-up) Recommended changes are made on datatable and butons

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
Comment 18 ByWater Sandboxes 2020-01-07 15:40:06 UTC
Created attachment 96910 [details] [review]
Bug 22773: The deprecated plugin is removed

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
Comment 19 Kelly McElligott 2020-01-07 15:44:05 UTC
I am signing off on this bug.  To comment on Jonathan's comments.



2. Click "Show only subscriptions" label does not select the checkbox. Missing id.

I created a subscription order and this worked as the other orders did.  If there are no subscriptions then it produces no results which will not let you move on. 


3. You can select subscriptions from the 2 tabs then click "Merge selected invoices". It's expected but as the button is inside the tab (so 2 buttons) it's not obvious.

I am not sure what this means?  I did merge two invoices and it worked correctly.  If anything the merge button is not as pronounced (no black outline)
Comment 20 ByWater Sandboxes 2020-02-11 22:28:54 UTC
Created attachment 98729 [details] [review]
Bug 22773: Bulk Close invoices and Filter invoice view (open/closed)

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
Comment 21 ByWater Sandboxes 2020-02-11 22:28:57 UTC
Created attachment 98730 [details] [review]
Bug 22773: (follow-up) Fix capitalization

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
Comment 22 ByWater Sandboxes 2020-02-11 22:29:00 UTC
Created attachment 98731 [details] [review]
Bug 22773: Message appearing if any invoice selected

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
Comment 23 ByWater Sandboxes 2020-02-11 22:29:02 UTC
Created attachment 98732 [details] [review]
Bug 22773: (follow-up) Recommended changes are made on datatable and butons

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>

Signed-off-by: Christopher Kellermeyer <ckellermeyer@altadenalibrary.org>
Comment 24 ByWater Sandboxes 2020-02-11 22:29:05 UTC
Created attachment 98733 [details] [review]
Bug 22773: The deprecated plugin is removed

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>

Signed-off-by: Christopher Kellermeyer <ckellermeyer@altadenalibrary.org>
Comment 25 Jonathan Druart 2020-02-12 14:03:54 UTC
(In reply to Kelly McElligott from comment #19)
> I am signing off on this bug.  To comment on Jonathan's comments.
> 
> 
> 
> 2. Click "Show only subscriptions" label does not select the checkbox.
> Missing id.
> 
> I created a subscription order and this worked as the other orders did.  If
> there are no subscriptions then it produces no results which will not let
> you move on. 

The "missing label" thing means that when you click on the label of the checkbox, the checkbox does not check/uncheck.

> 3. You can select subscriptions from the 2 tabs then click "Merge selected
> invoices". It's expected but as the button is inside the tab (so 2 buttons)
> it's not obvious.
> 
> I am not sure what this means?  I did merge two invoices and it worked
> correctly.  If anything the merge button is not as pronounced (no black
> outline)

The screen is: https://snipboard.io/yjkFzV.jpg

The first question is: Are we suppose to merge open and close invoices?

The "Merge selected invoices" button is inside the "open invoices" fieldset/block.
However, if you select invoices from the "open invoices" and the "closed invoices", the merge button will work. IMO it's ergonomically wrong.