Problem/Motivation

in PromotionSubscriber::getPromotions we first check what offer_ids are applicable, and filter out only the ones that implement ShipmentPromotionOfferInterface.

If this is exactly 0, the function getOfferIds will return an empty array.

It is the passed to loadAvailable from PromotionStorage. If the array is empty, then this check will not apply:

if ($offer_ids) {
  $query->condition('offer.target_plugin_id', $offer_ids, 'IN');
}

Leading us to load all available promotions (not only the ones that implements ShipmentPromotionOfferInterface). Later the site will then crash either because of this (depending on the php.ini values for assertions):

assert($offer instanceof ShipmentPromotionOfferInterface);

Or because the offer plugin does not have a method called isDisplayInclusive

Error: Call to undefined method MyOfferPlugin::isDisplayInclusive()

Either way, the site crashes, and it might not be possible to check out.

Steps to reproduce

- Make sure you have no shipping promotion plugins
- Make sure you have some non-shipping promotion plugins
- Make sure you have some promotion created
- Go to the checkout

Proposed resolution

Make sure we do not try to load non-relevant promotions

Remaining tasks

Write tests.
Review. Commit.

User interface changes

None.

API changes

None.

Data model changes

None.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

eiriksm created an issue. See original summary.

eiriksm’s picture

Status: Active » Needs review
FileSize
645 bytes

Should be possible to write a test for this, but here is a fix at least.

eiriksm’s picture

Issue summary: View changes
eiriksm’s picture

A test only patch (which hopefully fail), and the fix again.

The last submitted patch, 4: 3172728-test-only.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

jsacksick’s picture

This looks good except this :):

Tests the shipping method formatter.
+ *
+ * @coversDefaultClass \Drupal\commerce_shipping\Plugin\Field\FieldFormatter\ShippingMethodFormatter
jsacksick’s picture

The one weird thing though... is why don't you have any shipping promotion offers?

eiriksm’s picture

Fair question :)

Because they are unset so the client will not attempt to use them, since either the erp system or the payment provider (don't remember off the top of my head) does not support it somehow :)

  • jsacksick committed 1b8cda0 on 8.x-2.x authored by eiriksm
    Issue #3172728 by eiriksm, jsacksick: Ensure the site doesn't crash if...

  • jsacksick committed 67ca5b1 on 8.x-2.x
    Issue #3172728 followup: Fix phpcs violations.
    
jsacksick’s picture

Status: Needs review » Fixed

Fixed comments, violations and a slight change to the alter() method.

Status: Fixed » Closed (fixed)

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