mglaman / commerce_cart_api

Original sandbox WIP for the module now on Drupal.org. Do not fork. Patches accepted on Drupal.org
https://www.drupal.org/project/commerce_cart_api
9 stars 1 forks source link

RESTishness #15

Open gabesullice opened 5 years ago

gabesullice commented 5 years ago

https://github.com/mglaman/commerce_cart_api/blob/32b525d9d5463ad5cff17165b995952596613cac/src/Routing/Routes.php#L132

This seems very RPC, not REST. It makes me wonder: why is this not a POST to /cart/{cart}/items?

I dug into it and I realized what's happening... It's because the client doesn't know which cart to add it to and it shouldn't have to do the complicated work of figuring it out.

I think there's a more elegant RESTy solution: hypermedia!

For every purchasable entity, you should be able to add an add-to-cart link, like so:

{
  "data": {
    "type": "tshirt",
    "id": "some-uuid-here",
    "links": {
      "self": {"href": "https://tomandjerrystshirts.com/jsonapi/tshirts/some-uuid-here"},
      "add-to-cart": {
        "href": "https://tomandjerrystshirts.com/jsonapi/carts/{cart}/items",
      }
    }
  }
}

Then, the appropriate cart is explicitly communicated to the client and it won't need to know any implementation details. The rule is only that the client must use the add-to-cart link specific to each purchasable entity. There's another advantage to this too: the client app won't need to hardcode /carts/add either and will be less coupled to your implementation.

I've just added some stuff in JSON:API which will make it easy to add these links via a public API, but for now, there are some tricks we can use.

More to come!

mglaman commented 5 years ago

I dug into it and I realized what's happening... It's because the client doesn't know which cart to add it to and it shouldn't have to do the complicated work of figuring it out.

Because the client technically cannot know. The purchasable entity may have to go to another order type, or might not be allowed for the current store. The client does not have to do any of the hardwork, it's done in the endpoint.

That is this logic:

        $store = $this->selectStore($purchased_entity);
        $order_item = $this->orderItemStorage->createFromPurchasableEntity($purchased_entity, [
          'quantity' => !empty($order_item_data['quantity']) ? $order_item_data['quantity'] : 1,
        ]);
        $context = new Context($this->currentUser, $store);
        $order_item->setUnitPrice($this->chainPriceResolver->resolve($purchased_entity, $order_item->getQuantity(), $context));
        $order_type_id = $this->chainOrderTypeResolver->resolve($order_item);
        $cart = $this->cartProvider->getCart($order_type_id, $store);
        if (!$cart) {
          $cart = $this->cartProvider->createCart($order_type_id, $store);
        }
        $order_item = $this->cartManager->addOrderItem($cart, $order_item, TRUE);

This is because we support multiple carts.

gabesullice commented 5 years ago

Yep, that's what I realized. Thanks for confirming.

My suggestion is that you compute the cart when you normalize the purchaseable entity, instead of when that entity gets ordered. You do that by adding a link to the purchaseable entity which will add it to a specific cart.

Where the entity can't be added to a cart, you don't add the link. The consumer can then do something in the UI with that information (like gray out the purchase button) for a better UX.

mglaman commented 5 years ago

The other problem: what if the user does not have a cart? With this flow we would have to force a POST /cart endpoint to ensure a cart is available. That's one reason it is a more generic endpoint.

We could still keep an add-to-cart link which is POST /cart/add and if its not available, we know it is due to AvailabilityManager saying "sorry, not even one remains!"

gabesullice commented 5 years ago

Hmm. I see. Maybe the trick to make it feel a little more "right" is to combine the two approaches a bit by moving the path to /carts/items.

Then you could also do a GET request for all the user's items in all carts. With each item having a link to the specific cart that it's in (if that's important to know).

gabesullice commented 5 years ago

We could still keep an add-to-cart link which is POST /cart/add and if its not available, we know it is due to AvailabilityManager saying "sorry, not even one remains!"

I like this regardless :)