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.
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.
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>
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.
Created attachment 93448 [details] [review] Bug 22773: (follow-up) Fix capitalization
Created attachment 93660 [details] [review] Bug 22773: Message appearing if any invoice selected
(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.
Created attachment 94411 [details] [review] Bug 22773: (follow-up) Recommended changes are made on datatable and butons
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)"
(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?
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).
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.
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.
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.
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>
Created attachment 96907 [details] [review] Bug 22773: (follow-up) Fix capitalization Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
Created attachment 96908 [details] [review] Bug 22773: Message appearing if any invoice selected Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
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>
Created attachment 96910 [details] [review] Bug 22773: The deprecated plugin is removed Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
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)
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>
Created attachment 98730 [details] [review] Bug 22773: (follow-up) Fix capitalization Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
Created attachment 98731 [details] [review] Bug 22773: Message appearing if any invoice selected Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
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>
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>
(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.
I've tested merging invoices from 2 different tabs and it worked nicely. I did 2 follow-ups for some cosmetics (the label on the subscription limit and the buttons). Ready to PQA.
Created attachment 106538 [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> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 106539 [details] [review] Bug 22773: (follow-up) Fix capitalization Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 106540 [details] [review] Bug 22773: Message appearing if any invoice selected Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 106541 [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> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 106542 [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> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 106543 [details] [review] Bug 22773: (QA follow-up) Improve existing merge button and fix checkbox label Fixes the checkbox label by adding an id, so that when you click on "Show only subscriptons" the checkbox will be checked. Adds standard classes to the existing "Merge selected invoices" button to make things more consistent. Also added the fa-compress icon that we use in cataloguing on the merge records button. Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 106544 [details] [review] Bug 22773: (QA follow-up) Fix missing filters Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
3 smalls things: 1. Please squash Bug 22773: Bulk Close invoices and Filter invoice view (open/closed) and Bug 22773: The deprecated plugin is removed to avoid dataTables.fnFilterAll.js to be tracked (we never used it) 2. Buttons are not aligned/styled correctly: https://snipboard.io/wyac78.jpg 3. We should not send the checkboxes from the other tabs. To recreate: - Have 1+ closed and 1+ opened invoice - Make sure the closedate is != from today - Don't tick anything - Click "Please select at least one invoice." => You get the JS alert OK - Tick the closed invoices and click the "Close selected invoices" from the "Open invoices" tab => You don't get the JS alert KO => the closedate is not today KO
Created attachment 115420 [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> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Bug 22773: The deprecated plugin is removed Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com> Signed-off-by: Christopher Kellermeyer <ckellermeyer@altadenalibrary.org> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Remove asset for removed js
Created attachment 115421 [details] [review] Bug 22773: (follow-up) Fix capitalization Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 115422 [details] [review] Bug 22773: Message appearing if any invoice selected Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 115423 [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> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 115424 [details] [review] Bug 22773: (QA follow-up) Improve existing merge button and fix checkbox label Fixes the checkbox label by adding an id, so that when you click on "Show only subscriptons" the checkbox will be checked. Adds standard classes to the existing "Merge selected invoices" button to make things more consistent. Also added the fa-compress icon that we use in cataloguing on the merge records button. Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 115425 [details] [review] Bug 22773: (QA follow-up) Fix missing filters Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 115426 [details] [review] Bug 22773: (follow-up) Ensure buttons only affect their related table Also cleans up a few rebase issues
(In reply to Jonathan Druart from comment #34) > 1. Please squash to avoid dataTables.fnFilterAll.js to be tracked (we never used it) Done > 2. Buttons are not aligned/styled correctly: https://snipboard.io/wyac78.jpg yarn build -the margin on merge button is removed in these patches and fixes alignment > 3. We should not send the checkboxes from the other tabs. Fixed
Created attachment 116010 [details] [review] Bug 22773: Assign different id to checkboxes Otherwise clicking the label does not work
We would have certainly saved time, lines of code, readability and maintainability with only one table and correct filters. [un]select all links are missing, not considering blocker however.
Sorry, found a bug. Merge is broken, there are elements sharing the same id (in this case #merge). To recreate, have 2 open invoices, select them, click merge You get "You must select at least two invoices to merge."...
Created attachment 119282 [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> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Bug 22773: The deprecated plugin is removed Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com> Signed-off-by: Christopher Kellermeyer <ckellermeyer@altadenalibrary.org> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Remove asset for removed js
Created attachment 119283 [details] [review] Bug 22773: (follow-up) Fix capitalization Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 119284 [details] [review] Bug 22773: Message appearing if any invoice selected Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 119285 [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> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Rebase add comma
Created attachment 119286 [details] [review] Bug 22773: (QA follow-up) Improve existing merge button and fix checkbox label Fixes the checkbox label by adding an id, so that when you click on "Show only subscriptons" the checkbox will be checked. Adds standard classes to the existing "Merge selected invoices" button to make things more consistent. Also added the fa-compress icon that we use in cataloguing on the merge records button. Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 119287 [details] [review] Bug 22773: (QA follow-up) Fix missing filters Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 119288 [details] [review] Bug 22773: (follow-up) Ensure buttons only affect their related table Also cleans up a few rebase issues
Created attachment 119289 [details] [review] Bug 22773: Assign different id to checkboxes Otherwise clicking the label does not work
Created attachment 119290 [details] [review] Bug 22773: (follow-up) Fix id clash and data element
Pushed to master for 21.05, thanks to everybody involved!
Enhancement not pushed to 20.11.x
(In reply to Jonathan Druart from comment #44) > We would have certainly saved time, lines of code, readability and > maintainability with only one table and correct filters. > > [un]select all links are missing, not considering blocker however. We are very happy with this enhancement! It would be even greater if there would be "(un)select all" links. Not sure if I have to file a new bug for this?
(In reply to Mirjam Vantieghem from comment #57) > (In reply to Jonathan Druart from comment #44) > > We would have certainly saved time, lines of code, readability and > > maintainability with only one table and correct filters. > > > > [un]select all links are missing, not considering blocker however. > > We are very happy with this enhancement! > It would be even greater if there would be "(un)select all" links. > Not sure if I have to file a new bug for this? Hi Mirjam, once a feature/bug has been published in a release, we always need a new bug for any fixes or further improvements. Please file one! :)
Thanks for your reply, Katrin. Someone has already done this: bug 30438. I saw this only after I placed my comment ...
(In reply to Mirjam Vantieghem from comment #59) > Thanks for your reply, Katrin. Someone has already done this: bug 30438. > I saw this only after I placed my comment ... Great :)