Add a status field (enabled/disabled) to stores

Sure, why not.

Might need a bit more work, let's see.

New patch in PR at https://github.com/drupalcommerce/commerce/pull/829

To prevent a similar situation to the 'default' widget description, let's debate the following before this gets committed:

Do you:
A. Publish / Unpublish a store
or
B. Enable / Disable a store

Published is really a 'node'-thing i believe. And it's actually called status in the database (the field).

If we go with B the PR needs a tweak.

TODO: a description might not hurt for the status field?

I think we should use the terms enable / disable.

Ok, i'll update the PR.

Are revisions possible for the shops ?

Not at this time. You might want to create a new issue for this, and not sure what the maintainers think about revisionable stores. (@bojanz what do you think about revisions for stores and the direction of the current PR)

I like "Enable / Disable a store"

This is good if you're bringing a new store online and adding products, but need to reference that store -- and that store can't show up in a store selector, for instance.

Can we moderate the stores ?

Only if we made them support revisions. But I'm not sure the benefit there? In a marketplace I would assume the "store" entity acts like a settings page, not a piece of content.

Here is a rerolled patch. We no longer need specific actions as core provides them. This also copies the Promotion format for "Status" as a radio of enabled or disabled.

Forgot an upgrade path!

Retitling.

It's obvious from the discussion above that it makes more sense for a store to be enabled/disabled than published/unpublished.
This is already the case for Promotion, Coupon, ShippingMethod, PriceList, PriceListItem.

As a result of that, we should not be using EntityPublishedInterface and EntityPublishedTrait (just like the other entity types listed above don't).
It might make sense to introduce our own interface/trait, now that it's a common pattern.

The listed entities also don't have a per-bundle setting ("Enable new stores of this type by default"), we always default to TRUE instead, but that's cause they don't use bundle entities at all. Do we still want that feature, or was it just copied from core?

EDIT: Opened #3004026: Add EntityEnabledInterface and EntityEnabledTrait.

Closed #3004026: Add EntityEnabledInterface and EntityEnabledTrait:

We have decided not to do this.

Lots of code assumes EntityPublishedInterface (Entity API access/query access, Search API filters, Views, Content Moderation, etc). It is not feasible to reimplement all of it. We'll have to accept that content entities always implement EntityPublishedInterface, even if their UI labels don't match the code terminology. After all, we already do it with bundles/types and labels/names/titles.

This means that the current patch is close to done. We are missing update code for adding the "published" entity key though.

As for this:

The listed entities also don't have a per-bundle setting ("Enable new stores of this type by default"), we always default to TRUE instead, but that's cause they don't use bundle entities at all. Do we still want that feature, or was it just copied from core?

Doesn't hurt to keep it, we have it on product types too.

At one of my projects, I've added state_machine for the organization of moderation workflow.
After last updates, all views are throwing exception with: The "commerce_store" entity type did not define a "published" key in
I've added published key to the store entity and it is fixed my problem.
I see from description that this issue is connected to my problem and fix.
How I can help you to fix finish patch here?

Rerolled patch against the latest version.

Update patch.

If the store is unpublished, the associated products must be unpublished automatically (or manually with a warning message "To unpublish the store, you must either unpublish the associated products or dissociate them from the store).

If the store is unpublished and the products are associated with more than one store, then the products must have remained unpublished.

That's a lot. The products don't need to be unpublished, with the purchased_entity constraint it should just not be "add to cartable" because it has no valid stores. Using event subscribers/hooks you can add a more robust reaction.

I rerolled this for Commerce 8.x-2.24, hope someone finds this helpful.

We are requiring this feature in our project. Would like to see this added shortly.

From what I can see, this patch does nothing more than create the field, anyone can do that via the entity system. What we need is to make sure that the commerce module takes this into account in all the important places (checkout, order creation, store_domain resolvers etc

I think this feature makes a lot of sense. You create a store, you have orders associated to a store, then you close it. Is there another way of approaching this?

From what I can see, this patch does nothing more than create the field, anyone can do that via the entity system. What we need is to make sure that the commerce module takes this into account in all the important places (checkout, order creation, store_domain resolvers etc

Because we use the EntityAccessControlHandler and implement the EntityPublishedInterface, we also get access control "for free".

See:

      if ($entity instanceof EntityPublishedInterface && !$entity->isPublished()) {
        if ($account->id() != $entity->getOwnerId()) {
          // There's no permission for viewing other user's unpublished entity.
          return AccessResult::neutral()->cachePerUser();
        }

        $permissions = [
          "view own unpublished {$entity->getEntityTypeId()}",
        ];
        $result = AccessResult::allowedIfHasPermissions($account, $permissions)->cachePerUser();
      }

We probably need tests coverage and need to think which code needs to change...
We should try by opening a merge request.

I'm going to remove some of the code from the patch, especially this:

+    if ($store->status->value != $value) {
+      $fields = $this->entityFieldManager->getFieldDefinitions('commerce_store', $this->entity->id());
+      $fields['status']->getConfig($this->entity->id())->setDefaultValue($value)->save();
+      $this->entityFieldManager->clearCachedFieldDefinitions();
+    }

We might need a bundleFieldDefinitions() implementation from the Store entity class to properly handle the default status value for bundles.
For now, I'll ignore this.

EDIT: Hm... I just realized this was copied from the ProductTypeForm.

So, I expanded the checkout access logic to ensure access is allowed if the order belongs to a published store. I made changes to the cart provider to flag a cart as non eligible if it belongs to a disabled store.

The product form still allows assigning a product to disabled stores... But perhaps we should keep that? As this technically allows preparing products for a store that will be enabled in the future?

Let's see what the tests say now.

I wonder if we need a hook_entity_access() implementation to restrict access to products belonging to a store that is unpublished; At the same time, if a product belongs to multiple stores, perhaps we should still allow access.

Tempted to merge the current MR and implement additional access logic in followup issues. This should be a good starting point.

Ok, I had started working on denying view access to products belonging to unpublished stores, but I changed my mind after discussing with @rszrama because:

A store might be "closed" for business but a product page still be a relevant link to remain active

.

Will go ahead and merge the MR as is for now.