Bug 24454 - currency formatting from JS
Summary: currency formatting from JS
Status: RESOLVED DUPLICATE of bug 12987
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Agustín Moyano
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-01-17 18:10 UTC by Agustín Moyano
Modified: 2021-09-23 14:41 UTC (History)
3 users (show)

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


Attachments
Bug 24454: Add Currencies.pm template plugin and currency.inc files (5.44 KB, patch)
2020-01-17 20:55 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 24454: Add Currencies.pm template plugin and currency.inc files (5.47 KB, patch)
2020-01-31 13:16 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 28854: Database update (6.29 KB, patch)
2021-09-23 14:39 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 28854: DBIC Schema Updates (7.92 KB, patch)
2021-09-23 14:39 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 28854: Expose functionality to attach items to bundles (36.70 KB, patch)
2021-09-23 14:39 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 28854: Add bundle content display to OPAC (6.65 KB, patch)
2021-09-23 14:39 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 28854: Update circulation functionality for bundles (24.80 KB, patch)
2021-09-23 14:39 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 28854: Record and display who lost the item (3.89 KB, patch)
2021-09-23 14:39 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 28854: Respect WhenLostChargeReplacementFee (2.78 KB, patch)
2021-09-23 14:39 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 Agustín Moyano 2020-01-17 18:10:15 UTC
API calls in OPAC brings raw JSON.

We need a way to format data into the configured currency from JS.
Comment 1 Agustín Moyano 2020-01-17 20:55:22 UTC
Created attachment 97573 [details] [review]
Bug 24454: Add Currencies.pm template plugin and currency.inc files

This patch adds a new TT plugin called Currencies which retrieves information about the active currency.

It also adds a currency.inc file in both opac and intranet, which if included in your TT, you will have the $currency function available in JS.

The $currency function formats numbers into the configured active currency format.

For example:

$currency(12322.4532) //Returns '$12,322.45' if currency format is 'US' and active currency has '$' symbol and no space separation.

To test you must implement and test bug 20212, where it will be used
Comment 2 Martin Renvoize 2020-01-31 09:03:47 UTC
Erm....  did you see /koha-tmpl/intranet-tmpl/prog/en/includes/format_price.inc ?

Just trying to wrap my head around the difference between this and that at the moment.
Comment 3 Martin Renvoize 2020-01-31 09:05:49 UTC
(In reply to Martin Renvoize from comment #2)
> Erm....  did you see
> /koha-tmpl/intranet-tmpl/prog/en/includes/format_price.inc ?
> 
> Just trying to wrap my head around the difference between this and that at
> the moment.

Frankly, that include is well hidden.. it was introduced in bug 12987
Comment 4 Jonathan Druart 2020-01-31 09:41:44 UTC
Hola Agustín,
As Martin said, there already is another include file doing more or less the same things.

Several points:
1. Do you have an example that is using that code?
2. The existing include file is wrong (written 6 years ago), and is ignoring Koha::Number::Price, in which we are defining the display option of prices.
3. See also bug 20352, this is where are should go, ie. all the parameters defined in the currency table, not depending on the CurrencyFormat syspref (we should get rid of it)
4. I have the feeling that the currency TT plugin should not exist, but Price could be used instead, to retrieve the format_parameters.
5. Note that the copyright statement is wrong

Let me know if you have any questions!
Comment 5 Tomás Cohen Arazi 2020-01-31 13:16:46 UTC
Created attachment 98228 [details] [review]
Bug 24454: Add Currencies.pm template plugin and currency.inc files

This patch adds a new TT plugin called Currencies which retrieves information about the active currency.

It also adds a currency.inc file in both opac and intranet, which if included in your TT, you will have the $currency function available in JS.

The $currency function formats numbers into the configured active currency format.

For example:

$currency(12322.4532) //Returns '$12,322.45' if currency format is 'US' and active currency has '$' symbol and no space separation.

To test you must implement and test bug 20212, where it will be used

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 6 Tomás Cohen Arazi 2020-01-31 13:22:52 UTC
I added my signature, but keeping the NSO, but I have to say: this is awesome Agustín! I left it for the last step in my dev (bug 20212) and cought me by surprise: it is so elegant :-D

Jonathan: take a look at the latest WIP patch in bug 20212 to see how to use it!

Agustín: I think we should be able to specify if we want the currency symbol. In my use case the original table doesn't show it. But if we mixed currencies, it would be great being able to choose which one to use, not just the active. So default: active, but have the option to pass a currency id.

I know we need a third party signature.
Comment 7 Tomás Cohen Arazi 2020-01-31 14:42:28 UTC
Jonathan, I belive we would be good with just using format_price. We overlooked it in the codebase. You mention it should do things different (using Koha::Number::Price) does that imply some work is needed on it? Regarding the currency symbol we can go without it, and add it later if required. The symbol is not used in the table I'm wokring on (pending orders).

Closing this, bug if you say we can just use your method.

*** This bug has been marked as a duplicate of bug 12987 ***
Comment 8 Martin Renvoize 2021-09-23 14:39:06 UTC
Created attachment 125193 [details] [review]
Bug 28854: Database update
Comment 9 Martin Renvoize 2021-09-23 14:39:09 UTC
Created attachment 125194 [details] [review]
Bug 28854: DBIC Schema Updates
Comment 10 Martin Renvoize 2021-09-23 14:39:13 UTC
Created attachment 125195 [details] [review]
Bug 28854: Expose functionality to attach items to bundles

This patch adds methods the the Koha::Item object for managing item
bundling operations and then exposes those methods via the REST API.

We include the new `BundleNotLoanValue` preference for setting not
for loan values when an item is added to a bundle.

Finally, we expose bundle management via the catalogue details page.

Test plan:
0) Apply patches up to this point
1) Creating a new bundle
   * Add a new bib record
   * Mark the bib record as a 'collection' type by setting leader
     position 7 to 'c'
   * Add a new item to this bib record
   * You should see a new 'Manage bundle' button available in the
     'Actions' column of the Holdings table.
   * Clicking 'Manage bundle' should expand the table to include a new
     row directly beneath this one.
   * Use the new 'Add to bundle' button that appears in this row to
     trigger a modal that allows entering the barcode of items you wish
     to add to the bundle
   * Upon closing the modal, the bundle content table should reload and
     contain your newly associated items.
   * You can subsequently remove an item from a bundle using the new
     'Remove' button.
2) Not for loan
   * Items that have been added into a bundle should now appear as 'Not
     for loan' from their original biblio record and note which bundle
     they belong to.
3) Error cases
   * Try adding an item that already belongs to a bundle to another
     bundle: Note an error is displayed in the modal form.

TODO: Add Object Tests + API Tests
Comment 11 Martin Renvoize 2021-09-23 14:39:17 UTC
Created attachment 125196 [details] [review]
Bug 28854: Add bundle content display to OPAC

TODO: Switch to /public api route
TODO: Move expand button to right like staff client
Comment 12 Martin Renvoize 2021-09-23 14:39:20 UTC
Created attachment 125197 [details] [review]
Bug 28854: Update circulation functionality for bundles

This patch updates the circulation system to account for bundle
checkins. We add a content verification step to ensure bundle content is
all present at checkin and we use this comparison to mark missing items
as lost.

Test plan
0) Apply patches up to this point
1) Checkin an item that belongs to a bundle
   * An alert should be triggered noting that the item belongs to a
     bundle
   * The option to remove the item from the bundle should be clear
   * Click remove should result in the alert dissapearing and the item
     having been removed from the bundle.
2) Checkin an item bundle
   * A modal confirmation dialog should appear requesting each item
     barcode be scanned
   * As items are scanned they should be highlighted in yellow in the
     bundle content table
   * Upon submission;
     * The user will be alerted to any unexpected items that were
       scanned and told to put them to one side.
     * The user will be alerted that any missing items in the validation
       will have been marked as lost.
     * The bundle item will be marked as checked in.
Comment 13 Martin Renvoize 2021-09-23 14:39:24 UTC
Created attachment 125198 [details] [review]
Bug 28854: Record and display who lost the item

This patch records the bundle issue from which an item is marked as lost
so that we may use that to infer who lost the item (for later charges
and display).

Test plan
0) Apply all patches up to this point
1) Checkout a bundle to a user
2) Checkin the bundle and do not scan one of the barcodes at
   confirmation
   * Note that the item not scanned is marked as lost
3) Navigate to the biblio for the lost item and note that the patron who
   lost the item is displayed
4) Checkin the lost item
   * The item should be marked as found and the items_lost_issue table
     should no longer contain a link to the issue
Comment 14 Martin Renvoize 2021-09-23 14:39:29 UTC
Created attachment 125199 [details] [review]
Bug 28854: Respect WhenLostChargeReplacementFee