Bug 27583

Summary: Clarify how cash management fits together
Product: Koha Reporter: Martin Renvoize <martin.renvoize>
Component: Fines and feesAssignee: Martin Renvoize <martin.renvoize>
Status: RESOLVED FIXED QA Contact: Marcel de Rooy <m.de.rooy>
Severity: enhancement    
Priority: P5 - low CC: andrew, gmcharlt, katrin.fischer, kyle, lucy.vaux-harvey, m.de.rooy, oleonard, sally.healey
Version: masterKeywords: Manual
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Small patch
Text to go in the release notes:
This enhancement serves to clarify the parts of the cash management module by updating the names or pages and embellishing the breadcrumb navigation for these pages.
Version(s) released in:
21.11.00
Bug Depends on: 27582    
Bug Blocks: 13985    
Attachments: Bug 27583: Proposed terminology changes
Bug 27583: Proposed terminology changes
Bug 27583: Proposed terminology changes
Bug 27583: (QA follow-up) Undo developer readability
Bug 27583: Proposed terminology changes
Bug 27583: Proposed terminology changes

Description Martin Renvoize 2021-02-01 12:56:53 UTC
The new(ish) cash management options in Koha aren't clearly defined... you can enable parts of the system without enabling all of it.

We should try to clarify the module by more clearly defining how it fits together.

I propose to add a 'Cash management' heading under 'Tools' to move the 'Cashuup registers' option to and renaming some of the interface area's to better clarify their purposes.
Comment 1 Martin Renvoize 2021-02-01 13:17:56 UTC
Created attachment 116156 [details] [review]
Bug 27583: Proposed terminology changes

This patch impliments some terminology changes and clarifications for
the cash management system

Test plan
1/ Apply the patches
2/ Enable 'UseCashRegisters'
3/ Navigate around the cash management system (via the Tools menu)
4/ Enable 'EnablePointOfSale'
5/ Navigate around the cash management system (via the Point of sale
module)
6/ Comment on what you think about the changes
Comment 2 Martin Renvoize 2021-02-01 13:27:28 UTC
Created attachment 116157 [details] [review]
Bug 27583: Proposed terminology changes

This patch impliments some terminology changes and clarifications for
the cash management system

Test plan
1/ Apply the patches
2/ Enable 'UseCashRegisters'
3/ Navigate around the cash management system (via the Tools menu)
4/ Enable 'EnablePointOfSale'
5/ Navigate around the cash management system (via the Point of sale
module)
6/ Comment on what you think about the changes
Comment 3 ByWater Sandboxes 2021-03-11 14:18:26 UTC
Created attachment 118127 [details] [review]
Bug 27583: Proposed terminology changes

This patch impliments some terminology changes and clarifications for
the cash management system

Test plan
1/ Apply the patches
2/ Enable 'UseCashRegisters'
3/ Navigate around the cash management system (via the Tools menu)
4/ Enable 'EnablePointOfSale'
5/ Navigate around the cash management system (via the Point of sale
module)
6/ Comment on what you think about the changes

Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk>
Comment 4 Sally 2021-03-11 14:19:46 UTC
I like these changes - the placement is logical, and I like the way it displays when you're logged in with a till attached to your session vs without.  

The POS menu, in particular, is much improved.
Comment 5 Katrin Fischer 2021-03-13 17:09:53 UTC
Comment on attachment 118127 [details] [review]
Bug 27583: Proposed terminology changes

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

::: koha-tmpl/intranet-tmpl/prog/en/modules/pos/register.tt
@@ +19,5 @@
> +<div id="breadcrumbs">
> +    <a href="/cgi-bin/koha/mainpage.pl">Home</a>
> +    &rsaquo; <a href="/cgi-bin/koha/tools/tools-home.pl">Cash management</a>
> +    &rsaquo; Transaction history for [% register.name | html %]
> +</div>

Please don't do this - it breaks the strings up for translations. It's easier to translate the full path for context.
Comment 6 Katrin Fischer 2021-03-13 19:48:11 UTC
(In reply to Katrin Fischer from comment #5)
> Comment on attachment 118127 [details] [review] [review]
> Bug 27583: Proposed terminology changes
> 
> Review of attachment 118127 [details] [review] [review]:
> -----------------------------------------------------------------
> 
> ::: koha-tmpl/intranet-tmpl/prog/en/modules/pos/register.tt
> @@ +19,5 @@
> > +<div id="breadcrumbs">
> > +    <a href="/cgi-bin/koha/mainpage.pl">Home</a>
> > +    &rsaquo; <a href="/cgi-bin/koha/tools/tools-home.pl">Cash management</a>
> > +    &rsaquo; Transaction history for [% register.name | html %]
> > +</div>
> 
> Please don't do this - it breaks the strings up for translations. It's
> easier to translate the full path for context.

Actually I am having trouble to prove that claim and have quite some errors running gulp po:update hmm.
Comment 7 Martin Renvoize 2021-03-15 09:37:54 UTC
I've proved it for you here.. sorry, I wasn't aware that splitting it nicely over lines caused translations issues.. it's done like that all over the codebase and in my opinion, is much more readable.. such a shame we can't use it :(.
Comment 8 Martin Renvoize 2021-03-15 09:40:59 UTC
Having said that, from my layman's perspective having the segments as distinct strings (and being able to share a single translation for the first chunk 'Home' for example seems better for translation rather than worse to me..
Comment 9 Martin Renvoize 2021-03-15 10:02:06 UTC
Created attachment 118224 [details] [review]
Bug 27583: (QA follow-up) Undo developer readability

I added new lines for each slug of the breadcrumbs (As is the case in
many other locations in Koha), but Katrin highlighted that this breaks
the string into distinct slugs without context for translation and this
is apparently a bad thing. So this patch undoes that change and puts
them back onto one long line.
Comment 10 Martin Renvoize 2021-03-15 10:13:05 UTC
OK.. I've added a followup to undo the readability improvements.

Katrin, can you bring this up as a topic for a dev meeting.. if we really do need to keep breadcrumb slugs all on one line I feel we should have a guideline written for it with clear reasoning so QA team are aware and can enforce the policy.
Comment 11 Martin Renvoize 2021-03-16 14:33:49 UTC
Obsoleted my final patch.. after some discussion we decided the split wasn't a bad thing.
Comment 12 Martin Renvoize 2021-05-13 08:40:31 UTC
Created attachment 120912 [details] [review]
Bug 27583: Proposed terminology changes

This patch impliments some terminology changes and clarifications for
the cash management system

Test plan
1/ Apply the patches
2/ Enable 'UseCashRegisters'
3/ Navigate around the cash management system (via the Tools menu)
4/ Enable 'EnablePointOfSale'
5/ Navigate around the cash management system (via the Point of sale
module)
6/ Comment on what you think about the changes

Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk>
Comment 13 Marcel de Rooy 2021-07-09 06:21:54 UTC
Created attachment 122690 [details] [review]
Bug 27583: Proposed terminology changes

This patch impliments some terminology changes and clarifications for
the cash management system

Test plan
1/ Apply the patches
2/ Enable 'UseCashRegisters'
3/ Navigate around the cash management system (via the Tools menu)
4/ Enable 'EnablePointOfSale'
5/ Navigate around the cash management system (via the Point of sale
module)
6/ Comment on what you think about the changes

Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 14 Jonathan Druart 2021-07-26 10:47:22 UTC
Pushed to master for 21.11, thanks to everybody involved!
Comment 15 Kyle M Hall 2021-07-30 14:58:35 UTC
Enhancement not backported.