Wrong next step after login pane [#2981922]

Configure a checkout flow with

Login => Login or continue as guest
Order information=> Contact information and nothing else

Start checkout as anonymous user. On the login page, choose to login. Error message is

Notice: Undefined index: order_information in Drupal\commerce_checkout\Plugin\Commerce\CheckoutFlow\CheckoutFlowBase->buildForm() (line 259 of XXX/modules/commerce/modules/checkout/src/Plugin/Commerce/CheckoutFlow/CheckoutFlowBase.php).

I guess that the problem is that commerce evaluated the next step based on the steps that were visible when logged out which includes order_information. However once logged in, that step is no longer visible. Maybe the problem is that CheckoutFlowWithPanesBase.php caches the value of visibleSteps and this cache is not cleared when the current user changes.

Comment	File	Size	Author
#10	commerce_checkout.commerce_checkout_flow.default.yml	772 bytes	AdamPS
#7	commerce-checkout_flow_steps-2981922-7-EXPECTED_FAIL.patch	731 bytes	AaronBauman
#7	8.x-2.x: PHP 5.5 & MySQL 5.5, D8.5 496 pass
#4	commerce.checkout_steps.2981922-4.patch	2.75 KB	AdamPS
#4	8.x-2.x: PHP 5.5 & MySQL 5.5, D8.5 482 pass
#2	commerce.checkout_steps.2981922-2.patch	2.75 KB	AdamPS
#2	8.x-2.x: PHP 5.5 & MySQL 5.5, D8.5 Patch Failed to Apply

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

26 June 2018 at 21:07

AdamPS created an issue. See original summary.

Comment #2

AdamPS CreditAttribution: AdamPS at AlbanyWeb commented 27 June 2018 at 17:34

Status:

Active

» Needs review

File	Size
commerce.checkout_steps.2981922-2.patch	2.75 KB
8.x-2.x: PHP 5.5 & MySQL 5.5, D8.5 Patch Failed to Apply

It's a bit awkward to fix - after login, the existing login step is no longer visible, so getNextStepId doesn't work.

Comment #3

27 June 2018 at 17:39

Status:

Needs review

» Needs work

The last submitted patch, 2: commerce.checkout_steps.2981922-2.patch, failed testing. View results

Comment #4

AdamPS CreditAttribution: AdamPS at AlbanyWeb commented 27 June 2018 at 18:16

Status:

Needs work

» Needs review

File	Size
commerce.checkout_steps.2981922-4.patch	2.75 KB
8.x-2.x: PHP 5.5 & MySQL 5.5, D8.5 482 pass

Comment #5

bojanz CreditAttribution: bojanz at Centarro for Ny Media AS commented 27 June 2018 at 21:50

We need a failing test, the fix can come later.

Comment #6

AdamPS CreditAttribution: AdamPS at AlbanyWeb commented 28 June 2018 at 06:25

Status:	Needs review	» Needs work
Issue tags:		+Needs tests

@bojanz thanks for taking a look at the issue. Setting status to "Needs work"/"Needs tests" as I don't have time to write a failing test right at this moment. If any one else is interested in this issue and willing to write a test please go ahead.

Comment #7

AaronBauman

he/him

CreditAttribution: AaronBauman at Message Agency commented 17 August 2018 at 14:36

Status:

Needs work

» Needs review

File	Size
commerce-checkout_flow_steps-2981922-7-EXPECTED_FAIL.patch	731 bytes
8.x-2.x: PHP 5.5 & MySQL 5.5, D8.5 496 pass

I'm getting the same notice.

This patch isn't a test, but demonstrates the problem by adding an input check where the notice is being generated: if the given $step_id is not returned from ::getVisibleSteps(), then we shouldn't be trying to display it.

This causes some existing tests to fail, demonstrating the error.

Comment #8

AaronBauman

he/him

CreditAttribution: AaronBauman at Message Agency commented 17 August 2018 at 15:20

OK i guess i'm eating crow - must be something specific to site configuration.
@AdamPS, can you apply patch #7 to see if this exception gets triggered?

Comment #9

AaronBauman

he/him

CreditAttribution: AaronBauman at Message Agency commented 17 August 2018 at 15:20

Status:

Needs review

» Needs work

Comment #10

AdamPS CreditAttribution: AdamPS at AlbanyWeb commented 17 August 2018 at 17:24

File	Size
commerce_checkout.commerce_checkout_flow.default.yml	772 bytes

@aaronbauman Thanks for your help.

Yes it is specific to site configuration. The issue summary says "Configure a checkout flow with...". To generate a failing test would require making a test with that specific commerce flow.

I don't really know anything about writing tests for Commerce. However I have attached an export of my checkout flow. I know it seems odd putting payment_information in step "review", but this is a site that does not collect address information and we have overridden the class for payment_information using ys_commerce_checkout_pane_info_alter.