Description
Martin Renvoize (ashimema)
2020-08-21 13:59:43 UTC
This sounds like a great idea. My instinct would be to also have an index - perhaps where each linked summary is labelled with the cash up date? A bit like this: 05-01-2021 || Link to summary 01-01-2021 || Link to summary 22-12-2020 || Link to summary 15-12-2020 || Link to summary 08-12-2020 || Link to summary 01-12-2020 || Link to summary 29-11-2020 || Link to summary 26-11-2020 || Link to summary 13-11-2020 || Link to summary 04-11-2020 || Link to summary We tend to seek out previous cash ups when there's a problem, so being able to quickly view an index of that till's cash ups to find the correct session would be great. If we had 2-3 cash ups a week and we wanted to go back 6 months, just prev/next controls would be a little frustrating. Or, if it's not too much work, a data table would do the job - then we could always use the search / sort controls? *** Bug 26676 has been marked as a duplicate of this bug. *** Katrin Fischer 2020-10-14 12:34:55 BST At the moment when we do multiple cashups, there is no overview page to see the past ones. We can only see a bit of information on the last cash-ups. It would be nice to have a way to list cashups for a timeframe for audit and other purposes. Created attachment 116176 [details] [review] Bug 26273: Add historic cashups table to register page This patch adds a historic cashups table to the bottom of the register transaction details page. It will only display if one or more cashups have taken place and contains links to allow display of the summary detail modal for each cashup. Test plan 1/ Enable UseCashRegisters 2/ Undertake a few transactions and cashups 3/ View the 'Register details' page 4/ Note the appearance of a 'Cashup history' section at the bottom of the page 5/ Note that 'Summary' button displays the relevant modal 6/ Signoff I'm looking for feedback on this one now.. I'm not entirely sure about the placement of the index.. whether the register details page is the right place or whether this warrants a page of it's own (but then.. where should it be linked from and what should it be called?) The attached patch is a proof of concept for how we may achieve it however :) Created attachment 116640 [details] [review] Bug 26273: Add historic cashups table to register page This patch adds a historic cashups table to the bottom of the register transaction details page. It will only display if one or more cashups have taken place and contains links to allow display of the summary detail modal for each cashup. Test plan 1/ Enable UseCashRegisters 2/ Undertake a few transactions and cashups 3/ View the 'Register details' page 4/ Note the appearance of a 'Cashup history' section at the bottom of the page 5/ Note that 'Summary' button displays the relevant modal 6/ Signoff Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk> The sign off is under my name, but both Daniel and I have been testing with it. We both really like this. We like the placement of the summaries, as we think it's easy for the staff to find and easy to use. We particularly like that the filters for the datatable show automatically, and that you can sort/search by takings as well as date. The date formatting is slightly unusual with the +00.00 at the end: e.g. 2021-02-10T10:04:56+00:00 But that's not a blocker for us - it's very useful to see the time and to be able to sort by it, in case the staff cash up multiple times in one day. Created attachment 116685 [details] [review] Bug 26273: (follow-up) Format dates and money correctly. This patch adds the missing formatting for the Datetime and Amount fields. (In reply to Sally from comment #7) > The sign off is under my name, but both Daniel and I have been testing with > it. We both really like this. > > We like the placement of the summaries, as we think it's easy for the staff > to find and easy to use. > > We particularly like that the filters for the datatable show automatically, > and that you can sort/search by takings as well as date. > > The date formatting is slightly unusual with the +00.00 at the end: e.g. > 2021-02-10T10:04:56+00:00 > > But that's not a blocker for us - it's very useful to see the time and to be > able to sort by it, in case the staff cash up multiple times in one day. Thanks for testing, and good catch on the missing formatting.. I've added a followup not to take care of both the datetime formatting and the cashup amounts. I meant to ask.. I could probably get at the staff user name for who carried out the cashup.. would that be helpful in the index table at all too? Created attachment 116691 [details] [review] Bug 26273: Add historic cashups table to register page This patch adds a historic cashups table to the bottom of the register transaction details page. It will only display if one or more cashups have taken place and contains links to allow display of the summary detail modal for each cashup. Test plan 1/ Enable UseCashRegisters 2/ Undertake a few transactions and cashups 3/ View the 'Register details' page 4/ Note the appearance of a 'Cashup history' section at the bottom of the page 5/ Note that 'Summary' button displays the relevant modal 6/ Signoff Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk> Signed-off-by: Daniel Jones <daniel.jones@cheshiresharedservices.gov.uk> Created attachment 116692 [details] [review] Bug 26273: (follow-up) Format dates and money correctly. This patch adds the missing formatting for the Datetime and Amount fields. (In reply to Martin Renvoize from comment #10) > I meant to ask.. I could probably get at the staff user name for who carried > out the cashup.. would that be helpful in the index table at all too? If it's not too much work, that'd be great! (Or even just the borrowernumber.) Created attachment 116697 [details] [review] Bug 26273: Add historic cashups table to register page This patch adds a historic cashups table to the bottom of the register transaction details page. It will only display if one or more cashups have taken place and contains links to allow display of the summary detail modal for each cashup. Test plan 1/ Enable UseCashRegisters 2/ Undertake a few transactions and cashups 3/ View the 'Register details' page 4/ Note the appearance of a 'Cashup history' section at the bottom of the page 5/ Note that 'Summary' button displays the relevant modal 6/ Signoff Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk> Signed-off-by: Daniel Jones <daniel.jones@cheshiresharedservices.gov.uk> Created attachment 116698 [details] [review] Bug 26273: (follow-up) Format dates and money correctly. This patch adds the missing formatting for the Datetime and Amount fields. Test plan 1/ Apply patch and reload register page to re-display the cashups table 2/ Note that the 'Date' field now displays a nicely formatted date without an offset. 3/ Note that the 'Amount' field now displays the number according to your money formatting preferences (with 2 decimal places in most cases) 4/ Signoff Created attachment 116699 [details] [review] Bug 26273: (follow-up) Add 'Cashier' to the cashups table This patch adds the 'Cashier' field as discussed on the bug report. Test plan 1/ Apply patch and reload the register page to refresh the table 2/ Note that a 'Cashier' field now appears in the table 3/ Sorting and searching on 'Cashier' should work as expected 4/ Default sorting should remain as 'Date' order. 5/ Signoff Created attachment 116710 [details] [review] Bug 26273: (follow-up) Add 'Cashier' to the cashups table This patch adds the 'Cashier' field as discussed on the bug report. Test plan 1/ Apply patch and reload the register page to refresh the table 2/ Note that a 'Cashier' field now appears in the table 3/ Sorting and searching on 'Cashier' should work as expected 4/ Default sorting should remain as 'Date' order. 5/ Signoff Created attachment 116771 [details] [review] Bug 26273: Add historic cashups table to register page This patch adds a historic cashups table to the bottom of the register transaction details page. It will only display if one or more cashups have taken place and contains links to allow display of the summary detail modal for each cashup. Test plan 1/ Enable UseCashRegisters 2/ Undertake a few transactions and cashups 3/ View the 'Register details' page 4/ Note the appearance of a 'Cashup history' section at the bottom of the page 5/ Note that 'Summary' button displays the relevant modal 6/ Signoff Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk> Signed-off-by: Daniel Jones <daniel.jones@cheshiresharedservices.gov.uk> Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk> Created attachment 116772 [details] [review] Bug 26273: (follow-up) Format dates and money correctly. This patch adds the missing formatting for the Datetime and Amount fields. Test plan 1/ Apply patch and reload register page to re-display the cashups table 2/ Note that the 'Date' field now displays a nicely formatted date without an offset. 3/ Note that the 'Amount' field now displays the number according to your money formatting preferences (with 2 decimal places in most cases) 4/ Signoff Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk> Created attachment 116773 [details] [review] Bug 26273: (follow-up) Add 'Cashier' to the cashups table This patch adds the 'Cashier' field as discussed on the bug report. Test plan 1/ Apply patch and reload the register page to refresh the table 2/ Note that a 'Cashier' field now appears in the table 3/ Sorting and searching on 'Cashier' should work as expected 4/ Default sorting should remain as 'Date' order. 5/ Signoff Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk> Both patches work as expected - cashier field, and date/time. The dependency patch is not yet signed off - moving to BLOCKED for the moment until that is sorted. Created attachment 117449 [details] [review] Bug 26273: Add historic cashups table to register page This patch adds a historic cashups table to the bottom of the register transaction details page. It will only display if one or more cashups have taken place and contains links to allow display of the summary detail modal for each cashup. Test plan 1/ Enable UseCashRegisters 2/ Undertake a few transactions and cashups 3/ View the 'Register details' page 4/ Note the appearance of a 'Cashup history' section at the bottom of the page 5/ Note that 'Summary' button displays the relevant modal 6/ Signoff Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk> Signed-off-by: Daniel Jones <daniel.jones@cheshiresharedservices.gov.uk> Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk> Created attachment 117450 [details] [review] Bug 26273: (follow-up) Format dates and money correctly. This patch adds the missing formatting for the Datetime and Amount fields. Test plan 1/ Apply patch and reload register page to re-display the cashups table 2/ Note that the 'Date' field now displays a nicely formatted date without an offset. 3/ Note that the 'Amount' field now displays the number according to your money formatting preferences (with 2 decimal places in most cases) 4/ Signoff Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk> Created attachment 117451 [details] [review] Bug 26273: (follow-up) Add 'Cashier' to the cashups table This patch adds the 'Cashier' field as discussed on the bug report. Test plan 1/ Apply patch and reload the register page to refresh the table 2/ Note that a 'Cashier' field now appears in the table 3/ Sorting and searching on 'Cashier' should work as expected 4/ Default sorting should remain as 'Date' order. 5/ Signoff Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk> Rebased against the now PQA dependency bug and set status back to SO Some small things: 1) If there is no first name for the "cashier" the table will display "null", for example: null koha. A lot of libraries may use "functional" accounts where this is a likely scneario. 2) Quite nit-picky (I know!)... the heading "Cashup history" is really close to the "Details" button and further away form the table, much further than the heading of the same label to the table above. I think this does make it a little hard to understand the table structure at a quick glance. Overall this works well, failing for 1) but happy to get something for 2)! Created attachment 117787 [details] [review] Bug 26273: (QA follow-up) Check for null firstname This patch adds a test for firstname being defined before constructing fullname from it. (In reply to Katrin Fischer from comment #27) > Some small things: > > 1) If there is no first name for the "cashier" the table will display > "null", for example: null koha. A lot of libraries may use "functional" > accounts where this is a likely scenario. Fixed in followup > 2) Quite nit-picky (I know!)... the heading "Cashup history" is really close > to the "Details" button and further away form the table, much further than > the heading of the same label to the table above. I think this does make it > a little hard to understand the table structure at a quick glance. This appears to be something funky in the API driven datatables wrapper.. as such I'm tempted to open a distinct bug about it rather than fix it here as it'll affect both here, the Cities management page and the orders tables. Created attachment 117793 [details] [review] Bug 26273: (QA follow-up) Remove unrequired table_controls div Looks like a copy/paste issue.. I don't believe we need this div at all for the table to function as expected and adding it just creates uneeded whitespace. (In reply to Martin Renvoize from comment #29) > This appears to be something funky in the API driven datatables wrapper.. as > such I'm tempted to open a distinct bug about it rather than fix it here as > it'll affect both here, the Cities management page and the orders tables. I take that back.. somehow a spare div crept into the source in this template.. I've removed it in a followup :) Still stuck with the nulls, sorry! Also a little extra space between heading and "Display" could be nice. :) Created attachment 117809 [details] [review] Bug 26273: (QA follow-up) Check for null firstname This patch adds a test for firstname being defined before constructing fullname from it. Created attachment 117810 [details] [review] Bug 26273: (QA follow-up) Remove unrequired table_controls div Looks like a copy/paste issue.. I don't believe we need this div at all for the table to function as expected and adding it just creates uneeded whitespace. Created attachment 117811 [details] [review] Bug 26273: (QA follow-up) Add horizontal rule to distinguish tables try now Created attachment 117829 [details] [review] Bug 26273: Add historic cashups table to register page This patch adds a historic cashups table to the bottom of the register transaction details page. It will only display if one or more cashups have taken place and contains links to allow display of the summary detail modal for each cashup. Test plan 1/ Enable UseCashRegisters 2/ Undertake a few transactions and cashups 3/ View the 'Register details' page 4/ Note the appearance of a 'Cashup history' section at the bottom of the page 5/ Note that 'Summary' button displays the relevant modal 6/ Signoff Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk> Signed-off-by: Daniel Jones <daniel.jones@cheshiresharedservices.gov.uk> Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 117830 [details] [review] Bug 26273: (follow-up) Format dates and money correctly. This patch adds the missing formatting for the Datetime and Amount fields. Test plan 1/ Apply patch and reload register page to re-display the cashups table 2/ Note that the 'Date' field now displays a nicely formatted date without an offset. 3/ Note that the 'Amount' field now displays the number according to your money formatting preferences (with 2 decimal places in most cases) 4/ Signoff Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 117831 [details] [review] Bug 26273: (follow-up) Add 'Cashier' to the cashups table This patch adds the 'Cashier' field as discussed on the bug report. Test plan 1/ Apply patch and reload the register page to refresh the table 2/ Note that a 'Cashier' field now appears in the table 3/ Sorting and searching on 'Cashier' should work as expected 4/ Default sorting should remain as 'Date' order. 5/ Signoff Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 117832 [details] [review] Bug 26273: (QA follow-up) Check for null firstname This patch adds a test for firstname being defined before constructing fullname from it. Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 117833 [details] [review] Bug 26273: (QA follow-up) Remove unrequired table_controls div Looks like a copy/paste issue.. I don't believe we need this div at all for the table to function as expected and adding it just creates uneeded whitespace. Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 117834 [details] [review] Bug 26273: (QA follow-up) Add horizontal rule to distinguish tables Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> + return data.escapeHtml(); It explodes if data is not defined (see bug 25293 for instance). I think we should move the JS function '_escape_str' out of in acqui/parcel.tt to have it available from anywhere. (In reply to Jonathan Druart from comment #43) > + return data.escapeHtml(); > > It explodes if data is not defined (see bug 25293 for instance). > > I think we should move the JS function '_escape_str' out of in > acqui/parcel.tt to have it available from anywhere. staff-global.js perhaps.. or a new variable_filter.js perhaps? Created attachment 118125 [details] [review] Bug 26273: (QA follow-up) Use _escape_str/price for escaping This patch moves the _escape_str and _escape_price functions into the staff-global js include and then uses them in the register page staff-global already includes the afore used escapeHtml prototype extensions and is loaded on all pages... seemed good choice. Tested change in register page and on parcel page too... all working still. :) I am sorry Martin but I think this patch must be moved to its own bug report for backport. Maybe also remove the '_'? I can do it if you want me to. I can do it.. I was just hoping to avoid holding this bug up with another SO + QA cycle on a new bug. Alas, I do agree with you really though. Created attachment 118129 [details] [review] Bug 26273: (QA follow-up) Use _escape_str/price for escaping This patch moves the _escape_str and _escape_price functions into the staff-global js include and then uses them in the register page Done, bug 27930 Created attachment 118130 [details] [review] Bug 26273: (QA follow-up) Use escape_str/price for escaping This patch uses the global escape_str/escape_price functions in the register page. Just a rewording of the commit message to account for most of the actual change being moved to another bug ;) Created attachment 118149 [details] [review] Bug 26273: Add historic cashups table to register page This patch adds a historic cashups table to the bottom of the register transaction details page. It will only display if one or more cashups have taken place and contains links to allow display of the summary detail modal for each cashup. Test plan 1/ Enable UseCashRegisters 2/ Undertake a few transactions and cashups 3/ View the 'Register details' page 4/ Note the appearance of a 'Cashup history' section at the bottom of the page 5/ Note that 'Summary' button displays the relevant modal 6/ Signoff Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk> Signed-off-by: Daniel Jones <daniel.jones@cheshiresharedservices.gov.uk> Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Bug 26273: (QA follow-up) Remove unrequired table_controls div Looks like a copy/paste issue.. I don't believe we need this div at all for the table to function as expected and adding it just creates uneeded whitespace. Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Bug 26273: (QA follow-up) Add horizontal rule to distinguish tables Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 118150 [details] [review] Bug 26273: (follow-up) Format dates and money correctly. This patch adds the missing formatting for the Datetime and Amount fields. Test plan 1/ Apply patch and reload register page to re-display the cashups table 2/ Note that the 'Date' field now displays a nicely formatted date without an offset. 3/ Note that the 'Amount' field now displays the number according to your money formatting preferences (with 2 decimal places in most cases) 4/ Signoff Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 118151 [details] [review] Bug 26273: (follow-up) Add 'Cashier' to the cashups table This patch adds the 'Cashier' field as discussed on the bug report. Test plan 1/ Apply patch and reload the register page to refresh the table 2/ Note that a 'Cashier' field now appears in the table 3/ Sorting and searching on 'Cashier' should work as expected 4/ Default sorting should remain as 'Date' order. 5/ Signoff Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Bug 26273: (QA follow-up) Check for null firstname This patch adds a test for firstname being defined before constructing fullname from it. Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 118152 [details] [review] Bug 26273: (QA follow-up) Use escape_str/price for escaping This patch uses the global escape_str/escape_price functions in the register page. Pushed to master for 21.05, thanks to everybody involved! Enhancement not pushed to 20.11.x Created attachment 120403 [details] [review] Bug 28273: Unit tests Created attachment 120404 [details] [review] Bug 28273: Add Koha::Biblios->pickup_locations |