cross-solution / YAWIK

YAWIK is a web application. It can be used as an ATS applicant tracking system or as a jobboard.
https://yawik.org
MIT License
126 stars 67 forks source link

Wrong loading order of Modules generated by Core\Application #506

Closed TiSiE closed 6 years ago

TiSiE commented 6 years ago

https://github.com/cross-solution/YAWIK/blob/911e586f7b36bf2fbf6e09a6769279104f4ed19b/module/Core/src/Application.php#L72-L80

This will produce DependencyCheck errors, because additional modules will be loaded BEFORE the YAWIK modules.

In particular, when using yawik/JobsByMail, an exception occurs, because JobsByMail requires the Jobs module.

To get the ordering right, this might be a better approach:

    public static function generateModuleConfiguration($loadModules=[]) 
     { 
         $modules = array_merge( 
             static::getRequiredModules(), 
             $loadModules,
             static::scanAdditionalModule() 
         ); 

         return $modules; 
     } 
TiSiE commented 6 years ago

I forgot something: https://github.com/cross-solution/YAWIK/blob/911e586f7b36bf2fbf6e09a6769279104f4ed19b/module/Core/src/Application.php#L158-L163

This code was meant to allow to remove modules from the list prior to loading, but it is completely useless in this position, because the list of all modules is not generated at this point in the code execution.

kilip commented 6 years ago

@TiSiE I will try to use DependencyIndicatorInterface in Organizations module, I hope this can make zf3 component installer making modules config in spesific order in yawik/standard:

/**
 * Bootstrap class of the organizations module
 */
class Module implements BootstrapListenerInterface, DependencyIndicatorInterface
{
    public function getModuleDependencies()
    {
        return [
            'Core',
            'Auth',
        ];
    }
}

I hope this can fix issue like #507 in future, because it will raise an error if Organization module loaded before Core or Auth module

I will fix that ordering issue in next pull request.

This code was meant to allow to remove modules from the list prior to loading, but it is completely useless in this position, because the list of all modules is not generated at this point in the code execution.

What about this issue? Can we delete this scanAdditionalModule function or you have any suggestion to fix this?

TiSiE commented 6 years ago

What about this issue? Can we delete this scanAdditionalModule function or you have any suggestion to fix this?

See yawik/standard#3