From a4dae44e4cd0dbe0388c3f0c324b76a373fcf00b Mon Sep 17 00:00:00 2001
From: Gabriel Silva <gabriel93.silva@gmail.com>
Date: Tue, 30 Aug 2016 22:21:19 +0000
Subject: [PATCH] Reviews ExternalProfile methods

---
 app/models/concerns/follower.rb   |  9 +--------
 app/models/external_person.rb     | 76 +---------------------------------------------------------------------------
 app/models/external_profile.rb    | 64 ++++++++--------------------------------------------------------
 test/unit/external_person_test.rb |  6 ++++++
 test/unit/followable_test.rb      | 59 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 test/unit/follower_test.rb        | 39 +++++++++++++++++++++++++++++----------
 test/unit/profile_test.rb         | 17 -----------------
 7 files changed, 104 insertions(+), 166 deletions(-)
 create mode 100644 test/unit/followable_test.rb

diff --git a/app/models/concerns/follower.rb b/app/models/concerns/follower.rb
index a9ec03c..ee78ebb 100644
--- a/app/models/concerns/follower.rb
+++ b/app/models/concerns/follower.rb
@@ -5,9 +5,6 @@ module Follower
     circles = [circles] unless circles.is_a?(Array)
     circles.each do |new_circle|
       next if new_circle.owner != self || !profile.kind_of?(new_circle.profile_type.constantize)
-      if (new_circle.owner != self && !(self.kind_of?(new_circle.owner.class)) || new_circle.profile_type != profile.class.name)
-        next
-      end
       ProfileFollower.create(profile: profile, circle: new_circle)
     end
   end
@@ -22,7 +19,7 @@ module Follower
   end
 
   def followed_profiles
-    external_people = ExternalPerson.joins(:circles).where("circles.owner_id = ?", self.id)
+    external_people = ExternalProfile.joins(:circles).where("circles.owner_id = ?", self.id)
     profiles = Profile.joins(:circles).where("circles.owner_id = ?", self.id)
 
     external_people.uniq + profiles.uniq
@@ -43,8 +40,4 @@ module Follower
     ProfileFollower.with_profile(profile).with_circle(circle).destroy_all
   end
 
-  def in_circle?(circle)
-    circle.in? self.circles
-  end
-
 end
diff --git a/app/models/external_person.rb b/app/models/external_person.rb
index f1fe26d..d479054 100644
--- a/app/models/external_person.rb
+++ b/app/models/external_person.rb
@@ -5,8 +5,6 @@ class ExternalPerson < ExternalProfile
   include ProfileEntity
   include Follower
 
-  has_many :profile_followers, :as => :profile
-  has_many :circles, :through => :profile_followers
   has_many :owned_circles, as: :owner, :class_name => "Circle"
 
   validates_uniqueness_of :identifier, scope: :source
@@ -180,91 +178,19 @@ class ExternalPerson < ExternalProfile
     derivated_methods.merge(methods_and_responses)
   end
 
-  def profile_instance_methods
-    methods_and_responses = {
-     role_assignments: RoleAssignment.none, favorite_enterprises:
-     Enterprise.none, memberships: Profile.none, friendships: Profile.none,
-     tasks: Task.none, suggested_profiles: ProfileSuggestion.none,
-     suggested_people: ProfileSuggestion.none, suggested_communities:
-     ProfileSuggestion.none, public_profile: true, nickname: nil, custom_footer:
-     '', custom_header: '', address: '', zip_code: '', contact_phone: '',
-     image_builder: nil, description: '', closed: false, template_id: nil, lat:
-     nil, lng: nil, is_template: false, fields_privacy: {}, preferred_domain_id:
-     nil, category_ids: [], country: '', city: '', state: '',
-     national_region_code: '', redirect_l10n: false, notification_time: 0,
-     custom_url_redirection: nil, email_suggestions: false,
-     allow_members_to_invite: false, invite_friends_only: false, secret: false,
-     profile_admin_mail_notification: false, redirection_after_login: nil,
-     profile_activities: ProfileActivity.none, action_tracker_notifications:
-     ActionTrackerNotification.none, tracked_notifications:
-     ActionTracker::Record.none, scraps_received: Scrap.none, template:
-     Profile.none, comments_received: Comment.none, email_templates:
-     EmailTemplate.none, members: Profile.none, members_like: Profile.none,
-     members_by: Profile.none, members_by_role: Profile.none, scraps:
-     Scrap.none, welcome_page_content: nil, settings: {}, find_in_all_tasks:
-     nil, top_level_categorization: {}, interests: Category.none, geolocation:
-     '', country_name: '', pending_categorizations: [], add_category: false,
-     create_pending_categorizations: false, top_level_articles: Article.none,
-     valid_identifier: true, valid_template: false, create_default_set_of_boxes:
-     true, copy_blocks_from: nil, default_template: nil,
-     template_without_default: nil, template_with_default: nil, apply_template:
-     false, iframe_whitelist: [], recent_documents: Article.none, last_articles:
-     Article.none, is_validation_entity?: false, hostname: nil, own_hostname:
-     nil, article_tags: {}, tagged_with: Article.none,
-     insert_default_article_set: false, copy_articles_from: true,
-     copy_article_tree: nil, copy_article?: false, add_member: false,
-     remove_member: false, add_admin: false, remove_admin: false, add_moderator:
-     false, display_info_to?: true, update_category_from_region: nil,
-     accept_category?: false, custom_header_expanded: '',
-     custom_footer_expanded: '', public?: true, themes: [], find_theme: nil,
-     blogs: Blog.none, blog: nil, has_blog?: false, forums: Forum.none, forum:
-     nil, has_forum?: false, admins: [], settings_field: {}, setting_changed:
-     false, public_content: true, enable_contact?: false, folder_types: [],
-     folders: Article.none, image_galleries: Article.none, image_valid: true,
-     update_header_and_footer: nil, update_theme: nil, update_layout_template:
-     nil, recent_actions: ActionTracker::Record.none, recent_notifications:
-     ActionTracker::Record.none, more_active_label: _('no activity'),
-     more_popular_label: _('no members'), profile_custom_image: nil,
-     is_on_homepage?: false, activities: ProfileActivity.none,
-     may_display_field_to?: true, may_display_location_to?: true, public_fields:
-     {}, display_private_info_to?: true, can_view_field?:
-     true, remove_from_suggestion_list: nil, layout_template: 'default',
-     is_admin?: false, add_friend: false, is_a_friend?: false,
-     already_request_friendship?: false, tracked_actions: ActionTracker::Record.none
-    }
-
-    derivated_methods = generate_derivated_methods(methods_and_responses)
-    derivated_methods.merge(methods_and_responses)
-  end
-
   def method_missing(method, *args, &block)
     if person_instance_methods.keys.include?(method)
       return person_instance_methods[method]
     end
-    if profile_instance_methods.keys.include? method
-      return profile_instance_methods[method]
-    end
-    super
+    super(method, *args, &block)
   end
 
   def respond_to_missing?(method_name, include_private = false)
     person_instance_methods.keys.include?(method_name) ||
-    profile_instance_methods.keys.include?(method_name) ||
     super
   end
 
   def kind_of?(klass)
     (klass == Person) ? true : super
   end
-
-  private
-
-  def generate_derivated_methods(methods)
-    derivated_methods = {}
-    methods.keys.each do |method|
-     derivated_methods[method.to_s.insert(-1, '?').to_sym] = false
-      derivated_methods[method.to_s.insert(-1, '=').to_sym] = nil
-    end
-    derivated_methods
-  end
 end
diff --git a/app/models/external_profile.rb b/app/models/external_profile.rb
index db72fca..b3bf687 100644
--- a/app/models/external_profile.rb
+++ b/app/models/external_profile.rb
@@ -2,7 +2,8 @@ class ExternalProfile < ActiveRecord::Base
 
   include Followable
 
-  has_many :circles, as: :person
+  has_many :profile_followers, :as => :profile
+  has_many :circles, :through => :profile_followers
 
   def name
     "#{self[:name]}@#{self.source}"
@@ -40,53 +41,8 @@ class ExternalProfile < ActiveRecord::Base
     "http://#{self.source}/profile/#{self.identifier}/icon/"
   end
 
-  # External Person should respond to all methods in Person and Profile
-  def person_instance_methods
-    methods_and_responses = {
-     enterprises: Enterprise.none, communities: Community.none, friends:
-     Person.none, memberships: Profile.none, friendships: Person.none,
-     following_articles: Article.none, article_followers: ArticleFollower.none,
-     requested_tasks: Task.none, mailings: Mailing.none, scraps_sent:
-     Scrap.none, favorite_enterprise_people: FavoriteEnterprisePerson.none,
-     favorite_enterprises: Enterprise.none, acepted_forums: Forum.none,
-     articles_with_access: Article.none, suggested_profiles:
-     ProfileSuggestion.none, suggested_people: ProfileSuggestion.none,
-     suggested_communities: ProfileSuggestion.none, user: nil,
-     refused_communities: Community.none, has_permission?: false,
-     has_permission_with_admin?: false, has_permission_without_admin?: false,
-     has_permission_with_plugins?: false, has_permission_without_plugins?:
-     false, memberships_by_role: Person.none, can_change_homepage?: false,
-     can_control_scrap?: false, receives_scrap_notification?: false,
-     can_control_activity?: false, can_post_content?: false,
-     suggested_friend_groups: [], friend_groups: [], add_friend: nil,
-     already_request_friendship?: false, remove_friend: nil,
-     presence_of_required_fields: nil, active_fields: [], required_fields: [],
-     signup_fields: [], default_set_of_blocks: [], default_set_of_boxes: [],
-     default_set_of_articles: [], cell_phone: nil, comercial_phone: nil,
-     nationality: nil, schooling: nil, contact_information: nil, sex: nil,
-     birth_date: nil, jabber_id: nil, personal_website: nil, address_reference:
-     nil, district: nil, schooling_status: nil, formation: nil,
-     custom_formation: nil, area_of_study: nil, custom_area_of_study: nil,
-     professional_activity: nil, organization_website: nil, organization: nil,
-     photo: nil, city: nil, state: nil, country: nil, zip_code: nil,
-     address_line2: nil, copy_communities_from: nil,
-     has_organization_pending_tasks?: false, organizations_with_pending_tasks:
-     Organization.none, pending_tasks_for_organization: Task.none,
-     build_contact: nil, is_a_friend?: false, ask_to_join?: false, refuse_join:
-     nil, blocks_to_expire_cache: [], cache_keys: [], communities_cache_key: '',
-     friends_cache_key: '', manage_friends_cache_key: '',
-     relationships_cache_key: '', is_member_of?: false,
-     each_friend: nil, is_last_admin?: false, is_last_admin_leaving?: false,
-     leave: nil, last_notification: nil, notification_time: 0, notifier: nil,
-     remove_suggestion: nil, allow_invitation_from?: false, in_social_circle?: false,
-     allow_followers: false, in_circles: Circle.none, followers: [], in_circle?: false
-    }
-
-    derivated_methods = generate_derivated_methods(methods_and_responses)
-    derivated_methods.merge(methods_and_responses)
-  end
-
-  def profile_instance_methods
+  # External Profile should respond to all methods in Profile
+def profile_instance_methods
     methods_and_responses = {
      role_assignments: RoleAssignment.none, favorite_enterprises:
      Enterprise.none, memberships: Profile.none, friendships: Profile.none,
@@ -133,10 +89,10 @@ class ExternalProfile < ActiveRecord::Base
      more_popular_label: _('no members'), profile_custom_image: nil,
      is_on_homepage?: false, activities: ProfileActivity.none,
      may_display_field_to?: true, may_display_location_to?: true, public_fields:
-     {}, followed_by?: false, display_private_info_to?: true, can_view_field?:
+     {}, display_private_info_to?: true, can_view_field?:
      true, remove_from_suggestion_list: nil, layout_template: 'default',
      is_admin?: false, add_friend: false, is_a_friend?: false,
-     already_request_friendship?: false
+     already_request_friendship?: false, tracked_actions: ActionTracker::Record.none
     }
 
     derivated_methods = generate_derivated_methods(methods_and_responses)
@@ -144,18 +100,14 @@ class ExternalProfile < ActiveRecord::Base
   end
 
   def method_missing(method, *args, &block)
-    if person_instance_methods.keys.include?(method)
-      return person_instance_methods[method]
-    end
     if profile_instance_methods.keys.include? method
       return profile_instance_methods[method]
     end
+    raise NoMethodError, "undefined method #{method} for #{self}"
   end
 
   def respond_to_missing?(method_name, include_private = false)
-    person_instance_methods.keys.include?(method_name) ||
-    profile_instance_methods.keys.include?(method_name) ||
-    super
+    profile_instance_methods.keys.include?(method_name) || super
   end
 
   private
diff --git a/test/unit/external_person_test.rb b/test/unit/external_person_test.rb
index 4d0ac7b..0d3a611 100644
--- a/test/unit/external_person_test.rb
+++ b/test/unit/external_person_test.rb
@@ -135,6 +135,12 @@ class ExternalPersonTest < ActiveSupport::TestCase
     end
   end
 
+  should 'raise error if method is not in Person or Profile' do
+    assert_raise NoMethodError do
+      ExternalPerson.new.send(:this_method_does_not_exist_right?)
+    end
+  end
+
   should 'be able to follow people' do
     person = fast_create(Person, :environment_id => Environment.default.id)
     circle = Circle.create(owner: @external_person, profile_type: "Person", name: "FRIENDSSS")
diff --git a/test/unit/followable_test.rb b/test/unit/followable_test.rb
new file mode 100644
index 0000000..a5343c6
--- /dev/null
+++ b/test/unit/followable_test.rb
@@ -0,0 +1,59 @@
+require_relative "../test_helper"
+
+class FollowerTest < ActiveSupport::TestCase
+
+  def setup
+    @person1 = create_user('perso-test-1').person
+    @person2 = create_user('person-test-2').person
+    @external_person = ExternalPerson.create!(identifier: 'johnlocke',
+                                     name: 'John Locke',
+                                     source: 'anerenvironment.org',
+                                     email: 'locke@island.org',
+                                     created_at: Date.yesterday)
+
+    @circle1 = Circle.create!(owner: @person1, name: "Zombies", profile_type: 'Person')
+    @circle2 = Circle.create!(owner: @person1, name: "Humans", profile_type: 'Person')
+    @circle3 = Circle.create!(owner: @external_person, name: "Crypt", profile_type: 'Person')
+
+    @external_profile = ExternalProfile.create
+  end
+
+  should 'return all unique circles and followers of a profile' do
+    @person1.follow(@person2, @circle1)
+    @person1.follow(@person2, @circle2)
+    @external_person.follow(@person2, @circle3)
+
+    assert_equivalent [@circle1, @circle2, @circle3], @person2.circles
+    assert_equivalent [@person1, @external_person], @person2.followers
+  end
+
+  should 'return all unique circles and followers of a external profile' do
+    Circle.any_instance.stubs(:profile_type).returns("ExternalProfile")
+    @person1.follow(@external_profile, @circle1)
+    @person1.follow(@external_profile, @circle2)
+    @external_person.follow(@external_profile, @circle3)
+
+    assert_equivalent [@circle1, @circle2, @circle3], @external_profile.circles
+    assert_equivalent [@person1, @external_person], @external_profile.followers
+  end
+
+  should 'onlyreturn true if profile is in circle' do
+    @person1.follow(@person2, @circle1)
+    @external_person.follow(@person2, @circle3)
+
+    assert @person2.in_circle? @circle1
+    assert @person2.in_circle? @circle3
+    refute @person2.in_circle? @circle2
+  end
+
+  should 'only return true if external profile is in circle' do
+    Circle.any_instance.stubs(:profile_type).returns("ExternalProfile")
+    @person1.follow(@external_profile, @circle1)
+    @external_person.follow(@external_profile, @circle3)
+
+    assert @external_profile.in_circle? @circle1
+    assert @external_profile.in_circle? @circle3
+    refute @external_profile.in_circle? @circle2
+  end
+
+end
diff --git a/test/unit/follower_test.rb b/test/unit/follower_test.rb
index 590fb49..8e98bb6 100644
--- a/test/unit/follower_test.rb
+++ b/test/unit/follower_test.rb
@@ -85,13 +85,13 @@ class FollowerTest < ActiveSupport::TestCase
   end
 
   should 'get the followed profiles for a regular user' do
-    person3 = create_user('person-test-3').person
     community = fast_create(Community)
 
     @person1.follow(@person2, @circle1)
-    @person1.follow(person3, @circle1)
+    @person1.follow(@external_person, @circle1)
     @person1.follow(community, @circle3)
-    assert_equivalent [@person2, person3, community], @person1.followed_profiles
+    assert_equivalent [@person2, @external_person, community],
+                      @person1.followed_profiles
   end
 
   should 'get the followed profiles for an external user' do
@@ -103,7 +103,8 @@ class FollowerTest < ActiveSupport::TestCase
     @external_person.follow(@person2, @circle1)
     @external_person.follow(person3, @circle1)
     @external_person.follow(community, @circle3)
-    assert_equivalent [@person2, person3, community], @external_person.followed_profiles
+    assert_equivalent [@person2, person3, community],
+                      @external_person.followed_profiles
   end
 
   should 'not follow same person twice even with different circles' do
@@ -145,7 +146,8 @@ class FollowerTest < ActiveSupport::TestCase
     @person1.follow(@person2, [@circle1, @circle2])
 
     @person1.remove_profile_from_circle(@person2, @circle2)
-    assert_equivalent [@circle1], @person2.circles
+    assert @person2.in_circle? @circle1
+    assert_not @person2.in_circle? @circle2
   end
 
   should 'not remove a person from a circle if the user is not the owner' do
@@ -153,13 +155,30 @@ class FollowerTest < ActiveSupport::TestCase
     @person1.follow(@person2, [@circle1, @circle2])
 
     person3.remove_profile_from_circle(@person2, @circle2)
-    assert_equivalent [@circle1, @circle2], @person2.circles
+    assert @person2.in_circle? @circle1
+    assert @person2.in_circle? @circle2
+  end
+
+  should 'follow external profile' do
+    external_profile = ExternalProfile.create
+    @circle1.stubs(:profile_type).returns("ExternalProfile")
+
+    assert_difference '@person1.followed_profiles.count' do
+      @person1.follow(external_profile, @circle1)
+    end
+    assert @person1.follows? external_profile
+    assert_equivalent [external_profile], @person1.followed_profiles
   end
 
-  should 'follow External Person' do
-    assert_difference 'ProfileFollower.count' do
-      @person1.follow(@external_person, @circle1)
+  should 'unfollow external profile' do
+    external_profile = ExternalProfile.create
+    @circle1.stubs(:profile_type).returns("ExternalProfile")
+    @person1.follow(external_profile, @circle1)
+
+    assert_difference '@person1.followed_profiles.count', -1 do
+      @person1.unfollow(external_profile)
     end
-    assert @person1.follows? @external_person
+    assert_not @person1.follows?(@person2)
   end
+
 end
diff --git a/test/unit/profile_test.rb b/test/unit/profile_test.rb
index 4938789..9c998ba 100644
--- a/test/unit/profile_test.rb
+++ b/test/unit/profile_test.rb
@@ -2232,21 +2232,4 @@ class ProfileTest < ActiveSupport::TestCase
       c.add_member(p)
     end
   end
-
-  should 'return all circles and followers, and do not repeat' do
-    person1 = create_user('testperson-1').person
-    person2 = create_user('testperson-2').person
-    person3 = create_user('testperson-3').person
-    circle1 = Circle.create!(:owner => person1, :name => "Night's Watch", :profile_type => 'Person')
-    circle2 = Circle.create!(:owner => person1, :name => "Free Folk", :profile_type => 'Person')
-    circle3 = Circle.create!(:owner => person2, :name => "The Unsullied", :profile_type => 'Person')
-
-    person1.follow(person3, circle1)
-    person1.follow(person3, circle2)
-    person2.follow(person3, circle3)
-
-    assert_equivalent person3.circles, [circle1, circle2, circle3]
-    assert_equivalent person3.followers, [person1, person2]
-  end
-
 end
--
libgit2 0.21.2