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

Error on show product without pricing #11400

Merged
merged 2 commits into from May 13, 2020
Merged

Error on show product without pricing #11400

merged 2 commits into from May 13, 2020

Conversation

oallain
Copy link
Member

@oallain oallain commented Apr 27, 2020

Q A
Branch? 1.6 and after
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Related tickets fixes #11399
License MIT

Behat :

@ui
Scenario: Viewing price block without channel enable
    Given this product is unavailable in "United States" channel
    When I access "Iron shield" product page
    Then I should see product name "Iron shield"
    And I should see the product in neither channel
    And I should not see price for channel "United States"

Before fix step Then I should see product name "Iron shield" throw error in cause of error 500 in page.

After fix, all steps are green ✔️

@oallain oallain requested a review from a team as a code owner April 27, 2020 08:44
@oallain oallain changed the base branch from master to 1.6 April 27, 2020 08:50
Copy link
Member

@lchrusciel lchrusciel left a comment

Choose a reason for hiding this comment

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

Hello Olivier,

thanks a lot for your contribution. Can you, however, cover this case with behat, please? Just to avoid regression in the future

@Zales0123 Zales0123 added the Potential Bug Potential bugs or bugfixes, that needs to be reproduced. label Apr 28, 2020
@probot-autolabeler probot-autolabeler bot added the Admin AdminBundle related issues and PRs. label Apr 30, 2020
/**
* @Then I should not see price for channel :channelName
*/
public function iShouldNotSeePriceForChannel(string $channelName)
Copy link
Member

Choose a reason for hiding this comment

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

Missing : void

@@ -22,6 +22,10 @@ public function getPriceForChannel(string $channelName): string
/** @var NodeElement $priceForChannel */
$channelPriceRow = $this->getDocument()->find('css', sprintf('#pricing tr:contains("%s")', $channelName));

if (null === $channelPriceRow) {
return '';
Copy link
Member

Choose a reason for hiding this comment

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

Maybe null? It clearly says that there is no price rather than there is something but empty 💃

@@ -37,6 +37,17 @@ public function hasChannel(string $channelName): bool
return false;
}

public function countChannels(): int
{
if (! $this->hasElement('channels')) {
Copy link
Member

@Zales0123 Zales0123 May 7, 2020

Choose a reason for hiding this comment

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

Extra space after !

@pamil pamil changed the base branch from 1.6 to 1.7 May 13, 2020 12:26
@pamil
Copy link
Contributor

pamil commented May 13, 2020

The base of this pull-request was changed, you need fetch and reset your local branch
if you want to add new commits to this pull request. Reset before you pull, else commits
may become messed-up.

Unless you added new commits (to this branch) locally that you did not push yet,
execute git fetch origin && git reset "fix-11399" to update your local branch.

Feel free to ask for assistance when you get stuck 👍

@probot-autolabeler probot-autolabeler bot added Documentation Documentation related issues and PRs - requests, fixes, proposals. Maintenance CI configurations, READMEs, releases, etc. labels May 13, 2020
@pamil pamil changed the base branch from 1.7 to master May 13, 2020 12:31
@pamil pamil changed the base branch from master to 1.7 May 13, 2020 12:31
@pamil pamil merged commit 205037d into Sylius:1.7 May 13, 2020
@pamil
Copy link
Contributor

pamil commented May 13, 2020

Thank you, @oallain! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Admin AdminBundle related issues and PRs. Documentation Documentation related issues and PRs - requests, fixes, proposals. Maintenance CI configurations, READMEs, releases, etc. Potential Bug Potential bugs or bugfixes, that needs to be reproduced.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BO-Product] Error on show product without pricing
4 participants