Handle no variation product gracefully

Created on 25 May 2024, 11 months ago

Describe your bug or feature request.

Short:
If a product type has no variations, there is only one place in ProductVariationContext that coughs.
Upcoming patch prevents this.

Long:
I know that a product type without variation bundles is bending the commerce data model.
Yet i have a use case (kind of multi product) where this is a perfect fit.
I audited commerce code how that will do, and found (apart from the form, which is easy to alter) only one minor place that cought: ProductVariationContext. And that can be silenced easily. MR will follow.

It allows people like us (yes i know, i'm on my own) an edge case without affecting anything else.

📌 Task
Status

Active

Version

2.0

Component

Product

Created by

🇩🇪Germany geek-merlin Freiburg, Germany

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @geek-merlin
  • Open in Jenkins → Open on Drupal.org →
    Core: 9.5.x + Environment: PHP 8.0 & MySQL 5.7
    last update 11 months ago
    794 pass
  • Status changed to Needs review 11 months ago
  • 🇩🇪Germany geek-merlin Freiburg, Germany
  • 🇩🇪Germany geek-merlin Freiburg, Germany
  • Pipeline finished with Success
    11 months ago
    Total: 499s
    #182151
  • Open in Jenkins → Open on Drupal.org →
    Core: 9.5.x + Environment: PHP 8.0 & MySQL 5.7
    last update 10 months ago
    794 pass
  • Pipeline finished with Success
    10 months ago
    Total: 553s
    #182699
  • 🇮🇱Israel jsacksick

    Are you sure this is the only place that is problematic to support this? There's probably plenty of code assuming variation types are returned by $product_type->getVariationTypeIds().

  • Pipeline finished with Success
    7 months ago
    Total: 368s
    #274884
  • Pipeline finished with Success
    6 months ago
    Total: 620s
    #300676
  • Pipeline finished with Success
    6 months ago
    Total: 52s
    #300757
  • Pipeline finished with Success
    6 months ago
    Total: 410s
    #305998
  • Pipeline finished with Canceled
    6 months ago
    Total: 440s
    #306001
  • Pipeline finished with Success
    6 months ago
    Total: 550s
    #306006
  • 🇩🇪Germany geek-merlin Freiburg, Germany

    Hey Jonathan,
    > Are you sure this is the only place that is problematic to support this? There's probably plenty of code assuming variation types are returned by $product_type->getVariationTypeIds().

    Short: Yes.

    Long version: I'm running this on a critical site and don't want to shoot me in the foot. So i checked all usages of said method (only 10-ish, most in foreach), and robustified the 3 places where it coughs.

    Now i rebased and simplified the MR, re-applied it on my site, and verified all integration tests are still green.

  • Pipeline finished with Success
    5 months ago
    Total: 158s
    #336486
  • Pipeline finished with Success
    5 months ago
    Total: 163s
    #336487
  • Pipeline finished with Success
    5 months ago
    Total: 159s
    #336490
  • 🇩🇪Germany geek-merlin Freiburg, Germany

    Added 3.0.x MR too.

    @jsacksick See #6.

  • Pipeline finished with Failed
    5 months ago
    Total: 520s
    #342134
  • 🇩🇪Germany geek-merlin Freiburg, Germany

    The only change in 3.0.x was due to the nice simplification in 📌 Remove the variationType setting Fixed .

  • Pipeline finished with Failed
    5 months ago
    Total: 776s
    #342150
  • 🇮🇱Israel jsacksick

    Would be great if we could get additional tests to demonstrate the failure... Also don't want to shoot myself in the foot here :)... As this is really an edge case.

  • 🇩🇪Germany geek-merlin Freiburg, Germany

    Not sure when, but challenge taken.

  • Pipeline finished with Success
    3 months ago
    Total: 166s
    #383950
  • Pipeline finished with Failed
    3 months ago
    Total: 166s
    #383952
  • Pipeline finished with Success
    3 months ago
    Total: 163s
    #383956
  • Pipeline finished with Failed
    3 months ago
    Total: 54s
    #387734
  • Pipeline finished with Failed
    3 months ago
    Total: 769s
    #387959
  • Pipeline finished with Failed
    3 months ago
    Total: 418s
    #389746
  • Pipeline finished with Failed
    3 months ago
    Total: 883s
    #389777
  • Pipeline finished with Success
    3 months ago
    Total: 1117s
    #389799
  • Pipeline finished with Canceled
    3 months ago
    Total: 237s
    #389846
  • Pipeline finished with Failed
    3 months ago
    Total: 375s
    #389851
  • Pipeline finished with Canceled
    3 months ago
    Total: 924s
    #389860
  • Pipeline finished with Success
    3 months ago
    Total: 890s
    #389880
  • Pipeline finished with Failed
    3 months ago
    Total: 207s
    #389898
Production build 0.71.5 2024