Problem/Motivation

According to OrderInterface the return value of getTotalPrice can be NULL. Same goes for getUnitPrice on the order item. In that case FacebookCommerce throws an error since the price rounder expects a Price object.

I have practically encountered this error in a customized checkout process.

Proposed resolution

Sent 0 as the price if there is no order total/order item unit price set.

Issue fork facebook_pixel-3300892

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

krystalcode created an issue. See original summary.

krystalcode opened merge request !7

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Status: Active » Needs work
Issue tags: +Needs tests

@krystalcode thank you, the MR looks good. But to be sure this won't break anything and won't break in the future it would be great to have some tests for this. Could you please do that?

Did you already test this manually for all possible cases?

krystalcode’s picture

krystalcode commented

Writing test will be difficult at this time, especially since there's no test already for this to build on top of.
I have done some limited manual testing, I can report back after running the patch on production for some time.

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented

Thanks @krystalcode that would be helpful. Anyway users complaining about issues here, should start adding tests to improve the situation. This is open source and we're paid 0 cent for this... ;) Just like you.

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented

@krystalcode any updates here? And could you help adding tests to the module?

krystalcode’s picture

krystalcode commented

@anybody

  • This patch has been running on production for several months now without reported issues.
  • Unfortunately I don't see being able to write tests. I contributed what I could at the time, somebody else can take it from here.
grevil’s picture

grevil commented

A similar problem arises in the module file, if the price is null:

TypeError: Drupal\commerce_price\Rounder::round(): Argument #1 ($price) must be of type Drupal\commerce_price\Price, null given, called in /var/www/html/web/modules/custom/facebook_pixel/modules/facebook_pixel_commerce/facebook_pixel_commerce.module on line 61 in Drupal\commerce_price\Rounder->round() (line 29 of modules/contrib/commerce/modules/price/src/Rounder.php).

Let's fix that here as well.

grevil’s picture

grevil commented
Status: Needs work » Needs review
Issue tags: -Needs tests

Cleaned up the existing code a bit and fixed the module file implementation. Please review!

Note, that by the official meta documentation, "value" doesn't seem to support NULL values, so let's use "0" where we can.

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Status: Needs review » Reviewed & tested by the community

Nice, good enough to fix the issue. Can't be perfect, I think...

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented

grevil’s picture

grevil commented
Status: Reviewed & tested by the community » Fixed

Thanks all!

Status: Fixed » Closed (fixed)

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