search

OXIDprojects / oxid_modules_config

OXID Module Configuration Im-/Exporter module by OXID Professional Services
GNU General Public License v3.0
3 stars 9 forks source link

Update ConfigImport #44

Closed nitinsinghoxidesales closed 3 years ago

nitinsinghoxidesales commented 4 years ago

We have noticed some issues with OXID Shop 6.2.X Issues:

  1. Config import is not enabling a module if it's deactivated in DB.
  2. Config import is not creating sub-shops through configuration files.

Changes:

  1. Checking whether module is already active before deactivating the module.

$oModuleStateFixer->deactivate($oModule)

  1. Creating sub-shops through defined configuration files.

TODO

  1. Need to test of Shop 6.1 (Currently checks are failing for it)
  2. Add some more UT test cases
nitinsinghoxidesales commented 4 years ago

Hi @keywan-ghadami-oxid @alfredbez - Can you please check this PR and share your inputs if you think we can improve it more?

Note: Still there are 2 TODO points need to implement.

nitinsinghoxidesales commented 4 years ago

@alfredbez @keywan-ghadami-oxid - Can you please review and merge this long pending PR?

Please let me know if you have any suggestion on this.

alfredbez commented 4 years ago

I can't do this in the next weeks

nitinsinghoxidesales commented 4 years ago

I can't do this in the next weeks

:( May be @keywan-ghadami-oxid would like to review and merge?

alfredbez commented 4 years ago

Looks like there are still some open todos from your initial comment on this PR, did you already checked them?

  1. Need to test of Shop 6.1 (Currently checks are failing for it)
  2. Add some more UT test cases
nitinsinghoxidesales commented 4 years ago

Yes, I have checked today.

  1. Shop 6.1 is now green. https://github.com/OXIDprojects/oxid_modules_config/pull/44/checks. Earlier it was failing because of DB connection issue.
  2. Adding more UT test cases - I wanted to add some more but we have some time constraint. It is working in the checks with earlier UT. Do you suggest to add more UT test cases?
nitinsinghoxidesales commented 3 years ago

@keywan-ghadami-oxid - Can you please review it and merge it?