Skip to content
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

Deleting item from cart problem #2433

Closed
uksitebuilder opened this issue Dec 4, 2014 · 14 comments
Closed

Deleting item from cart problem #2433

uksitebuilder opened this issue Dec 4, 2014 · 14 comments

Comments

@uksitebuilder
Copy link
Contributor

This is the third time I have posted this Daniel

Please check it before you jump to conclusions

This is from a stock install of the latest master with no modifications added

  1. SEO URls enabled in Admin settings
  2. Add an item to cart
  3. View the cart
  4. Delete the item

Result - Item is deleted in the mini cart in header but remains in the shopping cart main body.

Reason - See my Pull request ( #2312 ) that you dismissed out of hand calling it stupid. If you have a different/better fix, by all means, go with it, but don't you dare call me stupid again without just cause!

@mhcwebdesign
Copy link
Collaborator

I just tested it, and I can't reproduce your error, works fine here using the latest Firefox web browser on Linux. What web browser are you using?

@danielkerr
Copy link
Member

because your using a extension that changes the cart url

@danielkerr
Copy link
Member

it was a stupid commit because the only way it works is looking for the word cart in the url.

what about if some one used another language and decided not to have the word cart int he url.

@danielkerr
Copy link
Member

its pointless using seo urls on the cart page! google does not need to index an empty cart page for your seo rankings.

thsis code int he common.js is the issue but your code is not the solution.

            if (getURLVar('route') == 'checkout/cart' || getURLVar('route') == 'checkout/checkout') {
                location = 'index.php?route=checkout/cart';
            } else {
                $('#cart > ul').load('index.php?route=common/cart/info ul li');
            }

@danielkerr
Copy link
Member

It was opencart-russia

Idiot has also left a big possible security issue right int he middle of the code!

bdff397

@opencart-russia
Copy link
Contributor

Daniel, you did not want to take the commits, and said not to write SEO stuff more. And who's the idiot? It was necessary to understand and easy to fix, and not to leave everything as is. Idiot!

@danielkerr
Copy link
Member

i only accepted your commit because i thought you knew what you were doing.

@uksitebuilder
Copy link
Contributor Author

@mhcwebdesign doesn't matter now, I see a recent commit has remove the URL_Alias entries for cart, checkout etc.

I am using FireFox latest on Windows. Also got the same with IE on Windows.

@danijelGombac
Copy link
Contributor

I add this in header.php

if (isset($this->request->get['route'])) {
$data['route'] = $this->request->get['route'];
} else {
$data['route'] = '';
}

and in header..tpl

cur_route = '';

then replace

if (getURLVar('route') == 'checkout/cart' || getURLVar('route') == 'checkout/checkout') {

with

if (cur_route == 'checkout/cart' || cur_route == 'checkout/checkout') {

and work just fine.

@opencart-russia
Copy link
Contributor

@GomDani
the wrong solution you sent. Here's how you can make

    } else {
        var query = String(document.location.pathname).split('/');
        if (query[query.length - 1] == 'cart') value['route'] = 'checkout/cart';
        if (query[query.length - 1] == 'checkout') value['route'] = 'checkout/checkout';

        if (value[key]) {
            return value[key];
        } else {
            return '';
        }
    }

@ravilrrr
Copy link

ravilrrr commented Dec 5, 2014

@danielkerr, big possible security issue! Where?. Show all, and we will correct, if you can't! Why was it necessary to remove the right direction for seo ? Daniel, it is foolish on your part to go back commits.

https://github.com/opencart-russia/opencart/blob/master/upload/catalog/controller/common/seo_url.php

show me please where and let's fleshing out as it should be! Here 1000 developers and anyone can help, do not blindly reject help.

@OpenCartAddons
Copy link
Contributor

@ravilrrr The security hole I believe @danielkerr is referring to is that fact that @opencart-russia failed to escape the data during a sql query, which leaves the system open to attack.

@ravilrrr
Copy link

ravilrrr commented Dec 6, 2014

@OpenCartAddons
well received Daniel wrong code from opencart-russia. Why to return everything back when it was possible to correct properly ?. Seo url alias for all route this was a good start ...

@OpenCartAddons
Copy link
Contributor

@ravilrrr I agree, replacing route in all URLs with a clean SEO version is ideal. I'm sure @danielkerr isn't against this idea either.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants