https://www.redmine.org/https://www.redmine.org/favicon.ico?16793021292010-07-09T11:10:20ZRedmineRedmine - Patch #4665: Projects not calling EnabledModule callbacks in some caseshttps://www.redmine.org/issues/4665?journal_id=181132010-07-09T11:10:20ZFelix Schäfer
<ul></ul><p>Could you please give a way to reproduce any error this might cause?</p> Redmine - Patch #4665: Projects not calling EnabledModule callbacks in some caseshttps://www.redmine.org/issues/4665?journal_id=181572010-07-09T15:35:00ZEnrique Garcia
<ul></ul><p>Ok.</p>
<p>This is the current implementation:</p>
<pre>
def enabled_module_names=(module_names)
if module_names && module_names.is_a?(Array)
module_names = module_names.collect(&:to_s)
# remove disabled modules
enabled_modules.each {|mod| mod.destroy unless module_names.include?(mod.name)}
# add new modules
module_names.reject {|name| module_enabled?(name)}.each {|name| enabled_modules << EnabledModule.new(:name => name)}
else
enabled_modules.clear # this doesn't invoke callbacks
end
end
</pre>
<p>I believe there is no way to reproduce this in "vanilla" Redmine - it never does project.enabled_modules = nil.</p>
<p>But it is a problem for plugin development.</p>
<p>The way the method is constructed, it seems to say "I accept either a list of modules, or nil, and I do different things depending on that".</p>
<p>The "nil" part isn't used on redmine, I believe. It is allways a list of modules. (I suspect this isn't included on the tests either). I believe the code there is wrong because enabled_modules.clear eliminates the list of modules <em>without</em> invoking the corresponding callbacks. (eg before_destroy) on the modules.</p>
<p>The proposed solution is more straightforward - the code says "give me a list of modules. if you give me nil, I'll make it an empty list". And callbacks are allways called. No "nil special case". It is also shorter.</p>
<p>I was "bitten" by this code myself when developing a plugin - was confused by the "if", tried to use it, and module callbacks stopped working, and I went bonkers for 3 days.</p>
<p>By the way, I submitted this question to Eric via email. The code on the OP is his - I copy-pasted it from his e-mail. He asked me to open a ticket and assign it to him, and I did so. I didn't notice that he did not action on it until today.</p>
<p>If you need any more clarifications, just let me know.</p> Redmine - Patch #4665: Projects not calling EnabledModule callbacks in some caseshttps://www.redmine.org/issues/4665?journal_id=217422010-10-25T15:00:54ZEric Davis
<ul><li><strong>Assignee</strong> deleted (<del><i>Eric Davis</i></del>)</li></ul>