New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix cart front #8036

Merged
merged 2 commits into from Jun 21, 2017

Conversation

Projects
None yet
4 participants
@maximebiloe
Contributor

maximebiloe commented Jun 21, 2017

Questions Answers
Branch? develop
Description? The cart in the JS prestashop object wasn't updated after a change.
Type? bug fix
Category? FO
BC breaks? no
Deprecations? no
Fixed ticket?
How to test?

@maximebiloe maximebiloe added this to the 1.7.2.0 milestone Jun 21, 2017

@aleeks

This comment has been minimized.

Show comment
Hide comment
@aleeks

aleeks Jun 21, 2017

Contributor

Thank you @maximebiloe

Contributor

aleeks commented Jun 21, 2017

Thank you @maximebiloe

@aleeks aleeks merged commit eeafa5e into PrestaShop:develop Jun 21, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@aleeks aleeks deleted the maximebiloe:fix-cart-front branch Jun 21, 2017

@reho

This comment has been minimized.

Show comment
Hide comment
@reho

reho Nov 17, 2017

Hello @maximebiloe

My Prestahop theme just started to fails on this row which is added by you, during cart update (change quantity):
_prestashop2['default'].cart = event.reason.cart;

Can you answer it why is this needed? I can't find out why you leave this row but if I delete it, it works because the event.reason.cart doesn't exists in calls of updateCart. (i bought the theme which isn't works, not developed by me)

I am looking after the parts of core.js but i can't find this row in the cart.js part. But the final 1.7.2 and later contains it, this is how i faced the problem...

reho commented on 7656a40 Nov 17, 2017

Hello @maximebiloe

My Prestahop theme just started to fails on this row which is added by you, during cart update (change quantity):
_prestashop2['default'].cart = event.reason.cart;

Can you answer it why is this needed? I can't find out why you leave this row but if I delete it, it works because the event.reason.cart doesn't exists in calls of updateCart. (i bought the theme which isn't works, not developed by me)

I am looking after the parts of core.js but i can't find this row in the cart.js part. But the final 1.7.2 and later contains it, this is how i faced the problem...

This comment has been minimized.

Show comment
Hide comment
@maximebiloe

maximebiloe Nov 17, 2017

Contributor

Hello @reho,

Is your theme compatible with the PrestaShop version you're using? Which PrestaShop version do you use?
You can look at this other PR which was meant to fix the native PrestaShop theme: #8142
You should open a ticket on PrestaShop bug-tracker to let the team know (I'm no longer a team member) about the issue and fix it if there is an error.
Don't forget to precise the PrestaShop version, the error in the browser console and all the other relevant information

Regards

Contributor

maximebiloe replied Nov 17, 2017

Hello @reho,

Is your theme compatible with the PrestaShop version you're using? Which PrestaShop version do you use?
You can look at this other PR which was meant to fix the native PrestaShop theme: #8142
You should open a ticket on PrestaShop bug-tracker to let the team know (I'm no longer a team member) about the issue and fix it if there is an error.
Don't forget to precise the PrestaShop version, the error in the browser console and all the other relevant information

Regards

This comment has been minimized.

Show comment
Hide comment
@reho

reho Nov 17, 2017

The develeoper says it's compatible to 1.7.2.4 but i bet he doesn't tested ever on greater than 1.7.0.
I'm using 1.7.2.4 but the same error was on 1.7.2.3

reho replied Nov 17, 2017

The develeoper says it's compatible to 1.7.2.4 but i bet he doesn't tested ever on greater than 1.7.0.
I'm using 1.7.2.4 but the same error was on 1.7.2.3

This comment has been minimized.

Show comment
Hide comment
@maximebiloe

maximebiloe Nov 17, 2017

Contributor

Ok, so you need to open a ticket on the bug-tracker.
I also ping @eternoendless to let him know

Contributor

maximebiloe replied Nov 17, 2017

Ok, so you need to open a ticket on the bug-tracker.
I also ping @eternoendless to let him know

This comment has been minimized.

Show comment
Hide comment
@reho

reho Nov 17, 2017

Ok, I managed to debug the bought theme. The developer did wrong passing the dataset variable when emiting updateCart. (and there were a lot other bugs...)

It solved my problems but if you think you may add an "if (typeof event.reason.cart !== 'undefined')" before that row, to avoid js error and stop javascript. Should i open a ticket for this or it's not necessary?

reho replied Nov 17, 2017

Ok, I managed to debug the bought theme. The developer did wrong passing the dataset variable when emiting updateCart. (and there were a lot other bugs...)

It solved my problems but if you think you may add an "if (typeof event.reason.cart !== 'undefined')" before that row, to avoid js error and stop javascript. Should i open a ticket for this or it's not necessary?

This comment has been minimized.

Show comment
Hide comment
@kpodemski

kpodemski Nov 17, 2017

Contributor

@reho even if you'll have checking for exisintg of this variable the feature behind would not work, i think that this theme developer should just fix his theme :)

Contributor

kpodemski replied Nov 17, 2017

@reho even if you'll have checking for exisintg of this variable the feature behind would not work, i think that this theme developer should just fix his theme :)

This comment has been minimized.

Show comment
Hide comment
@reho

reho replied Nov 17, 2017

@kpodemski it's true.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment