Bug 27583 - Clarify how cash management fits together
Summary: Clarify how cash management fits together
Status: Signed Off
Alias: None
Product: Koha
Classification: Unclassified
Component: Staff Client (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Martin Renvoize
QA Contact: Testopia
URL:
Keywords: Manual
Depends on: 27582
Blocks: 13985
  Show dependency treegraph
 
Reported: 2021-02-01 12:56 UTC by Martin Renvoize
Modified: 2021-03-16 14:33 UTC (History)
6 users (show)

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


Attachments
Bug 27583: Proposed terminology changes (5.82 KB, patch)
2021-02-01 13:17 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 27583: Proposed terminology changes (5.87 KB, patch)
2021-02-01 13:27 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 27583: Proposed terminology changes (5.96 KB, patch)
2021-03-11 14:18 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 27583: (QA follow-up) Undo developer readability (2.30 KB, patch)
2021-03-15 10:02 UTC, Martin Renvoize
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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.