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

Add active state condition befor adding controllers to database: #12

Merged

Conversation

bissie
Copy link

@bissie bissie commented Dec 4, 2018

When fixing modules that has metadata Version > 2 and define controllers, this controllers are added to aModulesControllers in oxconfig.
This behavior is correct for active modules but when adding controllers for inactive modules, than activating it lead to an exception, as such controllers are already configured. In a second step activation works.

Steps to reproduce:
Set up shop including oxid console.
Test activation/deactivation for paypal
Run php vendor/bin/oxid modules:fix --all from installation path
Try to activate paypal module again.

BTW, changes looks strange but simple surround code with if($module->isActive()){}

When fixing modules that has metadata Version > 2 and define controllers, this controllers are added to aModulesControllers in oxconfig.
This behavior is correct for active modules but when adding controllers for inactibe modules, than activating it lead to an exception, as such controllers are already configured. In a second step activation works.

Steps to reproduce:
Set up shop including oxid console.
Test activation/deactivation for paypal
Run `php vendor/bin/oxid modules:fix --all` from installation path
Try to activate paypal module again.
@alfredbez
Copy link
Contributor

Thank you @bissie for the fix!

BTW, changes looks strange but simple surround code with if($module->isActive()){}

Diff looks a lot cleaner when you ignore whitespace changes: https://github.com/OXIDprojects/oxid-console/pull/12/files?w=1


It would be cool to have more tests for bugs like this.

@keywan-ghadami-oxid
Copy link
Contributor

this was also fixed in
https://github.com/OXIDprojects/oxid-module-internals
the fix:states command from the console will be removed in the next major version because the functionality to fix modules will be maintained in that module (to be available in all console implementations)

@keywan-ghadami-oxid keywan-ghadami-oxid merged commit 390893f into master Dec 5, 2018
@alfredbez alfredbez deleted the fixControllerDuplicationForInactiveModules branch December 6, 2018 08:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants