Steps to reproduce:

  • Install and enable uc_hosting_products (and all dependencies).
  • Create 2 products, and add the "Create a site and adjust quotas accordingly" feature to each.
  • Add one of each to your cart.

What I see is:

  1. Multiple lines with the same product will have the same domain name, updating the quantity of one will update the quantity of the other, and removing one line removes them all.
  2. Adding additional different site products update the domain in the first line, and lose it's own domain.
  3. The same line can have quantities above 1

Comments

gboudrias’s picture

gboudrias commented

Here's the bug as far as I understand it:

uc_hosting_orders has a site_url line. That means it considers that there's a single site URL per order that contains any amount of hosting products.

This means that the bug isn't that the site URL is updated for all products. The problem is that the products are working as designed, each showing (and being able to update) the site URL of the order (but there's only 1 site URL).

It seems to me that the site_url information needs to be in a new table, uc_hosting_line_items if we follow the UC schema/convention.

The lazy solution would be to stuff it in the order_data column of uc_hosting_orders, but ergonlogic and I agree that this isn't the right way to do it and that a new table is rather necessary in the long term.

There's still the problem of quantities in uc_hosting_products line items, but that should perhaps be its own issue, the solution to which could be "don't allow quantities greater than 1 for hosting products".

First I'm going to try and add a table get uc_hosting_products to consider line items. Could take a bit of refactoring so I'll report here first.

gboudrias’s picture

gboudrias commented

Actually, I just realized uc_hosting_line_items is for "non-product order items". The new table should be "uc_hosting_order_products".

Also, this change would require changing uc_hosting.install for the new table, uc_hosting.module to disregard site_url during order creation, uc_hosting_associate because the site would be associated with an order's product rather than just an order and finally uc_hosting_product because that's where the bulk of the site_url logic is.

I'll start a branch.

sfyn’s picture

sfyn commented

Your approach seems fine to me.

Note that this is a major refactor of the project - the assumption of one site per order is embedded deeply in the module and you will have to rework most of the UI flow in order to get this working. Good luck!

gboudrias’s picture

gboudrias commented
Assigned: Unassigned » gboudrias

Perfect!

Feature branch is dev-1766002-sites-cart-items

ergonlogic’s picture

ergonlogic
he/him
Primary language English
Location Montréal, Québec 🇨🇦
commented

On further reflection, we might want to take @sfyn's warning to heart. We're already planning a re-write to support Commerce and Services. Perhaps taking a lighter approach would work here. Rather than trying to shoe-horn multiple product orders and such into uc_hosting, perhaps we should just further enforce the assumptions already made.

That is, we could make it so that we can only order a single product (of quantity 1) at a time. I think that would solve the original issue. Ubercart itself doesn't really support subscriptions very well, so I expect we'd run into all kinds of other complications should we pursue this path.

For example, thanks to some recent patches, uc_recurring is working reasonably well. What if a client ordered (subscribed to) 2 Open Atriums sites and an Open Outreach site, then she decides that she wants to cancel her subscription to one of the Open Atriums. If we pursue the idea of supporting multiple quantities on multiple lines, this would likely all go on a single order, and we'd have to make sure our recurring billing takes into account the change. On the other hand, going with the single subscription per order model, all we'd have to do is stop recurring billing for one of the subscriptions. Probably a cleaner solution, all around.

gboudrias’s picture

gboudrias commented

Good points, ergonlogic. Perhaps it's best to reserve refactoring for a future major version, though I still think it's conceptually cleaner.

We should talk about it tomorrow.

ergonlogic’s picture

ergonlogic
he/him
Primary language English
Location Montréal, Québec 🇨🇦
commented
Status: Active » Fixed

We went ahead and limited orders of site products to a single product at a time, and only ever of quantity 1.

Status: Fixed » Closed (fixed)

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