search

OFFLINE-GmbH / oc-mall-plugin

:convenience_store: E-commerce solution for October CMS
https://offline-gmbh.github.io/oc-mall-plugin
MIT License
169 stars 114 forks source link

Error after failing payment in account/orders #360

Closed igor-tv closed 4 years ago

igor-tv commented 4 years ago

After failing to pay for an order using PayPal, I tried to go to the account's order page and received this error:

An exception has been thrown during the rendering of a template ("Trying to get property 'latest_file' of non-object").
/Applications/MAMP/htdocs/shop/plugins/offline/mall/components/orderslist/productlist.htm line 32

In this order there were only non-virtual goods.

igor-tv commented 4 years ago

Sorry, I was unable to replay this behavior. Perhaps the problem was in the wrong settings on my part. I changed the currency code.

igor-tv commented 4 years ago

But I found another bug. If you have an order with a failed payment, and for some reason you changed the currency code on the backend, then you will not be able to go to the details page of this unsuccessful order on the backend.

Error:

No query results for model [OFFLINE \ Mall \ Models \ Currency].
/Applications/MAMP/htdocs/shop/vendor/laravel/framework/src/Illuminate/Database/Eloquent/Builder.php line 414
tobias-kuendig commented 4 years ago

This is fixed in df69e4e, thanks @igor-tv!

chocolata commented 3 years ago

Hi guys,

I was able to reproduce the error... After loggin in in a users account, I get the following stack trace. Do you know what the issue might be?

 ERROR

We're sorry, but an unhandled error occurred. Please see the details below.

An exception has been thrown during the rendering of a template ("Trying to get property 'latest_file' of non-object").

/var/www/vhosts/mydomain.com/httpdocs/plugins/offline/mall/components/orderslist/productlist.htm line 32

26272829303132333435363738 | {% endif %}{% if product.product_file_grants.count > 0 %}    
    {% for grant in product.product_file_grants %}        
                    Download {{ grant.display_name }}            {{ loop.length > 1 ? ('#' ~ loop.index) }}                {% if grant.expires_at %}            
valid until {{ grant.expires_at.toDateString() }}        {% endif %}        {% if grant.max_download_count %} -- | --
chocolata commented 3 years ago

A quick update - I managed to get rid of the error. The database table offline_mall_product_files was empty, but the table offline_mall_product_file_grants did have a lot of references. I'm guessing the offline_mall_product_file_grants tried to look up files that don't exist. What do you think?

Is it the case that when a product file is deleted, the related file grant is deleted aswel? If not, I'm guessing that is the source of the error...

chocolata commented 3 years ago

Hi, I'm doing some more testing and it does seem like even though a product has no associated files, the table offline_mall_product_file grants is being populated. The problem is that is then displays this strange message: "Download download 114". If you click on it, the file is not found, because it doesn't exist.

What can we do to tackle this?

image

In the logs it then states [OFFLINE.Mall] A virtual product without a file attachment has been purchased. You need to fix this!. But what about virtual products like event tickets? I think we should allow virtual products not to have a file attached to them.

Can you let us know how to proceed?

tobias-kuendig commented 3 years ago

The log message above comes from here: https://github.com/OFFLINE-GmbH/oc-mall-plugin/blob/develop/classes/downloads/VirtualProductFileDownload.php#L65

This method is only triggered when you visit the download link. Where do you list the download link? If you don't have any file attached, why show a download link?

Do you generate a file on your own?

It's probably a good idea to check before this for loop if the product actually has any files attached and if not, the grant should never be created:

https://github.com/OFFLINE-GmbH/oc-mall-plugin/blob/b104849606639f2929f676c189d52a16aefc8628/models/ProductFileGrant.php#L64

chocolata commented 3 years ago

Hi @tobias-kuendig

Thanks so much for your answer. You are correct - I don't want to show a download link if there is no file associated with the virtual product. But at the moment the download link is being displayed regardless:

The download link is displayed in the following partials:

/plugins/offline/mall/components/orderslist/productslist.htm

                {% if product.product_file_grants.count > 0 %}
                    <br/>
                    {% for grant in product.product_file_grants %}
                        <br/>
                        <a href="{{ grant.download_link }}">
                            Download {{ grant.display_name }}
                            {{ loop.length > 1 ? ('#' ~ loop.index) }}
                        </a>
                        {% if grant.expires_at %}
                            <br>valid until {{ grant.expires_at.toDateString() }}
                        {% endif %}
                        {% if grant.max_download_count %}
                            <br>max. {{ grant.max_download_count }} downloads
                        {% endif %}
                    {% endfor %}
                {% endif %}

And also in one of the fragments of one of the emails sent by the system mall.order.table

So basically - if you create a virtual product and do not link a file to it, the above code does still display a download link in the format Download Download ID.

I assume that if there is no file connected to a virtual product, a file grant should not be created. Am I overlooking something?

Thanks for looking into it, I appreciate it.

P.S.: My client is absolutely thrilled with your e-commerce solution. It is so incredibly complete and practical right out of the box. And it's a joy to work with. We only need to remove a few small hiccups and we're golden!

tobias-kuendig commented 3 years ago

I assume that if there is no file connected to a virtual product, a file grant should not be created. Am I overlooking something?

This is correct. Before this line, you can try something like this (untested):

https://github.com/OFFLINE-GmbH/oc-mall-plugin/blob/b104849606639f2929f676c189d52a16aefc8628/models/ProductFileGrant.php#L64

// Do not create a download grant if the product has no files attached.
if (!$orderProduct->product->latest_file) {
    return;
}

This should get rid of the empty download grants, and since no grants are created, the code in the partial you linked above should be skipped ({% if product.product_file_grants.count > 0 %})

If this fixes your issue a PR is as always welcome!

chocolata commented 3 years ago

Hi @tobias-kuendig,

You're the master!

Just tested it out, both with and without a file attached to the product. Now it works perfectly. I've created a pull request here: https://github.com/OFFLINE-GmbH/oc-mall-plugin/pull/862

Please note one small thing, not important for me on the short term: I notice that the download link is available even if the order hasn't been paid yet. I can easily circumvent this by changing the Twig code, but I just wanted to let you know.

Thanks for all of your work and help.

dathwa commented 2 years ago

This is causing serious issues with my virtual downloads. My products do NOT have a file associated. A pdf is generated on the file, after the mall.product.file_grant.created event is triggered. But these changes (https://github.com/OFFLINE-GmbH/oc-mall-plugin/pull/862/commits/52f112c5d36b091b4a082472590cd407455c6121) are preventing this. I've commented out for my purposes, but can we have a fix for this?

  • © Githubissues.
  • Githubissues is a development platform for aggregating issues.