Body: <p>We are experiencing very unexpected behaviour with a very simple plugin we wrote and are stymied as to whether this is revealing a bug in Commerce or whether we have completed missed something.</p>
  
  <p>Here is what we're doing in conceptual terms:</p>
  
  <p>We need to offer different prices for users based on their country (just US and CAD right now). The Commerce base currency has been set to CAD and we have added a custom field for products to hold a US price. So each product has a CAD price using Commerce's base currency and a US price using a custom field.</p>
  
  <p>We check the user's country and based on whether it is Canada or the US, our plugin uses <code>onPopulateLineItem</code> and <code>onBeforeSaveOrder</code> to update the price and currency appropriately:</p>
  
  <pre><code>class FlatPricingPlugin extends BasePlugin
  {
      public function init()
      {
          parent::init();
  
          // If there is no log here, cp login will fail.  This line is
          // not nessesary if dev mode is turned on.  This is probably a
          // server specific issue.
          FlatPricingPlugin::log(
              'This is a required log line. See FlatPricingPlugin.php',
              LogLevel::Info, true);
  
  
          // Use the american price if currency is set to USD
          craft()-&gt;on('commerce_lineItems.onPopulateLineItem', function($event){
              $line = $event-&gt;params['lineItem'];
              $currency_key = 'currencyAbbreviation';
              $currency = craft()-&gt;httpSession-&gt;get($currency_key);
              if($currency == 'USD'){
                  $price = $line-&gt;purchasable-&gt;product-&gt;productPriceUSD;
                  $line['price'] = $price;
              }
          });
  
          // Set the currency and payment currency to the chosen currency
          craft()-&gt;on('commerce_orders.onBeforeSaveOrder', function($event){
              $order = $event-&gt;params['order'];
              $currency_key = 'currencyAbbreviation';
              $currency = craft()-&gt;httpSession-&gt;get($currency_key);
              FlatPricingPlugin::log("$currency -- {$order-&gt;itemSubtotal}, {$order-&gt;itemTotal}, {$order-&gt;totalPrice}, {$order-&gt;totalPaid}");
              if ($currency == 'USD'){
                  $order-&gt;paymentMethodId = 2;
                  $order-&gt;currency = $currency;
                  $order-&gt;paymentCurrency = $currency;
              }else{
                  $order-&gt;paymentMethodId = 1;
                  $order-&gt;currency = $currency;
                  $order-&gt;paymentCurrency = $currency;
              }
          });
  
      }
  }
  </code></pre>
  
  <p>This works but we have run into a reproducible scenario where placing an order overwrites the currency of previous orders. The scenario involves placing an order from one country and then, for the same user, placing an order when they are in the other country. In this case, the old order (or orders) have their currencies changed to match the new country.</p>
  
  <p>We have tested this on a vanilla install of Craft and Commerce with only this one plugin. Here is a <a href="http://dev.coalescentdesign.com/flat-pricing-issue.zip" rel="nofollow noreferrer">link</a> to a ZIP with a DB dump, our plugin and a readme with full setup and reproduce instructions.</p>
  
Comments.ContentLicense: CC BY-SA 3.0
Comments.CreationDate: 2017-06-28T06:43:07.830Z
Comments.Text: I think the issue stems from your onBeforeSaveOrder hook. You should only change the paymentMethodId if the order is not complete (`$order->isCompleted == false`). I think a re-save of the order is happening at a later time (possibly when changing an orders status or something else) and you are setting the paymentMethodId/currency of those orders based on something you have stored in the session.
Comments.UserDisplayName: Luke Holder
Comments.ContentLicense: CC BY-SA 3.0
Comments.CreationDate: 2017-06-28T06:46:07.653Z
Comments.Text: Also have you looked at the `COMMERCE_PAYMENT_CURRENCY` constant? It might be a better way to define the current currency based on the locale, better than the custom config variable you made. You then get automatic payment currency switching on the cart and you could use the same constant to switch on in your `onPopulateLineItem` event. 
   https://craftcommerce.com/docs/payment-currencies#switching-currencies
Comments.UserDisplayName: Luke Holder
Comments.ContentLicense: CC BY-SA 3.0
Comments.CreationDate: 2017-07-17T19:49:15.683Z
Comments.Text: It looks like the problem was caused by the `commerce_orders.onBeforeSaveOrder` event being fired for each of that user's previous orders when they log in (note: it seems to happen for a front end and CP login).  Adding the `isComplete` check fixed the issue. However, the event firing at login seems like an odd place for it to fire. Are we misunderstanding events or is that possible a bug?
Comments.UserDisplayName: Mike
Comments.ContentLicense: CC BY-SA 3.0
Comments.CreationDate: 2017-07-21T22:06:16.097Z
Comments.Text: Just a bump... wondering if Luke has thoughts on my last comment?
Comments.UserDisplayName: Mike
Tags: <plugin-craftcommerce>
Title: Commerce Plugin Unintentionally Rewriting Past Orders - Commerce Bug or Something Else?
ThreadText: # Commerce Plugin Unintentionally Rewriting Past Orders - Commerce Bug or Something Else?
  
  (Asked by: username_0 on 2017-06-27)
  
  We are experiencing very unexpected behaviour with a very simple plugin we wrote and are stymied as to whether this is revealing a bug in Commerce or whether we have completed missed something.
  
  Here is what we're doing in conceptual terms:
  
  We need to offer different prices for users based on their country (just US and CAD right now). The Commerce base currency has been set to CAD and we have added a custom field for products to hold a US price. So each product has a CAD price using Commerce's base currency and a US price using a custom field.
  
  We check the user's country and based on whether it is Canada or the US, our plugin uses `onPopulateLineItem` and `onBeforeSaveOrder` to update the price and currency appropriately:
  
  ```
  class FlatPricingPlugin extends BasePlugin
  {
      public function init()
      {
          parent::init();
  
          // If there is no log here, cp login will fail.  This line is
          // not nessesary if dev mode is turned on.  This is probably a
          // server specific issue.
          FlatPricingPlugin::log(
              'This is a required log line. See FlatPricingPlugin.php',
              LogLevel::Info, true);
  
          // Use the american price if currency is set to USD
          craft()->on('commerce_lineItems.onPopulateLineItem', function($event){
              $line = $event->params['lineItem'];
              $currency_key = 'currencyAbbreviation';
              $currency = craft()->httpSession->get($currency_key);
              if($currency == 'USD'){
                  $price = $line->purchasable->product->productPriceUSD;
                  $line['price'] = $price;
              }
          });
  
          // Set the currency and payment currency to the chosen currency
          craft()->on('commerce_orders.onBeforeSaveOrder', function($event){
              $order = $event->params['order'];
              $currency_key = 'currencyAbbreviation';
              $currency = craft()->httpSession->get($currency_key);
              FlatPricingPlugin::log("$currency -- {$order->itemSubtotal}, {$order->itemTotal}, {$order->totalPrice}, {$order->totalPaid}");
              if ($currency == 'USD'){
                  $order->paymentMethodId = 2;
                  $order->currency = $currency;
                  $order->paymentCurrency = $currency;
              }else{
                  $order->paymentMethodId = 1;
                  $order->currency = $currency;
                  $order->paymentCurrency = $currency;
              }
          });
  
      }
  }
  
  ```
  
  This works but we have run into a reproducible scenario where placing an order overwrites the currency of previous orders. The scenario involves placing an order from one country and then, for the same user, placing an order when they are in the other country. In this case, the old order (or orders) have their currencies changed to match the new country.
  
  We have tested this on a vanilla install of Craft and Commerce with only this one plugin. Here is a [link](http://dev.coalescentdesign.com/flat-pricing-issue.zip) to a ZIP with a DB dump, our plugin and a readme with full setup and reproduce instructions.
  
  **Tags:** plugin-craftcommerce
  
  > **Comments:**
  > **username_1**: I think the issue stems from your onBeforeSaveOrder hook. You should only change the paymentMethodId if the order is not complete (`$order->isCompleted == false`). I think a re-save of the order is happening at a later time (possibly when changing an orders status or something else) and you are setting the paymentMethodId/currency of those orders based on something you have stored in the session.  
  > **username_1**: Also have you looked at the `COMMERCE_PAYMENT_CURRENCY` constant? It might be a better way to define the current currency based on the locale, better than the custom config variable you made. You then get automatic payment currency switching on the cart and you could use the same constant to switch on in your `onPopulateLineItem` event.   https://craftcommerce.com/docs/payment-currencies#switching-currencies  
  > **username_0**: It looks like the problem was caused by the `commerce_orders.onBeforeSaveOrder` event being fired for each of that user's previous orders when they log in (note: it seems to happen for a front end and CP login).  Adding the `isComplete` check fixed the issue. However, the event firing at login seems like an odd place for it to fire. Are we misunderstanding events or is that possible a bug?  
  > **username_0**: Just a bump... wondering if username_1 has thoughts on my last comment?  
  
  ---
  
  ## 0 answers
  
  