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

Checkout/Model/Session doesn't set the $quoteIdMask #1631

Closed
daanggc opened this issue Aug 8, 2015 · 4 comments
Closed

Checkout/Model/Session doesn't set the $quoteIdMask #1631

daanggc opened this issue Aug 8, 2015 · 4 comments

Comments

@daanggc
Copy link

daanggc commented Aug 8, 2015

In the file magento2/app/code/Magento/Checkout/Model/Session.php on line 256 there seem to be a small error.

$this->isQuoteMasked() us used to get the bool $isQuoteMasked but that should be $this->getIsQuoteMasked() or $this->isQuoteMasked

@vpelipenko
Copy link
Contributor

Hi @daancart2quote, there is a long discussion about naming convention here #935. We don't have strict rules how method should be named, only recommendations. In this case, I think, it is OK to use name 'isQuoteMasked', look at the latest @TexanHogman comment.

@daanggc
Copy link
Author

daanggc commented Aug 11, 2015

Hello @vpelipenko,

The issue is not the naming, the issue is that function isQuoteMasked() doesn't exits.
So, $this->isQuoteMasked() won't work. (AFAIK this should be labeled as a bug)

So to follow the example of @TexanHogman, the way to solve this is to add the following function:

    /**
     * @return bool|null
     */
    protected function isQuoteMasked()
    {
        return $this->isQuoteMasked;
    }

@vpelipenko
Copy link
Contributor

Probably, I missed something... but isQuoteMasked method exists in this class https://github.com/magento/magento2/blob/develop/app/code/Magento/Checkout/Model/Session.php#L504

@daanggc
Copy link
Author

daanggc commented Aug 25, 2015

It looks like my beta was just a version older... Sorry!
Anyway thanks for your time!

magento-team pushed a commit that referenced this issue Nov 7, 2017
[Helix] Update Changelog based on delivered scope
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants