Problem

When the user has a address book entry and wants to enter a new billing address, the "billing same as shipping" checkbox gets checked on switching to "+ Enter new address".

Solution

TBD.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Siegrist created an issue. See original summary.

flocondetoile’s picture

Same bug here. Any advices ?

flocondetoile’s picture

Looks like when selecting "+ Enter new address", the ajax refresh send a $user_imput which doesn't have the copy_field input
and so in ProfileFieldCopy.php, line 83

if ($user_input && isset($user_input['copy_fields'])) {
      $enabled = $user_input['copy_fields']['enable'];
    }

We fallback to the default value $enabled set as TRUE because $user_input['copy_fields'] is not set.

flocondetoile’s picture

Note: this bug doesn't appears if we go to the review step (with new address selected and the copy_fields[enable] unchecked again) and go back on the order_information step.

flocondetoile’s picture

Status: Active » Needs review
FileSize
1.15 KB

Trying to demonstrate the bug with a failing test. Switching to Need review to trigger the bot.

Status: Needs review » Needs work

The last submitted patch, 5: 3119449-5.patch, failed testing. View results

flocondetoile’s picture

Status: Needs work » Needs review
FileSize
2.28 KB

Patch with a fix.

Status: Needs review » Needs work

The last submitted patch, 7: 3119449-7.patch, failed testing. View results

flocondetoile’s picture

New failing test pass (is green now), but we have then an error on another test about Confirm that the copy_fields checkbox is still checked after selecting a different payment option ("Credit card", in this case).. Anyone with a project with some payment methods to debug this ?

jsacksick’s picture

This is weird! I've been trying to reproduce this manually and I couldn't so far.

jsacksick’s picture

Actually, I misunderstood the bug in the first place, I managed to reproduce the bug and the test demonstrates it, but not 100% sure about the fix.

dgDatpasst’s picture

Hi, same error here, patch is working for me, thanks

mglaman’s picture

mglaman’s picture

+++ b/src/ProfileFieldCopy.php
@@ -79,11 +79,15 @@ public function alterForm(array &$inline_form, FormStateInterface $form_state) {
     $user_input = (array) NestedArray::getValue($form_state->getUserInput(), $inline_form['#parents']);
+    $user_values = (array) NestedArray::getValue($form_state->getValues(), $inline_form['#parents']);
...
+    elseif ($user_values && isset($user_values['copy_fields'])) {
+      $enabled = $user_values['copy_fields']['enable'];
+    }

I'm going to assume part of the problem is stale user input values

mglaman’s picture

Status: Needs work » Needs review
FileSize
122.23 KB
892 bytes

Okay, I think I found the logic problem.

    // Copying is enabled by default for new billing profiles.
    $enabled = $billing_profile->getData('copy_fields', $billing_profile->isNew());

We default to copying for new profiles by default, but this doesn't account for the fact the user may be choosing a new profile and not entering without an existing saved profile. Instead of checking if there were other eligible profiles, we can check if the select_address property exists in the user input.

The fix is:

    if ($user_input) {
      if (isset($user_input['copy_fields'])) {
        $enabled = (bool) $user_input['copy_fields']['enable'];
      }
      elseif (isset($user_input['select_address'])) {
        $enabled = FALSE;
      }
    }

It feels a bit messy, though. I wonder if it can be condensed at all.

mglaman’s picture

Forgot to include tests from #7

  • mglaman committed 5d21cd9 on 8.x-2.x
    Issue #3119449 by mglaman, flocondetoile, jsacksick, Siegrist,...
mglaman’s picture

Status: Needs review » Fixed

This has passed QA of a launched site. Committed! Thanks, all.

Status: Fixed » Closed (fixed)

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