From 07caf3d4b2431f2858c250972537a612650ac23d Mon Sep 17 00:00:00 2001 From: Rodrigo Souto Date: Mon, 15 Jul 2013 19:12:58 -0300 Subject: [PATCH] [macro-support-review] Refactoring macros infra-structure --- app/helpers/application_helper.rb | 26 +++++++------------------- app/helpers/macros_helper.rb | 58 ++++++++++++++++++++++++++++------------------------------ app/models/environment.rb | 4 ---- app/views/shared/tiny_mce.rhtml | 20 ++++++++++---------- lib/noosfero/plugin.rb | 15 ++++++--------- lib/noosfero/plugin/macro.rb | 44 ++++++++++++++++++++++++++++++++++++++++++++ lib/noosfero/plugin/manager.rb | 12 +++++++++++- test/unit/application_helper_test.rb | 45 ++++++++++++++++++++++++--------------------- test/unit/macros_helper_test.rb | 136 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---------------------------------------------------------------- test/unit/plugin_manager_test.rb | 26 ++++++++++++++++++++++++++ test/unit/plugin_test.rb | 29 ----------------------------- 11 files changed, 228 insertions(+), 187 deletions(-) create mode 100644 lib/noosfero/plugin/macro.rb diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index c4c95e0..9738f29 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -1413,29 +1413,17 @@ module ApplicationHelper def convert_macro(html, source) doc = Hpricot(html) - while element = doc.search('.macro').first - macro_name = element['data-macro'] - method_name = "macro_#{macro_name}" - attrs = collect_macro_attributes(element) - plugin_instance = Environment.macros[environment.id][method_name] - if plugin_instance - result = plugin_instance.send(method_name, attrs, element.inner_html, source) - element.inner_html = result.kind_of?(Proc) ? self.instance_eval(&result) : result - element['class'] = "parsed-macro #{macro_name}" - else - element.inner_html = _("Unsupported macro %s!") % macro_name - element['class'] = "failed-macro #{macro_name}" - end - attrs.each {|key, value| element.remove_attribute("data-macro-#{key}")} + #TODO This way is more efficient but do not support macro inside of + # macro. You must parse them from the inside-out in order to enable + # that. + doc.search('.macro').each do |macro| + macro_name = macro['data-macro'] + result = @plugins.parse_macro(macro_name, macro, source) + macro.inner_html = result.kind_of?(Proc) ? self.instance_eval(&result) : result end doc.html end - def collect_macro_attributes(element) - element.attributes.to_hash.select {|key, value| key[0..10] == 'data-macro-'}. - inject({}){|result, a| result.merge({a[0][11..-1] => a[1]})}.with_indifferent_access - end - def default_folder_for_image_upload(profile) default_folder = profile.folders.find_by_type('Gallery') default_folder = profile.folders.find_by_type('Folder') if default_folder.nil? diff --git a/app/helpers/macros_helper.rb b/app/helpers/macros_helper.rb index 73ad966..32bb648 100644 --- a/app/helpers/macros_helper.rb +++ b/app/helpers/macros_helper.rb @@ -1,34 +1,30 @@ module MacrosHelper def macros_in_menu - Environment.macros[@environment.id].reject{ |macro_name, plugin_instance| macro_configuration(macro_name)[:icon_path] } + @plugins.dispatch(:macros).reject{ |macro| macro.configuration[:icon_path] } end def macros_with_buttons - Environment.macros[@environment.id].reject{ |macro_name, plugin_instance| !macro_configuration(macro_name)[:icon_path] } - end + @plugins.dispatch(:macros).reject{ |macro| !macro.configuration[:icon_path] } - def macro_configuration(macro_name) - plugin_instance = Environment.macros[@environment.id][macro_name] - plugin_instance.send("config_#{macro_name}") end - def macro_title(macro_name) - macro_configuration(macro_name)[:title] || macro_name.to_s.humanize + def macro_title(macro) + macro.configuration[:title] || macro.name.humanize end - def generate_macro_config_dialog(macro_name) - if macro_configuration(macro_name)[:skip_dialog] - "function(){#{macro_generator(macro_name)}}" + def generate_macro_config_dialog(macro) + if macro.configuration[:skip_dialog] + "function(){#{macro_generator(macro)}}" else "function(){ - jQuery('
'+#{macro_configuration_dialog(macro_name).to_json}+'
').dialog({ - title: #{macro_title(macro_name).to_json}, + jQuery('
'+#{macro_configuration_dialog(macro).to_json}+'
').dialog({ + title: #{macro_title(macro).to_json}, modal: true, buttons: [ {text: #{_('Ok').to_json}, click: function(){ tinyMCE.activeEditor.execCommand('mceInsertContent', false, - (function(dialog){ #{macro_generator(macro_name)} })(this)); + (function(dialog){ #{macro_generator(macro)} })(this)); jQuery(this).dialog('close'); }}, {text: #{_('Cancel').to_json}, click: function(){jQuery(this).dialog('close');}} @@ -40,9 +36,9 @@ module MacrosHelper def include_macro_js_files plugins_javascripts = [] - Environment.macros[environment.id].map do |macro_name, plugin_instance| - if macro_configuration(macro_name)[:js_files] - macro_configuration(macro_name)[:js_files].map { |js| plugins_javascripts << plugin_instance.class.public_path(js) } + @plugins.dispatch(:macros).map do |macro| + if macro.configuration[:js_files] + macro.configuration[:js_files].map { |js| plugins_javascripts << macro.plugin.public_path(js) } end end javascript_include_tag(plugins_javascripts, :cache => 'cache/plugins-' + Digest::MD5.hexdigest(plugins_javascripts.to_s)) unless plugins_javascripts.empty? @@ -50,9 +46,9 @@ module MacrosHelper def macro_css_files plugins_css = [] - Environment.macros[environment.id].map do |macro_name, plugin_instance| - if macro_configuration(macro_name)[:css_files] - macro_configuration(macro_name)[:css_files].map { |css| plugins_css << plugin_instance.class.public_path(css) } + @plugins.dispatch(:macros).map do |macro| + if macro.configuration[:css_files] + macro.configuration[:css_files].map { |css| plugins_css << macro.plugin.public_path(css) } end end plugins_css.join(',') @@ -60,29 +56,31 @@ module MacrosHelper protected - def macro_generator(macro_name) - if macro_configuration(macro_name)[:generator] - macro_configuration(macro_name)[:generator] + def macro_generator(macro) + if macro.configuration[:generator] + macro.configuration[:generator] else - macro_default_generator(macro_name) + macro_default_generator(macro) end + end - - def macro_default_generator(macro_name) + + def macro_default_generator(macro) code = "var params = {};" - configuration = macro_configuration(macro_name) + macro_name = macro.name.undescore + configuration = macro_configuration(macro) configuration[:params].map do |field| code += "params.#{field[:name]} = jQuery('*[name=#{field[:name]}]', dialog).val();" end code + " - var html = jQuery('
'+#{macro_title(macro_name).to_json}+'
')[0]; + var html = jQuery('
'+#{macro_title(macro).to_json}+'
')[0]; for(key in params) html.setAttribute('data-macro-'+key,params[key]); return html.outerHTML; " end - def macro_configuration_dialog(macro_name) - macro_configuration(macro_name)[:params].map do |field| + def macro_configuration_dialog(macro) + macro.configuration[:params].map do |field| label_name = field[:label] || field[:name].to_s.humanize case field[:type] when 'text' diff --git a/app/models/environment.rb b/app/models/environment.rb index e372e2e..97aa9f3 100644 --- a/app/models/environment.rb +++ b/app/models/environment.rb @@ -16,10 +16,6 @@ class Environment < ActiveRecord::Base filename end - class << self - attr_accessor :macros - end - PERMISSIONS['Environment'] = { 'view_environment_admin_panel' => N_('View environment admin panel'), 'edit_environment_features' => N_('Edit environment features'), diff --git a/app/views/shared/tiny_mce.rhtml b/app/views/shared/tiny_mce.rhtml index e68c8dd..1ec8bc5 100644 --- a/app/views/shared/tiny_mce.rhtml +++ b/app/views/shared/tiny_mce.rhtml @@ -11,8 +11,8 @@ <% else %> first_line = "print,separator,copy,paste,separator,undo,redo,separator,search,replace,separator,forecolor,fontsizeselect,formatselect" second_line = "bold,italic,underline,strikethrough,separator,bullist,numlist,separator,justifyleft,justifycenter,justifyright,justifyfull,separator,link,unlink,image,table,separator,cleanup,code,macros" - <% macros_with_buttons.each do |macro_name, plugin_instance| %> - second_line += ',<%=macro_name %>' + <% macros_with_buttons.each do |macro| %> + second_line += ',<%=macro.name.underscore %>' <% end %> <% end %> @@ -32,11 +32,11 @@ tinymce.create('tinymce.plugins.MacrosPlugin', { icons : false }); - <% macros_in_menu.each do |macro_name, plugin_instance| %> + <% macros_in_menu.each do |macro| %> c.onRenderMenu.add(function(c, m) { m.add({ - title: <%= macro_title(macro_name).to_json %>, - onclick: <%= generate_macro_config_dialog(macro_name) %> + title: <%= macro_title(macro).to_json %>, + onclick: <%= generate_macro_config_dialog(macro) %> }); }); <% end %> @@ -75,11 +75,11 @@ tinyMCE.init({ language: <%= tinymce_language.inspect %>, entity_encoding: 'raw', setup : function(ed) { - <% macros_with_buttons.each do |macro_name, plugin_instance| %> - ed.addButton('<%= macro_name %>', { - title: <%= macro_title(macro_name).to_json %>, - onclick: <%= generate_macro_config_dialog(macro_name) %>, - image : '<%= macro_configuration(macro_name)[:icon_path]%>' + <% macros_with_buttons.each do |macro| %> + ed.addButton('<%= macro.name.underscore %>', { + title: <%= macro_title(macro).to_json %>, + onclick: <%= generate_macro_config_dialog(macro) %>, + image : '<%= macro.configuration[:icon_path]%>' }); <% end %> } diff --git a/lib/noosfero/plugin.rb b/lib/noosfero/plugin.rb index 081425b..d4de15e 100644 --- a/lib/noosfero/plugin.rb +++ b/lib/noosfero/plugin.rb @@ -7,9 +7,6 @@ class Noosfero::Plugin def initialize(context=nil) self.context = context - macro_methods.each do |method| - Environment.macros[context.environment.id][method] = self unless context.nil? - end end class << self @@ -147,6 +144,12 @@ class Noosfero::Plugin blocks || [] end + def macros + self.class.constants.map do |constant_name| + self.class.const_get(constant_name) + end.select {|klass| klass <= Noosfero::Plugin::Macro} + end + # Here the developer may specify the events to which the plugins can # register and must return true or false. The default value must be false. @@ -443,12 +446,6 @@ class Noosfero::Plugin nil end - # -> Register macro methods in environment - # returns = ['method1', 'method2', ...] - def macro_methods - [] - end - # -> Finds objects by their contents # returns = {:results => [a, b, c, ...], ...} # P.S.: The plugin might add other informations on the return hash for its diff --git a/lib/noosfero/plugin/macro.rb b/lib/noosfero/plugin/macro.rb new file mode 100644 index 0000000..aedf374 --- /dev/null +++ b/lib/noosfero/plugin/macro.rb @@ -0,0 +1,44 @@ +class Noosfero::Plugin::Macro + + attr_accessor :context + + def self.configuration +# {:generator => ''} + end + + def self.plugin + name.split('::')[0...-1].join('::').constantize + end + + def initialize(context=nil) + self.context = context + end + + def attributes(macro) + macro.attributes.to_hash. + select {|key, value| key[0..10] == 'data-macro-'}. + inject({}){|result, a| result.merge({a[0][11..-1] => a[1]})}. + with_indifferent_access + end + + def convert(macro, source) + macro_name = macro['data-macro'] + attrs = attributes(macro) + + begin + content = parse(attrs, macro.inner_html, source) + macro['class'] = "parsed-macro #{macro_name}" + rescue Exception => exception + content = _("Unsupported macro %s!") % macro_name + macro['class'] = "failed-macro #{macro_name}" + end + + attrs.each {|key, value| macro.remove_attribute("data-macro-#{key}")} + content + end + + def parse(attrs, inner_html, source) + raise + end + +end diff --git a/lib/noosfero/plugin/manager.rb b/lib/noosfero/plugin/manager.rb index ec4ce93..184ae31 100644 --- a/lib/noosfero/plugin/manager.rb +++ b/lib/noosfero/plugin/manager.rb @@ -6,7 +6,6 @@ class Noosfero::Plugin::Manager def initialize(environment, context) @environment = environment @context = context - Environment.macros = {environment.id => {}} unless environment.nil? end delegate :each, :to => :enabled_plugins @@ -54,12 +53,23 @@ class Noosfero::Plugin::Manager result end + def parse_macro(macro_name, macro, source = nil) + macro_instance = enabled_macros[macro_name] || enabled_macros['default'] + macro_instance.convert(macro, source) + end + def enabled_plugins @enabled_plugins ||= (Noosfero::Plugin.all & environment.enabled_plugins).map do |plugin| plugin.constantize.new(context) end end + def enabled_macros + @enabled_macros ||= dispatch(:macros).inject({}) do |memo, macro| + memo.merge!(macro.name.underscore => macro.new(context)) + end.merge('default' => Noosfero::Plugin::Macro.new(context)) + end + def [](name) klass = Noosfero::Plugin.klass(name) enabled_plugins.select do |plugin| diff --git a/test/unit/application_helper_test.rb b/test/unit/application_helper_test.rb index 880b7c7..8fa22bb 100644 --- a/test/unit/application_helper_test.rb +++ b/test/unit/application_helper_test.rb @@ -661,35 +661,38 @@ class ApplicationHelperTest < ActiveSupport::TestCase should 'parse macros' do class Plugin1 < Noosfero::Plugin - def macro_test1(params, inner_html, source) + end + + class Plugin1::Macro1 < Noosfero::Plugin::Macro + def parse(params, inner_html, source) 'Test1' end - def macro_test2(params, inner_html, source) - 'Test2' - end + end - def macro_methods - ['macro_test1', 'macro_test2'] + class Plugin1::Macro2 < Noosfero::Plugin::Macro + def parse(params, inner_html, source) + 'Test2' end end - @environment = Environment.default - Environment.macros = {@environment.id => {}} - context = mock() - context.stubs(:environment).returns(@environment) - Plugin1.new(context) - html = ' -
-
-
- ' + environment = Environment.default + environment.enable_plugin(Plugin1) + @plugins = Noosfero::Plugin::Manager.new(environment, self) + macro1_name = Plugin1::Macro1.name.underscore + macro2_name = Plugin1::Macro2.name.underscore + + html = " +
+
+
+ " parsed_html = convert_macro(html, mock()) parsed_divs = Hpricot(parsed_html).search('div') - expected_divs = Hpricot(' -
Test1
-
Test2
-
Unsupported macro unexistent!
- ').search('div') + expected_divs = Hpricot(" +
Test1
+
Test2
+
Unsupported macro unexistent!
+ ").search('div') # comparing div attributes between parsed and expected html parsed_divs.each_with_index do |div, i| diff --git a/test/unit/macros_helper_test.rb b/test/unit/macros_helper_test.rb index 382614b..511b096 100644 --- a/test/unit/macros_helper_test.rb +++ b/test/unit/macros_helper_test.rb @@ -24,14 +24,25 @@ class MacrosHelperTest < ActiveSupport::TestCase :title => _('Profile Image Link') } - should 'generate html for macro configuration' do + class Plugin1 < Noosfero::Plugin + end + + def setup @environment = Environment.default - Environment.macros = {} - macros = Environment.macros[@environment.id] = {} - plugin_instance = mock - plugin_instance.stubs('config_macro_example').returns(CONFIG) - macros['macro_example'] = plugin_instance - html = macro_configuration_dialog('macro_example') + @environment.enable_plugin(Plugin1) + @plugins = Noosfero::Plugin::Manager.new(@environment, self) + end + + attr_accessor :environment + + should 'generate html for macro configuration' do + class Plugin1::Macro < Noosfero::Plugin::Macro + def self.configuration + CONFIG + end + end + + html = macro_configuration_dialog(Plugin1::Macro) assert_tag_in_string html, :tag => 'label', :content => _('Identifier') assert_tag_in_string html, :tag => 'input', :attributes => {:name => 'identifier'} assert_tag_in_string html, :tag => 'label', :content => 'size'.humanize @@ -43,82 +54,79 @@ class MacrosHelperTest < ActiveSupport::TestCase end should 'get macro title' do - @environment = Environment.default - Environment.macros = {} - macros = Environment.macros[@environment.id] = {} - plugin_instance = mock - plugin_instance.stubs('config_macro_example').returns(CONFIG) - macros['macro_example'] = plugin_instance - title = macro_title('macro_example') + class Plugin1::Macro < Noosfero::Plugin::Macro + def self.configuration + CONFIG + end + end + title = macro_title(Plugin1::Macro) assert _('Profile Image Link'), title end + class Plugin1::Macro1 < Noosfero::Plugin::Macro + def self.configuration + {} + end + end + + class Plugin1::Macro2 < Noosfero::Plugin::Macro + def self.configuration + {:icon_path => 'icon.png'} + end + end + should 'get only macros in menu' do - @environment = Environment.default - Environment.macros = {} - macros = Environment.macros[@environment.id] = {} - plugin_instance = mock - plugin_instance.stubs('config_macro_example').returns({}) - macros['macro_example'] = plugin_instance - plugin_instance_other = mock - plugin_instance_other.stubs('config_macro_example_other').returns({:icon_path => 'icon.png'}) - macros['macro_example_other'] = plugin_instance_other - assert_equal [plugin_instance], macros_in_menu.values + assert_includes macros_in_menu, Plugin1::Macro1 + assert_not_includes macros_in_menu, Plugin1::Macro2 end should 'get only macros with buttons' do - @environment = Environment.default - Environment.macros = {} - macros = Environment.macros[@environment.id] = {} - plugin_instance = mock - plugin_instance.stubs('config_macro_example').returns({}) - macros['macro_example'] = plugin_instance - plugin_instance_other = mock - plugin_instance_other.stubs('config_macro_example_other').returns({:icon_path => 'icon.png'}) - macros['macro_example_other'] = plugin_instance_other - assert_equal [plugin_instance_other], macros_with_buttons.values + assert_includes macros_with_buttons, Plugin1::Macro2 + assert_not_includes macros_with_buttons, Plugin1::Macro1 end should 'skip macro config dialog and call generator directly' do - @environment = Environment.default - Environment.macros = {} - macros = Environment.macros[@environment.id] = {} - plugin_instance = mock - plugin_instance.stubs('config_macro_example').returns({:skip_dialog => true, :generator => '', :params => [] }) - macros['macro_example'] = plugin_instance - assert_equal 'function(){}', generate_macro_config_dialog('macro_example') + class Plugin1::Macro < Noosfero::Plugin::Macro + def self.configuration + {:skip_dialog => true, :generator => '', :params => []} + end + end + + assert_equal 'function(){}', generate_macro_config_dialog(Plugin1::Macro) end should 'include js files' do - @environment = Environment.default - Environment.macros = {} - macros = Environment.macros[@environment.id] = {} - plugin_instance = mock - plugin_instance.stubs('config_macro_example').returns({:js_files => 'macro.js' }) - plugin_instance.class.stubs(:public_path).with('macro.js').returns('macro.js') - macros['macro_example'] = plugin_instance - assert_equal '', include_macro_js_files + class Plugin1::Macro < Noosfero::Plugin::Macro + def self.configuration + {:js_files => 'macro.js' } + end + end + + assert_equal "", include_macro_js_files end should 'get macro css files' do - @environment = Environment.default - Environment.macros = {} - macros = Environment.macros[@environment.id] = {} - plugin_instance = mock - plugin_instance.stubs('config_macro_example').returns({:css_files => 'macro.css' }) - plugin_instance.class.stubs(:public_path).with('macro.css').returns('macro.css') - macros['macro_example'] = plugin_instance - assert_equal 'macro.css', macro_css_files + class Plugin1::Macro < Noosfero::Plugin::Macro + def self.configuration + {:css_files => 'macro.css' } + end + + def self.public_path(file) + 'macro.css' + end + end + + assert_equal Plugin1.public_path('macro.css'), macro_css_files end should 'get macro specific generator' do - @environment = Environment.default - Environment.macros = {} - macros = Environment.macros[@environment.id] = {} - plugin_instance = mock - plugin_instance.stubs('config_macro_example').returns({:generator => 'macro_generator' }) - macros['macro_example'] = plugin_instance - assert_equal 'macro_generator', macro_generator('macro_example') + class Plugin1::Macro < Noosfero::Plugin::Macro + def self.configuration + {:generator => 'macro_generator' } + end + end + + assert_equal 'macro_generator', macro_generator(Plugin1::Macro) end end diff --git a/test/unit/plugin_manager_test.rb b/test/unit/plugin_manager_test.rb index 7bb6e11..78cbc10 100644 --- a/test/unit/plugin_manager_test.rb +++ b/test/unit/plugin_manager_test.rb @@ -168,4 +168,30 @@ class PluginManagerTest < ActiveSupport::TestCase assert_equal Plugin2, manager.fetch_first_plugin(:random_event) end + should 'parse macro' do + class Plugin1 < Noosfero::Plugin + def macros + [Macro1, Macro2] + end + end + + class Plugin1::Macro1 < Noosfero::Plugin::Macro + def convert(macro, source) + macro.gsub('%{name}', 'Macro1') + end + end + + class Plugin1::Macro2 < Noosfero::Plugin::Macro + def convert(macro, source) + macro.gsub('%{name}', 'Macro2') + end + end + + environment.enable_plugin(Plugin1) + macro = 'My name is %{name}!' + + assert_equal 'My name is Macro1!', manager.parse_macro(Plugin1::Macro1.name.underscore, macro) + assert_equal 'My name is Macro2!', manager.parse_macro(Plugin1::Macro2.name.underscore, macro) + end + end diff --git a/test/unit/plugin_test.rb b/test/unit/plugin_test.rb index 9a4fd6e..9e95015 100644 --- a/test/unit/plugin_test.rb +++ b/test/unit/plugin_test.rb @@ -28,35 +28,6 @@ class PluginTest < ActiveSupport::TestCase assert_equal({:controller => 'plugin_test/plugin1_admin', :action => 'index'}, Plugin1.admin_url) end - should 'register its macros in the environment when instantiated' do - class Plugin1 < Noosfero::Plugin - def macro_example1(params, inner_html, source) - end - - def example2(params, inner_html, source) - end - - def not_macro - end - - def macro_methods - ['macro_example1', 'example2'] - end - end - - Environment.macros = {} - Environment.macros[environment.id] = {} - macros = Environment.macros[environment.id] - context = mock() - context.stubs(:environment).returns(environment) - - plugin_instance = Plugin1.new(context) - - assert_equal plugin_instance, macros['macro_example1'] - assert_equal plugin_instance, macros['example2'] - assert_nil macros['not_macro'] - end - should 'load_comments return nil by default' do class Plugin1 < Noosfero::Plugin; end; -- libgit2 0.21.2