Skip to content

Add failsafe to items.phtml#13086

Merged
magento-team merged 5 commits into
magento:2.2-developfrom
samgranger:patch-3
Jan 16, 2018
Merged

Add failsafe to items.phtml#13086
magento-team merged 5 commits into
magento:2.2-developfrom
samgranger:patch-3

Conversation

@samgranger

@samgranger samgranger commented Jan 10, 2018

Copy link
Copy Markdown
Contributor

Description

We received a warning that $exist is undefined - not entirely sure what the exact scenario was but adding this as a failsafe.

I have made sure that $exist is defined (null) in the 'default' and 'other' case.

Contribution checklist

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All new or changed code is covered with unit/integration tests (if applicable)
  • All automated tests passed successfully (all builds on Travis CI are green)

We got a warning that $exist is undefined - not entirely sure what the exact scenario was but adding this as a failsafe.

Added as default fallback and 'other'.
break;

default:
$exist = null;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Rather than adding the default options here what about adding an isset check as part of the if wrapping the content display.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Changed as requested. Your suggestion is a neater solution.

@samgranger samgranger left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I have removed the extra declarations for $exist and have added an isset check to the if statement as requested.

break;

default:
$exist = null;

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Changed as requested. Your suggestion is a neater solution.

@dmanners

Copy link
Copy Markdown
Contributor

@samgranger thank you for the quick update. I will process this pr now.

This should work if the variable is set
And the variable is true or 1 or greater
@magento-team magento-team merged commit d7dc6a9 into magento:2.2-develop Jan 16, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants