Problem/Motivation

It would be nice if we could change the default currency depending on the site, so if we have a US store it could be USD and if we have a Canadian store it would be CAD.

Right now it fallsback too quickly to the site default currency, if all the order line items are in one currency (or first one IIRC) then it will use that currency for the order.

Proposed resolution

Add an alter hook to commerce_default_currency so that we can hijack that value through a rule or alter hook.

Remaining tasks

User interface changes

N/A

API changes

New alter hook.

CommentFileSizeAuthor
#11 change_site_s_default-2415237-11.patch1.94 KBjaperry
#10 interdiff.txt654 bytesjoelpittet
#10 change_site_s_default-2415237-10.patch1.15 KBjoelpittet
#3 2415237-3-default-currency.patch526 bytesjoelpittet
#1 2415237-1.patch687 bytesjoelpittet
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

joelpittet’s picture

joelpittet
Primary language English
Location Vancouver
CreditAttribution: joelpittet commented
Status: Active » Needs review
FileSize
2415237-1.patch687 bytes

I think this is all that would be needed for this?

Then someone could do this:

function HOOK_commerce_default_currency_alter(&$default) {
  global $language;
  // Likely a map would be better here, but...
  if ($language->language == 'en-CA') {
    $default = 'CAD';
  }
}

Status: Needs review » Needs work

The last submitted patch, 1: 2415237-1.patch, failed testing.

joelpittet’s picture

joelpittet
Primary language English
Location Vancouver
CreditAttribution: joelpittet commented
Status: Needs work » Needs review
FileSize
2415237-3-default-currency.patch526 bytes

Whoops missing --relative there.

joelpittet’s picture

joelpittet
Primary language English
Location Vancouver
CreditAttribution: joelpittet commented
Title: commerce_default_currency to get an alter hook » Change site's default currency through a 'commerce_default_currency' alter hook
joelpittet’s picture

joelpittet
Primary language English
Location Vancouver
CreditAttribution: joelpittet commented
Issue tags: +commerce-sprint
mglaman’s picture

mglaman
Primary language English
Location WI, USA
CreditAttribution: mglaman at Centarro commented 
+++ b/commerce.module
@@ -470,7 +470,9 @@ function commerce_i18n_string($name, $default, $options = array()) {
 function commerce_default_currency() {
...
+  $default = variable_get('commerce_default_currency', 'USD');
+  drupal_alter('commerce_default_currency', $default);
+  return $default;
 }

Random Q: would be want to static cache this so we didn't need to run through all of the alters each time this is invoked?

joelpittet’s picture

joelpittet
Primary language English
Location Vancouver
CreditAttribution: joelpittet commented

@mglaman That may be a good addition. If you feel it's needed we can add that here. There is at least a few function calls in drupal_alter() that could add to the total number of calls.

mglaman’s picture

mglaman
Primary language English
Location WI, USA
CreditAttribution: mglaman at Centarro commented

Never mind my comment. Does exactly as it should. I think any implications (CAD might have a higher price than USD on some stores) would be up to those who implement this hook.

mglaman’s picture

mglaman
Primary language English
Location WI, USA
CreditAttribution: mglaman at Centarro commented
Status: Needs review » Reviewed & tested by the community

Forgot I ended up using this patch as a quick way to switch currencies based on some global variables (such as language.)

joelpittet’s picture

joelpittet
Primary language English
Location Vancouver
CreditAttribution: joelpittet commented
FileSize
change_site_s_default-2415237-10.patch1.15 KB
interdiff.txt654 bytes

Adding api docs.

japerry’s picture

japerry
Location KVUO
CreditAttribution: japerry at Drupal Association commented
Status: Reviewed & tested by the community » Needs review
FileSize
change_site_s_default-2415237-11.patch1.94 KB

heh interesting that we're working on the same issue at the same time! :-D

My patch is slightly different, in that it allows functions calling the default to provide a context, since the currency might get defined by a group. It also sets up a static var so it doesn't get called over and over again during a call.

joelpittet’s picture

joelpittet
Primary language English
Location Vancouver
CreditAttribution: joelpittet commented

It's commerce-sprint time:) interdiff?

joelpittet’s picture

joelpittet
Primary language English
Location Vancouver
CreditAttribution: joelpittet commented

@japerry What kinds of context would you expect to be passed, use-case?

japerry’s picture

japerry
Location KVUO
CreditAttribution: japerry at Drupal Association commented

Mainly we're using the commerce_discount module with conferences to establish different currencies per discount. We pass the discount (Which contains a conference reference) into the default currency check, which is passed into an alter, which grabs the conference node id and a currency sitting in the conference.

joelpittet’s picture

joelpittet
Primary language English
Location Vancouver
CreditAttribution: joelpittet commented

@japerry how do you ensure that context get's passed in on other calls within the system?

joelpittet’s picture

joelpittet
Primary language English
Location Vancouver
CreditAttribution: joelpittet commented
Status: Needs review » Needs work

@japerry I just noticed the static cache. That could actual counter what you are trying to do with the context. Because the context could change the results of the function... maybe you could put this idea into a follow-up so we can get at least this in to commerce?

japerry’s picture

japerry
Location KVUO
CreditAttribution: japerry at Drupal Association commented

For our specific case, we need a static cache because the context should only get called once. When doing a discount calculation, when this is first called, it has a context -- but later on in the same call it does not have it. In general, we don't want the context changing during the page request. There should only be one context being passed in.

og_context went through a similar problem, and was resolved with a static cache, figured a similar strategy could be used here.

japerry’s picture

japerry
Location KVUO
CreditAttribution: japerry at Drupal Association commented
Status: Needs work » Reviewed & tested by the community

Made #2657718: Add a static cache and context to determine default currency in lieu of continuing to stall this issue. #10 is good by me.

rszrama’s picture

rszrama CreditAttribution: rszrama at Centarro commented
Status: Reviewed & tested by the community » Fixed

Thanks guys, committed! (Changed the variable name from $default to $currency_code.)

  • rszrama committed 8d8c8be on 7.x-1.x authored by joelpittet 
    Issue #2415237 by joelpittet: add a hook_commerce_default_currency_alter...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.