Owner is not a required field.
But site crash when create/save a store if leave owner be empty

Drupal\Core\Entity\EntityStorageException: SQLSTATE[23000]: Integrity constraint violation: 1048 Column 'uid' cannot be null: INSERT INTO {commerce_store_field_data} (store_id, type, langcode, uid, name, mail, default_currency, address__langcode, address__country_code, address__administrative_area, address__locality, address__dependent_locality, address__postal_code, address__sorting_code, address__address_line1, address__address_line2, address__organization, address__given_name, address__additional_name, address__family_name, default_langcode, prices_include_tax) VALUES (:db_insert_placeholder_0, :db_insert_placeholder_1, :db_insert_placeholder_2, :db_insert_placeholder_3, :db_insert_placeholder_4, :db_insert_placeholder_5, :db_insert_placeholder_6, :db_insert_placeholder_7, :db_insert_placeholder_8, :db_insert_placeholder_9, :db_insert_placeholder_10, :db_insert_placeholder_11, :db_insert_placeholder_12, :db_insert_placeholder_13, :db_insert_placeholder_14, :db_insert_placeholder_15, :db_insert_placeholder_16, :db_insert_placeholder_17, :db_insert_placeholder_18, :db_insert_placeholder_19, :db_insert_placeholder_20, :db_insert_placeholder_21); Array ( [:db_insert_placeholder_0] => 1 [:db_insert_placeholder_1] => online [:db_insert_placeholder_2] => en [:db_insert_placeholder_3] => [:db_insert_placeholder_4] => www [:db_insert_placeholder_5] => wwww@163.com [:db_insert_placeholder_6] => USD [:db_insert_placeholder_7] => [:db_insert_placeholder_8] => AF [:db_insert_placeholder_9] => [:db_insert_placeholder_10] => www [:db_insert_placeholder_11] => [:db_insert_placeholder_12] => [:db_insert_placeholder_13] => [:db_insert_placeholder_14] => www [:db_insert_placeholder_15] => www [:db_insert_placeholder_16] => [:db_insert_placeholder_17] => [:db_insert_placeholder_18] => [:db_insert_placeholder_19] => [:db_insert_placeholder_20] => 1 [:db_insert_placeholder_21] => 0 ) in Drupal\Core\Entity\Sql\SqlContentEntityStorage->save() (line 783 of /app/www/d86/core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorage.php).
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

caseylau created an issue. See original summary.

lawxen’s picture

Issue summary: View changes
mglaman’s picture

How was this created? Over API or in the UI? We have a default value defined, which makes the UI work. Just curious.

      ->setDefaultValueCallback('Drupal\commerce_store\Entity\Store::getCurrentUserId')

But it does look like we need to mark the base field required.

lawxen’s picture

FileSize
133.82 KB

Over API or in the UI?

Both over api and the ui.

Example in the UI

lawxen’s picture

Example in the api:

$store->set('uid', NULL);
$store->save();

mglaman’s picture

Ha! I never tried outright deleting the value. Yep. We should patch and set the flag

init90’s picture

Set 'owner' field as required to prevent error during store save

mglaman’s picture

Status: Active » Needs review
vdenis’s picture

Nice patch tbh!

I've tested and patch applies cleanly. Good job.

init90’s picture

Status: Needs review » Reviewed & tested by the community
bojanz’s picture

Status: Reviewed & tested by the community » Needs work

This needs more research.

None of Drupal's core entities have a required owner field.
Neither do products in Commerce, and yet deleting the author doesn't trigger a crash like it does for stores.
When you delete the value on a product, it saves "Anonymous (0)". When you do the same on a store, it saves NULL. And yet the field definitions look the same.

EDIT: I've also confirmed that doing ->set('uid', NULL) on a product, followed by a save, doesn't trigger a crash.

init90’s picture

@bojanz thanks for the review.

For products, this problem was solved earlier. Related issue:

https://www.drupal.org/project/commerce/issues/2854193

Also, the same solution used for nodes.

In the patch, I reuse code from the issue above, but not sure if simple copy-paste proper way for it. Perhaps we should use a trait or something similar to avoid code duplication?

init90’s picture

Status: Needs work » Needs review

  • bojanz committed f6d7947 on 8.x-2.x authored by pavlo.dovhan
    Issue #3020240 by pavlo.dovhan, caseylau, bojanz: Site crash when create...
bojanz’s picture

Status: Needs review » Fixed

I like #12, since it matches products and others.

Let's fix the bug first and think about code reuse later. Thanks!

Status: Fixed » Closed (fixed)

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