Problem/Motivation

Testing a new order created from the UI, when I try to enter a "Billing information", "The billing information is the same as the shipping information" checkbox is presented to me despite having no shipping information entered.

This seems to be due to the ShippingProfileSubscriber that returns a new profile:

      if (!$shipping_profile instanceof ProfileInterface) {
        $shipping_profile = $this->shippingOrderManager->createProfile($order);
      }

So this makes me wonder if we shouldn't update ProfileFieldCopy::alterForm() to return early if the shipping profile is new?
With that said, I wonder if that means the profile copy checkbox is no longer going to be added / presented in checkout when it should be? Because the profile will be created on submission, or even earlier if the auto recalculation is turned on for shipping rates and the profile is saved early.

At the minimum, we shouldn't output "Current shipping information" if the shipping profile is empty.

So I'm thinking we have 2 potential solutions:

  1. Return early if $shipping_profile->isNew() but that might actually break checkout?
  2. Add a helper method that checks if any of the configurable field names is empty?

For 2), that means copying the logic from Profile::getConfigurableFieldNames() that loops over configurable fields, the logic should return FALSE if one of the configurable fields isn't empty:

    protected function profileIsEmpty(ProfileInterface $profile): bool {
      foreach ($profile->getFieldDefinitions() as $field_name => $definition) {
        if ($definition instanceof BaseFieldDefinition)) {
          continue;
        }
        if ($profile->get($field_name)->isEmpty()) {
          return FALSE;
        }
      }
     return TRUE;
   }

Attaching a screenshot that demonstrates what I'm referring to:

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:

Comments

jsacksick created an issue. See original summary.

jsacksick’s picture

Issue summary: View changes
StatusFileSize
new34.57 KB
jsacksick’s picture

Issue summary: View changes

Also, I'm thinking that it might make sense to provide different handling for the admin, and return early if the profile is new when we're in the admin, like the following:

      /** @var \Drupal\commerce\Plugin\Commerce\InlineForm\EntityInlineFormInterface $plugin */
      $plugin = $inline_form['#inline_form'];
      $configuration = $plugin->getConfiguration();
  
      if (!empty($configuration['admin']) && $shipping_profile->isNew()) {
         return;
      }

This way, the fix is targeted for the admin and this ensures we don't break checkout.

jsacksick’s picture

Status: Active » Needs review

Let's see which tests are failing.

jsacksick’s picture

Title: Profile copying shouldn't be offered if the source profile is empty » Profile copying shouldn't be offered in the admin if the source profile is empty

  • jsacksick committed 7232f57b on 3.x
    fix: #3561183 Profile copying shouldn't be offered if the source profile...
jsacksick’s picture

Status: Needs review » Fixed

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

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