ellisv / oxid-console

[UNMAINTAINED] OXID Console is php console application for OXID Shop. It provides an API for writing various commands. We have shipped very common for every oxid shop project commands out of the box.
MIT License
23 stars 17 forks source link

fix fixstates command can not fix new modules on subshops #32

Open keywan-ghadami-oxid opened 8 years ago

ellisv commented 8 years ago

Hello @keywan-ghadami-oxid,

Can you please explain me what do you mean by unable to fix new modules on subshops? Or yet better, give me your use case. Currently _getAvailableModuleIds() should return all modules ids independently on which shop this gets called.

keywan-ghadami-oxid commented 8 years ago

@EllisV it's all about that workaround: // We are calling getModulesFromDir() because we want to refresh // the list of available modules. This is a workaround for OXID // bug. getModulesFromDir loads modules from directory and adds module information to oxid oxconfig database. But it must be called on the right subshop. to test it you can add a new module to the modules folder and then run "oxid fix:states -a" it will give some errormessages in the subshops

ellisv commented 8 years ago

@keywan-ghadami-oxid Alright. I'll test it and evaluate the problem on a weekend. Stay patient! (:

keywan-ghadami-oxid commented 8 years ago

thank you! BTW it's nothing urgent or even important just a nice to have thing in automated deployments

ellisv commented 8 years ago

Can not really test it against EE version as I do not have access to it but I take your word that there is a bug on this. One little minor improvement that I want you to do is that _parseModuleIds() shouldn't be done on all shops. It can be done on just root shop. Just call _getModulesFromDir() on every shop loop entry. What do you think?

keywan-ghadami-oxid commented 8 years ago

one important thing is that _getModulesFromDir() has also to be called before accessing that aModulePaths variable because it adds the new modules. The second thing to think about is that aModulePaths can also contain different corrupt entries for each subshop, i need that information to remove not existing modules in subshops.

BTW: We currently thinking about back porting some stuff from new shopversions so we can unify oxidconsole1.1 and 1.2. We are working in both versions on the fixstate command to remove old modules, warn on error caused by errors in module meta data, giving information about what was fixed, and if modules will become active.

ellisv commented 8 years ago

@keywan-ghadami-oxid I am aware of that calling _getModulesFromDir() on every shop oxConfig is the part of the fix. But within this pull-request you are also doing _parseModuleIds() on every shop oxConfig which is unnecessary. _parseModuleIds() is a command helper for getting ids of modules to fix - passed by an argument(-s) or get all if --all flag is specified. You can still call _getModulesFromDir() on every loop entry. Or am I overseeing something?

Also there were quite a few requests on having OXID console as composer package which I currently plan to have at 1.3. On this release I also plan to unify both 1.1 and 1.2 and provide clear migration path.

keywan-ghadami-oxid commented 8 years ago

you are right _parseModuleIds() is a little bit much here, but if you only call getModulesFromDir within the loop it would not update the variable $aModuleIds (which not yet a problem, but might be needed for other stuff i am currently working on).