Commit b3756e146cf4594195d3f4b73dd2abddb2c7648e
Exists in
staging
and in
42 other branches
Merge branch 'plugins_hotspots_support' into 'next'
Plugins hotspots support This MR introduce the required code to support plugins extention through hotspots. This is a generic way that must be used to add specific plugins hotspots, as can been seen in FooPlugin. Also, it is not necessary to declare the hotspot in Noosfero::Plugin class. All you need is implement the method plugin_hotspots returning an array with your plugin's hotspot name and document their use in the main plugin class. This MR also introduce a new hotspot in LDAP plugin to manipulate the person data before user creation. This MR substitutes the old MR !507 See merge request !534
Showing
7 changed files
with
102 additions
and
12 deletions
Show diff stats
app/models/user.rb
| @@ -45,7 +45,7 @@ class User < ActiveRecord::Base | @@ -45,7 +45,7 @@ class User < ActiveRecord::Base | ||
| 45 | p = Person.new | 45 | p = Person.new |
| 46 | 46 | ||
| 47 | p.attributes = user.person_data | 47 | p.attributes = user.person_data |
| 48 | - p.identifier = user.login | 48 | + p.identifier = user.login if p.identifier.blank? |
| 49 | p.user = user | 49 | p.user = user |
| 50 | p.environment = user.environment | 50 | p.environment = user.environment |
| 51 | p.name ||= user.name || user.login | 51 | p.name ||= user.name || user.login |
lib/noosfero/plugin.rb
| @@ -8,6 +8,10 @@ class Noosfero::Plugin | @@ -8,6 +8,10 @@ class Noosfero::Plugin | ||
| 8 | self.context = context | 8 | self.context = context |
| 9 | end | 9 | end |
| 10 | 10 | ||
| 11 | + def environment | ||
| 12 | + context.environment if self.context | ||
| 13 | + end | ||
| 14 | + | ||
| 11 | class << self | 15 | class << self |
| 12 | 16 | ||
| 13 | attr_writer :should_load | 17 | attr_writer :should_load |
| @@ -35,6 +39,7 @@ class Noosfero::Plugin | @@ -35,6 +39,7 @@ class Noosfero::Plugin | ||
| 35 | # filters must be loaded after all extensions | 39 | # filters must be loaded after all extensions |
| 36 | klasses.each do |plugin| | 40 | klasses.each do |plugin| |
| 37 | load_plugin_filters plugin | 41 | load_plugin_filters plugin |
| 42 | + load_plugin_hotspots plugin | ||
| 38 | end | 43 | end |
| 39 | end | 44 | end |
| 40 | 45 | ||
| @@ -108,6 +113,23 @@ class Noosfero::Plugin | @@ -108,6 +113,23 @@ class Noosfero::Plugin | ||
| 108 | end | 113 | end |
| 109 | end | 114 | end |
| 110 | 115 | ||
| 116 | + # This is a generic method to extend the hotspots list with plugins | ||
| 117 | + # hotspots. This allows plugins to extend other plugins. | ||
| 118 | + # To use this, the plugin must define its hotspots inside a module Hotspots. | ||
| 119 | + # Its also needed to include Noosfero::Plugin::HotSpot module | ||
| 120 | + # in order to dispatch plugins methods. | ||
| 121 | + # | ||
| 122 | + # Checkout FooPlugin for usage example. | ||
| 123 | + def load_plugin_hotspots(plugin) | ||
| 124 | + ActionDispatch::Reloader.to_prepare do | ||
| 125 | + begin | ||
| 126 | + module_name = "#{plugin.name}::Hotspots" | ||
| 127 | + Noosfero::Plugin.send(:include, module_name.constantize) | ||
| 128 | + rescue NameError | ||
| 129 | + end | ||
| 130 | + end | ||
| 131 | + end | ||
| 132 | + | ||
| 111 | def add_controller_filters(controller_class, plugin, filters) | 133 | def add_controller_filters(controller_class, plugin, filters) |
| 112 | unless filters.is_a?(Array) | 134 | unless filters.is_a?(Array) |
| 113 | filters = [filters] | 135 | filters = [filters] |
plugins/foo/lib/foo_plugin.rb
| 1 | class FooPlugin < Noosfero::Plugin | 1 | class FooPlugin < Noosfero::Plugin |
| 2 | + include Noosfero::Plugin::HotSpot | ||
| 2 | 3 | ||
| 3 | def self.plugin_name | 4 | def self.plugin_name |
| 4 | "Foo" | 5 | "Foo" |
| @@ -8,12 +9,29 @@ class FooPlugin < Noosfero::Plugin | @@ -8,12 +9,29 @@ class FooPlugin < Noosfero::Plugin | ||
| 8 | _("A sample plugin to test autoload craziness.") | 9 | _("A sample plugin to test autoload craziness.") |
| 9 | end | 10 | end |
| 10 | 11 | ||
| 12 | + module Hotspots | ||
| 13 | + # -> Custom foo plugin hotspot | ||
| 14 | + # do something to extend the FooPlugin behaviour | ||
| 15 | + # receive params a, b and c | ||
| 16 | + # returns = boolean or something else | ||
| 17 | + def foo_plugin_my_hotspot(a, b, c) | ||
| 18 | + end | ||
| 19 | + | ||
| 20 | + # -> Custom title for foo profiles tab | ||
| 21 | + # returns = a string with a custom title | ||
| 22 | + def foo_plugin_tab_title | ||
| 23 | + end | ||
| 24 | + end | ||
| 25 | + | ||
| 11 | def control_panel_buttons | 26 | def control_panel_buttons |
| 12 | {:title => 'Foo plugin button', :icon => '', :url => ''} | 27 | {:title => 'Foo plugin button', :icon => '', :url => ''} |
| 13 | end | 28 | end |
| 14 | 29 | ||
| 15 | def profile_tabs | 30 | def profile_tabs |
| 16 | - {:title => 'Foo plugin tab', :id => 'foo_plugin', :content => lambda {'Foo plugin random content'} } | 31 | + title = plugins.dispatch_first(:foo_plugin_tab_title) |
| 32 | + title = 'Foo plugin tab' unless title | ||
| 33 | + | ||
| 34 | + {:title => title, :id => 'foo_plugin', :content => lambda {'Foo plugin random content'} } | ||
| 17 | end | 35 | end |
| 18 | 36 | ||
| 19 | end | 37 | end |
plugins/foo/test/unit/foo_plugin_test.rb
| @@ -4,7 +4,25 @@ class FooPluginTest < ActiveSupport::TestCase | @@ -4,7 +4,25 @@ class FooPluginTest < ActiveSupport::TestCase | ||
| 4 | def test_foo | 4 | def test_foo |
| 5 | FooPlugin::Bar.create! | 5 | FooPlugin::Bar.create! |
| 6 | end | 6 | end |
| 7 | + | ||
| 7 | def test_monkey_patch | 8 | def test_monkey_patch |
| 8 | Profile.new.bar | 9 | Profile.new.bar |
| 9 | end | 10 | end |
| 11 | + | ||
| 12 | + should "respond to new hotspots" do | ||
| 13 | + plugin = FooPlugin.new | ||
| 14 | + | ||
| 15 | + assert plugin.respond_to?(:foo_plugin_my_hotspot) | ||
| 16 | + assert plugin.respond_to?(:foo_plugin_tab_title) | ||
| 17 | + end | ||
| 18 | + | ||
| 19 | + should "other plugin respond to new hotspots" do | ||
| 20 | + class TestPlugin < Noosfero::Plugin | ||
| 21 | + end | ||
| 22 | + | ||
| 23 | + plugin = TestPlugin.new | ||
| 24 | + | ||
| 25 | + assert plugin.respond_to?(:foo_plugin_my_hotspot) | ||
| 26 | + assert plugin.respond_to?(:foo_plugin_tab_title) | ||
| 27 | + end | ||
| 10 | end | 28 | end |
plugins/ldap/lib/ldap_authentication.rb
| @@ -77,18 +77,20 @@ class LdapAuthentication | @@ -77,18 +77,20 @@ class LdapAuthentication | ||
| 77 | end | 77 | end |
| 78 | 78 | ||
| 79 | def get_user_attributes_from_ldap_entry(entry) | 79 | def get_user_attributes_from_ldap_entry(entry) |
| 80 | - { | ||
| 81 | - :dn => entry.dn, | ||
| 82 | - :fullname => LdapAuthentication.get_attr(entry, self.attr_fullname), | ||
| 83 | - :mail => LdapAuthentication.get_attr(entry, self.attr_mail), | ||
| 84 | - } | 80 | + attributes = entry.instance_values["myhash"] |
| 81 | + | ||
| 82 | + attributes[:dn] = entry.dn | ||
| 83 | + attributes[:fullname] = LdapAuthentication.get_attr(entry, self.attr_fullname) | ||
| 84 | + attributes[:mail] = LdapAuthentication.get_attr(entry, self.attr_mail) | ||
| 85 | + | ||
| 86 | + attributes | ||
| 85 | end | 87 | end |
| 86 | 88 | ||
| 87 | # Return the attributes needed for the LDAP search. It will only | 89 | # Return the attributes needed for the LDAP search. It will only |
| 88 | # include the user attributes if on-the-fly registration is enabled | 90 | # include the user attributes if on-the-fly registration is enabled |
| 89 | def search_attributes | 91 | def search_attributes |
| 90 | if onthefly_register? | 92 | if onthefly_register? |
| 91 | - ['dn', self.attr_fullname, self.attr_mail] | 93 | + nil |
| 92 | else | 94 | else |
| 93 | ['dn'] | 95 | ['dn'] |
| 94 | end | 96 | end |
| @@ -111,6 +113,7 @@ class LdapAuthentication | @@ -111,6 +113,7 @@ class LdapAuthentication | ||
| 111 | end | 113 | end |
| 112 | login_filter = Net::LDAP::Filter.eq( self.attr_login, login ) | 114 | login_filter = Net::LDAP::Filter.eq( self.attr_login, login ) |
| 113 | object_filter = Net::LDAP::Filter.eq( "objectClass", "*" ) | 115 | object_filter = Net::LDAP::Filter.eq( "objectClass", "*" ) |
| 116 | + | ||
| 114 | attrs = {} | 117 | attrs = {} |
| 115 | 118 | ||
| 116 | search_filter = object_filter & login_filter | 119 | search_filter = object_filter & login_filter |
plugins/ldap/lib/ldap_plugin.rb
| 1 | require File.dirname(__FILE__) + '/ldap_authentication.rb' | 1 | require File.dirname(__FILE__) + '/ldap_authentication.rb' |
| 2 | 2 | ||
| 3 | class LdapPlugin < Noosfero::Plugin | 3 | class LdapPlugin < Noosfero::Plugin |
| 4 | + include Noosfero::Plugin::HotSpot | ||
| 4 | 5 | ||
| 5 | def self.plugin_name | 6 | def self.plugin_name |
| 6 | "LdapPlugin" | 7 | "LdapPlugin" |
| @@ -10,6 +11,25 @@ class LdapPlugin < Noosfero::Plugin | @@ -10,6 +11,25 @@ class LdapPlugin < Noosfero::Plugin | ||
| 10 | _("A plugin that add ldap support.") | 11 | _("A plugin that add ldap support.") |
| 11 | end | 12 | end |
| 12 | 13 | ||
| 14 | + module Hotspots | ||
| 15 | + # -> Custom ldap plugin hotspot to set profile data before user creation | ||
| 16 | + # receive the followings params: | ||
| 17 | + # - attrs with ldap received data | ||
| 18 | + # - login received by ldap | ||
| 19 | + # - params from current context | ||
| 20 | + # returns = updated person_data hash | ||
| 21 | + def ldap_plugin_set_profile_data(attrs, login, params) | ||
| 22 | + end | ||
| 23 | + | ||
| 24 | + # -> Custom ldap plugin hotspot to update user object | ||
| 25 | + # receive the followings params: | ||
| 26 | + # - user: user object | ||
| 27 | + # - attrs with ldap received data | ||
| 28 | + # returns = none | ||
| 29 | + def ldap_plugin_update_user(user, attrs) | ||
| 30 | + end | ||
| 31 | + end | ||
| 32 | + | ||
| 13 | def allow_user_registration | 33 | def allow_user_registration |
| 14 | false | 34 | false |
| 15 | end | 35 | end |
| @@ -39,13 +59,15 @@ class LdapPlugin < Noosfero::Plugin | @@ -39,13 +59,15 @@ class LdapPlugin < Noosfero::Plugin | ||
| 39 | user.name = attrs[:fullname] | 59 | user.name = attrs[:fullname] |
| 40 | user.password = password | 60 | user.password = password |
| 41 | user.password_confirmation = password | 61 | user.password_confirmation = password |
| 42 | - user.person_data = context.params[:profile_data] | 62 | + person_data = plugins.dispatch(:ldap_plugin_set_profile_data, attrs, login, context.params) |
| 63 | + user.person_data = person_data.blank? ? context.params[:profile_data] : person_data | ||
| 43 | user.activated_at = Time.now.utc | 64 | user.activated_at = Time.now.utc |
| 44 | user.activation_code = nil | 65 | user.activation_code = nil |
| 45 | 66 | ||
| 46 | ldap = LdapAuthentication.new(context.environment.ldap_plugin_attributes) | 67 | ldap = LdapAuthentication.new(context.environment.ldap_plugin_attributes) |
| 47 | begin | 68 | begin |
| 48 | - user = nil unless user.save | 69 | + user = nil unless user.save! |
| 70 | + plugins.dispatch(:ldap_plugin_update_user, user, attrs) | ||
| 49 | rescue | 71 | rescue |
| 50 | #User not saved | 72 | #User not saved |
| 51 | end | 73 | end |
| @@ -54,7 +76,6 @@ class LdapPlugin < Noosfero::Plugin | @@ -54,7 +76,6 @@ class LdapPlugin < Noosfero::Plugin | ||
| 54 | end | 76 | end |
| 55 | 77 | ||
| 56 | else | 78 | else |
| 57 | - | ||
| 58 | return nil if !user.activated? | 79 | return nil if !user.activated? |
| 59 | 80 | ||
| 60 | begin | 81 | begin |
test/unit/user_test.rb
| @@ -87,6 +87,14 @@ class UserTest < ActiveSupport::TestCase | @@ -87,6 +87,14 @@ class UserTest < ActiveSupport::TestCase | ||
| 87 | assert_equal person_count + 1, Person.count | 87 | assert_equal person_count + 1, Person.count |
| 88 | end | 88 | end |
| 89 | 89 | ||
| 90 | + def test_should_create_person_with_identifier_different_from_login | ||
| 91 | + user = User.create!(:login => 'new_user', :email => 'new_user@example.com', :password => 'test', :password_confirmation => 'test', :person_data => {:identifier => "new_test"}) | ||
| 92 | + | ||
| 93 | + assert Person.exists?(['user_id = ?', user.id]) | ||
| 94 | + | ||
| 95 | + assert user.login != user.person.identifier | ||
| 96 | + end | ||
| 97 | + | ||
| 90 | def test_login_validation | 98 | def test_login_validation |
| 91 | u = User.new | 99 | u = User.new |
| 92 | u.valid? | 100 | u.valid? |
| @@ -355,7 +363,7 @@ class UserTest < ActiveSupport::TestCase | @@ -355,7 +363,7 @@ class UserTest < ActiveSupport::TestCase | ||
| 355 | Person.any_instance.stubs(:created_at).returns(DateTime.parse('16-08-2010')) | 363 | Person.any_instance.stubs(:created_at).returns(DateTime.parse('16-08-2010')) |
| 356 | expected_hash = { | 364 | expected_hash = { |
| 357 | 'login' => 'x_and_y', 'is_admin' => true, 'since_month' => 8, | 365 | 'login' => 'x_and_y', 'is_admin' => true, 'since_month' => 8, |
| 358 | - 'chat_enabled' => false, 'since_year' => 2010, 'email_domain' => nil, | 366 | + 'chat_enabled' => false, 'since_year' => 2010, 'email_domain' => nil, |
| 359 | 'amount_of_friends' => 0, 'friends_list' => {}, 'enterprises' => [], | 367 | 'amount_of_friends' => 0, 'friends_list' => {}, 'enterprises' => [], |
| 360 | } | 368 | } |
| 361 | 369 |