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.

Sign up for GitHub

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

Jump to bottom

MAGECLOUD-3866: Warm up functionality does not work if no store with 'default' code #560

Merged
merged 16 commits into from Jul 31, 2019
Merged

MAGECLOUD-3866: Warm up functionality does not work if no store with 'default' code #560

merged 16 commits into from Jul 31, 2019

Conversation

oshmyheliuk
Copy link
Contributor

@oshmyheliuk oshmyheliuk commented Jul 9, 2019 •
edited

Description

Fixed an issue when store with code 'default' doesn't exist or different store assigned as the default.

Fixed Issues (if relevant)

https://magento2.atlassian.net/browse/MAGECLOUD-3866

Related PR

magento/magento-cloud-components#5

Manual testing scenarios

https://jira.corp.magento.com/browse/MAGECLOUD-85

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)

@oshmyheliuk oshmyheliuk mentioned this pull request Jul 9, 2019
Merged
@shiftedreality shiftedreality added Progress: review PR/Issue status Progress: on hold PR/issue status labels Jul 9, 2019
@oshmyheliuk oshmyheliuk added the Release: 2002.0.21 ECE-Tools Release label Jul 9, 2019
@oshmyheliuk oshmyheliuk added this to Dev in progress in Backlog via automation Jul 9, 2019
@oshmyheliuk oshmyheliuk removed the Progress: on hold PR/issue status label Jul 9, 2019
NadiyaS
NadiyaS previously approved these changes Jul 9, 2019
View reviewed changes
NadiyaS
NadiyaS reviewed Jul 9, 2019
View reviewed changes
src/Util/UrlManager.php
@@ -154,7 +154,7 @@ public function getBaseUrl(): string
{
if ($this->baseUrl === null) {
try {
$process = $this->shell->execute('php bin/magento config:show:store-url default');
$process = $this->shell->execute('php bin/magento config:show:default-url');

$this->baseUrl = $process->getOutput();
} catch (ShellException $e) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Message in exception need to change to avoid confusion.
e.g. "Can't fetch base URL using command config:show:default-url. Instead URL from routes will be used.

Backlog automation moved this from Dev in progress to Review in progress Jul 10, 2019
NadiyaS
NadiyaS previously approved these changes Jul 10, 2019
View reviewed changes
bbatsche
bbatsche previously approved these changes Jul 10, 2019
View reviewed changes
Backlog automation moved this from Review in progress to QA in progress Jul 10, 2019
@YPyltiai YPyltiai mentioned this pull request Jul 11, 2019
Closed
@mveeramneni mveeramneni added Progress: testing in progress PR/issue status and removed Progress: review PR/Issue status labels Jul 11, 2019
@mveeramneni
Copy link

Waiting for magento/magento-cloud-components#5 to be merged

@oshmyheliuk
Merge remote-tracking branch 'origin/develop' into MAGECLOUD-3866
791735a 
# Conflicts:
#	src/Test/Unit/Util/UrlManagerTest.php
#	src/Util/UrlManager.php
@oshmyheliuk oshmyheliuk dismissed stale reviews from bbatsche and NadiyaS via 791735a July 17, 2019 17:17
Backlog automation moved this from QA in progress to Review in progress Jul 17, 2019
@mveeramneni mveeramneni requested review from shiftedreality and removed request for NadiyaS July 29, 2019 14:43
shiftedreality
shiftedreality suggested changes Jul 29, 2019
View reviewed changes
composer.json Outdated
"magento/magento-cloud-components": "^1.0",


"magento/framework": "*"
Copy link
Member

Choose a reason for hiding this comment

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

Remove excess lines

@oshmyheliuk oshmyheliuk requested a review from NadiyaS July 29, 2019 15:15
shiftedreality
shiftedreality previously approved these changes Jul 29, 2019
View reviewed changes
bbatsche
bbatsche previously approved these changes Jul 29, 2019
View reviewed changes
Backlog automation moved this from Review in progress to QA in progress Jul 29, 2019
@shiftedreality shiftedreality added Progress: testing in progress PR/issue status and removed Progress: review PR/Issue status labels Jul 29, 2019
@shiftedreality shiftedreality removed their assignment Jul 29, 2019
@andriyShevtsov
Copy link
Contributor

QA approved

@oshmyheliuk oshmyheliuk dismissed stale reviews from bbatsche and shiftedreality via 84578de July 31, 2019 15:41
Backlog automation moved this from QA in progress to Review in progress Jul 31, 2019
Backlog automation moved this from Review in progress to QA in progress Jul 31, 2019
@shiftedreality shiftedreality added Progress: accept PR/issue status and removed Progress: testing in progress PR/issue status labels Jul 31, 2019
@shiftedreality shiftedreality merged commit 8a4610e into develop Jul 31, 2019
@ghost
Copy link

ghost commented Jul 31, 2019

Hi @oshmyheliuk, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.

Backlog automation moved this from QA in progress to Done Jul 31, 2019
@shiftedreality shiftedreality deleted the MAGECLOUD-3866 branch July 31, 2019 19:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bbatsche bbatsche bbatsche approved these changes

@shiftedreality shiftedreality shiftedreality approved these changes

@NadiyaS NadiyaS Awaiting requested review from NadiyaS

@hguthrie hguthrie Awaiting requested review from hguthrie

Assignees
No one assigned
Labels
Progress: accept PR/issue status Release: 2002.0.21 ECE-Tools Release
Projects
Backlog
Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@oshmyheliuk @mveeramneni @andriyShevtsov @bbatsche @shiftedreality @hguthrie @NadiyaS