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

Upgrade an uploaded module when already installed #5909

Merged
merged 2 commits into from Jul 27, 2016

Conversation

@Quetzacoalt91
Copy link
Member

commented Jul 26, 2016

Questions Answers
Branch? develop
Description? Currently, when we upload a module which is already installed, an error is thrown by the ModuleManager. In case of faillure, the ModuleController cleans the uploaded files, which are ... the module previously installed. This PR fixes the issue by launching an upgrade instead.
At the same time, we introduce a ModuleZipManager to handle in only one place all the tasks about module name detection and saving.
Type? bug fix
Category? CO
BC breaks? Nope
Deprecations? Nope
Fixed ticket? BOOM-637
How to test? Upload an already installed module. You should not have the error 'this module is already installed' anymore.
@mickaelandrieu

View changes

src/Adapter/Module/ModuleZipManager.php Outdated
private $finder;
private $translator;

public function __construct(TranslatorInterface $translator)

This comment has been minimized.

Copy link
@mickaelandrieu

mickaelandrieu Jul 26, 2016

Member

can you put all dependencies as arguments of constructor ?

pros: easier to test.
cons: no cons ?

This comment has been minimized.

Copy link
@Quetzacoalt91

Quetzacoalt91 Jul 26, 2016

Author Member

One of them, finder, does not exist as a service.

This comment has been minimized.

Copy link
@mickaelandrieu

mickaelandrieu Jul 26, 2016

Member

you dont need to declare it as a service, only pass the FQCN :)

@mickaelandrieu

View changes

src/Adapter/Module/ModuleZipManager.php Outdated
$directories = $this->finder->directories()
->in($sandboxPath)
->depth('== 0')
->exclude(['__MACOSX'])

This comment has been minimized.

Copy link
@mickaelandrieu

mickaelandrieu Jul 26, 2016

Member

php-cs-fix :p

@Quetzacoalt91 Quetzacoalt91 force-pushed the Quetzacoalt91:BOOM-637 branch Jul 26, 2016

@Quetzacoalt91 Quetzacoalt91 force-pushed the Quetzacoalt91:BOOM-637 branch to 5ae3320 Jul 26, 2016

@mickaelandrieu

This comment has been minimized.

Copy link
Member

commented Jul 26, 2016

Good job.

Dont forget to update your commit label ;)

Mickaël

@Quetzacoalt91 Quetzacoalt91 force-pushed the Quetzacoalt91:BOOM-637 branch to cd02c7f Jul 26, 2016

@vTerenti

This comment has been minimized.

Copy link
Contributor

commented Jul 27, 2016

Status : 'QA approved'

Best regards!

@mickaelandrieu mickaelandrieu merged commit 122f9a9 into PrestaShop:develop Jul 27, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@mickaelandrieu

This comment has been minimized.

Copy link
Member

commented Jul 27, 2016

Thank you @Quetzacoalt91

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.