verbb / events

Craft CMS Plugin for events management and ticketing.
Other
22 stars 13 forks source link

Error when updating ticket capacity #154

Closed kennethormandy closed 1 week ago

kennethormandy commented 1 month ago

Describe the bug

When trying to update the ticket quantity, I get:

Typed property craft\commerce\behaviors\CurrencyAttributeBehavior::$_defaultCurrency must not be accessed before initialization

Thanks!

Steps to reproduce

  1. Save a new ticket with a capacity
  2. Update the capacity

Craft CMS version

5.4.8

Plugin version

3.0.0-beta.5

Multi-site?

No

Additional context

No response

kennethormandy commented 1 month ago

Not sure if this is the same issue or a different one, but:

engram-design commented 1 month ago

Good call, I wasn't expecting a difference in behaviour for this being out of the slideout. Fixed for the next release. To get this early, run composer require verbb/events:"dev-craft-5 as 3.0.0-beta.5".

So changing the price should certainly be changing the price on the front-end, because a Ticket's price is resolved dynamically back to the Ticket Type. Maybe if you can check that again after this fix?

kennethormandy commented 3 weeks ago

Hey, that seems to have gotten rid of the error, but I’m seeing some other strange things. This is all with 3.0.0-beta.6 now.

If I try and create a new event, and make the session and/or ticket type at the same time (in a slideout), the blue line shows up on the left like the have been edited, but they aren’t actually in the table.

They don’t show up after creating the entry.

If I create and save the event (no ticket type or session), and then make those individually after the fact, it does work.

So changing the price should certainly be changing the price on the front-end, because a Ticket's price is resolved dynamically back to the Ticket Type. Maybe if you can check that again after this fix?

I’m still seeing this too: if I edit a price, it does change correctly on the ticket type, but on the front-end it still just shows the original price.

engram-design commented 3 weeks ago

Yeah, I've seen this happen sporadically when doing it all in one go. Something to do with Craft's element index cache. But hitting save always makes them show up on the next page refresh..

Can you let me know what templating you're using to show your tickets? That seems to be working for me with the following:

<form method="POST">
    <input type="hidden" name="action" value="commerce/cart/update-cart">
    {{ redirectInput('shop/cart') }}
    {{ csrfInput() }}

    <input type="number" name="qty" value="1">

    <select name="purchasableId">
        {% for ticket in event.tickets %}
            <option value="{{ ticket.id }}" {% if not ticket.isAvailable %}disabled{% endif %}>
                {{ ticket.title }} - ${{ ticket.price }} x{{ ticket.stock }}
            </option>
        {% endfor %}
    </select>

    <input type="submit" value="Add to cart" class="button">
</form>
engram-design commented 3 weeks ago

I think I've gotten to the bottom of the caching issue at least. Fixed for the next release. To get this early, run composer require verbb/events:"dev-craft-5 as 3.0.0-beta.6".

But, still need to test of the ticket price issue.

kennethormandy commented 2 weeks ago

Hey, I’m seeing everything I would expect in the control panel now, but not the front-end.

When I edit the ticket types, the price and capacity do get updated as expected in the element index. But no matter what I do, I’m not ever seeing any updates on the front-end. The price always stays as what its first value was.

So if I make a new ticket type, set the price to $50, it will always be $50 on the front-end. If I change it to $49, it will show correctly as $49 in the control panel, but still say and dump as $50 in Twig.

Here’s the form I’m using, nearly identical to what you provided:

  <form method="POST">
    {{ csrfInput() }}
    {{ actionInput('commerce/cart/update-cart') }}
    {{ redirectInput('shop/cart') }}

    {{ input('number', 'qty', 1) }}

    <select name="purchasableId">
      {%- for ticket in event.getTickets() -%}

        <option value="{{ ticket.id }}" {% if not ticket.isAvailable %}disabled{% endif %}>
          {{ ticket.title }} - {{ ticket.price|commerceCurrency(cart.currency) }}
        </option>
      {%- endfor -%}
    </select>

    <button type="submit">Add to cart</button>
  </form>

Manually clearing all caches doesn’t seem to do anything, nor does generating new catalogue pricing.

When I save the initial ticket, I see a Commerce catalogue pricing job run, and not when I edit the ticket type. But when I manually run another one using the CLI, that doesn’t change anything either.

kennethormandy commented 2 weeks ago

One other thing that seems relevant: if I export a CSV from the Tickets element table, I see the same data and prices that I see on the front-end—ie. what I would consider to be the outdated data.

It’s only in the element table UI that the new price is showing correctly for me.

engram-design commented 2 weeks ago

So the Commerce catalog pricing shouldn't even be doing anything, because we don't actually use it. The price is dynamically resolved from the ticket type, so I'm struggling to figure out how that's even being cached or out of date.

Maybe it's something different compared to our Commerce store setups? Is your store a multi-store (or a multi-site) install? I don't actually see the catalog pricing stuff, so maybe that's a clue as to what's going on.

If you hop in the commerce_catalogpricing database table, and modify the price column for a matching Ticket element (the purchasableId column), I'd reckon that fixes things, but it shouldn't be using that value at all!

kennethormandy commented 1 week ago

No, I’m not doing anything special with Commerce—it’s a completely fresh Craft 5 and Commerce 5 install I set up locally to try out the plugin.

You might be right that’s relevant, though. A fresh Commerce 5 won’t use the old sales feature, but an upgraded Commerce 4 project that had a least one sale might still be using the old sales feature? Would that maybe be the difference in our setups?

If I output this:

<ul>
  {% for ticket in event.tickets %}
    {% set ticketType = ticket.type %}

    <li>
      “{{ ticketType.title }}” ticket type price: {{ ticketType.price }}<br />
      “{{ ticket.title }}” ticket base price: {{ ticket.basePrice }}<br />
      “{{ ticket.title }}” ticket price: {{ ticket.price }}<br />
    </li>

  {% endfor %}
</ul>

I see this:

Screenshot 2024-11-08 at 9 48 41 AM

So you’re right the basePrice is correct, but the price isn’t.

If you hop in the commerce_catalogpricing database table, and modify the price column for a matching Ticket element (the purchasableId column), I'd reckon that fixes things, but it shouldn't be using that value at all!

You’re correct, the outdated value is here.


Edit Okay—it seems to me like that is probably the reason: https://github.com/craftcms/commerce/blob/199220346d7bdf346e03013eff422dcda12c529b/src/base/Purchasable.php#L528-L530

engram-design commented 1 week ago

Thanks for investigating, I'll see if I can track that down and sort this out!

engram-design commented 1 week ago

So, still can't reproduce it with a brand-new, fresh install, but I'm now overriding the getPrice() method as well (should have in the first place!). Let me know if that's fixed for you. To get this early, run composer require verbb/events:"dev-craft-5 as 3.0.0".

Otherwise, maybe worth sending through your database to support@verbb.io so I can take a look!

engram-design commented 1 week ago

Should be fixed in 3.0.1

kennethormandy commented 1 week ago

Hey, thanks, this did fix the price issue for me!

I’m still seeing an issue with the ticket capacity, but I’ll open a new issue, since I don’t think it’s related to any of this.