Navigation Menu

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

MO: Viewed products module now fully working #3

Closed
wants to merge 1 commit into from

Conversation

kpodemski
Copy link

Well, module at current state doesn't work at all, this PR makes it working, you can display viewed products anywhere you want as this is a widget

Well, module at current state doesn't work at all, this PR makes it working, you can display viewed products anywhere you want as this is a widget
Copy link
Member

@Quetzacoalt91 Quetzacoalt91 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your work @kpodemski. I guess the fix is easier than you think. 😉

@@ -168,8 +168,8 @@ public function getCacheId($name = null)

public function renderWidget($hookName = null, array $configuration = [])
{
if ('displayProductButtons' === $hookName) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(I was writing another review, then I found the actual issue)

On PrestaShop 1.7.1.0, we declared an alias for the hook 'displayProductButtons'.
The only you have to replace in this module is this line with the following one:

if (in_array($hookName, array('displayProductButtons', 'displayProductAdditionalInfo')))

@kpodemski
Copy link
Author

@Quetzacoalt91 yeah but if you want to display module in leftColumn you can't because of this -_-

@Quetzacoalt91
Copy link
Member

Quetzacoalt91 commented Sep 19, 2017

Are you sure? :)
I just added the registerHook('displayLeftColumn'), and tried to look at the category page with my change:
capture du 2017-09-19 15-07-15

@kpodemski
Copy link
Author

@Quetzacoalt91 with dev_mode set to true it will return a notice as you want to have productId when it's not needed :) this is why there's else condition in my code and i'm not looking for a vars from $configuration which is not really a good way IMO

$products = $this->getViewedProducts($this->context->controller->getProduct()->id);
} else {
$products = $this->getViewedProducts(false);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For this check, I was also using the configuration variable.

$id_product = isset($configuration['product']['id_product']) ? $configuration['product']['id_product'] : false;
$products = $this->getViewedProducts($id_product);

@Quetzacoalt91
Copy link
Member

Quetzacoalt91 commented Sep 19, 2017

I do not understand why relying on the current function param is a bad idea for you. IMO it is even worse to look outside (even if the property you look after exists from PS 1.5).

@kpodemski
Copy link
Author

Yeah well we can argue about that, the thing is that we worked this out and module needs to be changed because right now it doesn't work :D

iqit-commerce added a commit to iqit-commerce/ps_viewedproduct that referenced this pull request Oct 25, 2017
In Prestashop 1.7.1 displayProductButtons hook was renamed to displayProductAdditionalInfo. This change caused viewed products not working at all. It also allow to hook module non product pages. Based on @kpodemksi PR PrestaShop#3 which is not merged yet
@idem84
Copy link

idem84 commented Nov 23, 2017

Hello, this code will fix this module, with that fix module work great on 1.7.2.4:

public function renderWidget($hookName = null, array $configuration = [])
{
    if (in_array($hookName, array('displayProductButtons', 'displayProductAdditionalInfo', 'displayFooterProduct'))) {
        if ('product' === $this->context->controller->php_self) {
            $id_product = $this->context->controller->getProduct()->id;
            $vieved = explode(',', $this->context->cookie->viewed);
            if (!in_array($id_product, $vieved)) {
                $this->addViewedProduct($id_product);
                return;
            }
        }
    }

    if (!isset($this->context->cookie->viewed) || empty($this->context->cookie->viewed)) {
        return;
    }

    $variables = $this->getWidgetVariables($hookName, $configuration);

    if (empty($variables)) {
        return false;
    }

    $this->smarty->assign($variables);

    return $this->fetch($this->templateFile);
}

@jolelievre
Copy link
Contributor

Hello, I am sorry I worked on this issue but I didn't have seen your PR
The bug will be fixed with this PR #7
Feel free to comment it

@Quetzacoalt91
Copy link
Member

Hi @kpodemski,

As we merged the PR #4, which also fix that issue, I'm closing this one.

FYI the version v1.1.0 of the module will be deployed tomorrow.

Regards

@Juiliard
Copy link

Hello, this code will fix this module, with that fix module work great on 1.7.2.4:

public function renderWidget($hookName = null, array $configuration = [])
{
    if (in_array($hookName, array('displayProductButtons', 'displayProductAdditionalInfo', 'displayFooterProduct'))) {
        if ('product' === $this->context->controller->php_self) {
            $id_product = $this->context->controller->getProduct()->id;
            $vieved = explode(',', $this->context->cookie->viewed);
            if (!in_array($id_product, $vieved)) {
                $this->addViewedProduct($id_product);
                return;
            }
        }
    }

    if (!isset($this->context->cookie->viewed) || empty($this->context->cookie->viewed)) {
        return;
    }

    $variables = $this->getWidgetVariables($hookName, $configuration);

    if (empty($variables)) {
        return false;
    }

    $this->smarty->assign($variables);

    return $this->fetch($this->templateFile);
}

works on PS 1.7.8.7 (after upgrade from 1.7.6.5)

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