Merge branch 'community_members' into 'master'

API: avoid to bring all members when get one community See merge request !1001

Merge branch 'community_members' into 'master'
API: avoid to bring all members when get one community See merge request !1001
Victor Costa
2 parents 0caacfdd 98f6dc63
Showing 7 changed files with 90 additions and 12 deletions Show diff stats
app/api/entities.rb
app/api/helpers.rb
app/api/v1/communities.rb
test/api/articles_test.rb
test/api/communities_test.rb
test/api/helpers_test.rb
test/api/people_test.rb
@@ -38,6 +38,13 @@ module Api
       PERMISSIONS[current_permission] <= PERMISSIONS[permission]
     end
+    def self.expose_optional_field?(field, options = {})
+      return false if options[:params].nil? 
+      optional_fields = options[:params][:optional_fields] || []
+      optional_fields.include?(field.to_s)
+    end
+
+
     class Image < Entity
       root 'images', 'image'
@@ -166,7 +173,8 @@ module Api
         community.admins.map{|admin| {"name"=>admin.name, "id"=>admin.id, "username" => admin.identifier}}
       end
       expose :categories, :using => Category
-      expose :members, :using => Person , :if => lambda{ |community, options| community.display_info_to? options[:current_person] }
+      expose :members_count
+      expose :members, :if => lambda {|community, options| Entities.expose_optional_field?(:members, options)}
     end
     class CommentBase < Entity
@@ -213,7 +221,7 @@ module Api
       expose :comments_count
       expose :archived, :documentation => {:type => "Boolean", :desc => "Defines if a article is readonly"}
       expose :type
-      expose :comments, using: CommentBase, :if => lambda{|obj,opt| opt[:params] && ['1','true',true].include?(opt[:params][:show_comments])}
+      expose :comments, using: CommentBase, :if => lambda{|comment,options| Entities.expose_optional_field?(:comments, options)}
       expose :published
       expose :accept_comments?, as: :accept_comments
     end
@@ -46,9 +46,11 @@ module Api
     def present_partial(model, options)
       if(params[:fields].present?)
         begin
-          fields = JSON.parse(params[:fields])
+          fields = JSON.parse((params.to_hash[:fields] || params.to_hash['fields']).to_json)
           if fields.present?
-            options.merge!(fields.symbolize_keys.slice(:only, :except))
+            fields = fields.symbolize_keys
+            options.merge!(:only => fields[:only]) if fields[:only].present?
+            options.merge!(:except => fields[:except]) if fields[:except].present?
           end
         rescue
           fields = params[:fields]
@@ -18,7 +18,7 @@ module Api
           communities = select_filtered_collection_of(environment, 'communities', params)
           communities = profiles_for_person(communities, current_person)
           communities = communities.by_location(params) # Must be the last. May return Exception obj
-          present communities, :with => Entities::Community, :current_person => current_person
+          present communities, :with => Entities::Community, :current_person => current_person, :params => params
         end
@@ -49,7 +49,7 @@ module Api
         get ':id' do
           community = profiles_for_person(environment.communities, current_person).find_by_id(params[:id])
-          present community, :with => Entities::Community, :current_person => current_person
+          present community, :with => Entities::Community, :current_person => current_person, :params => params
         end
       end
@@ -773,12 +773,12 @@ class ArticlesTest &lt; ActiveSupport::TestCase
     end
   end
-  should 'only show article comments when show_comments is present' do
+  should 'only show article comments when optional_fields comments is present' do
     person = fast_create(Person)
     article = fast_create(Article, :profile_id => person.id, :name => "Some thing")
     article.comments.create!(:body => "another comment", :author => person)
-    get "/api/v1/articles/#{article.id}/?#{params.merge(:show_comments => '1').to_query}"
+    get "/api/v1/articles/#{article.id}/?#{params.merge(:optional_fields => [:comments]).to_query}"
     json = JSON.parse(last_response.body)
     assert_includes json["article"].keys, "comments"
     assert_equal json["article"]["comments"].first["body"], "another comment"
@@ -805,4 +805,24 @@ class ArticlesTest &lt; ActiveSupport::TestCase
     json = JSON.parse(last_response.body)
     assert_includes json["article"]["permissions"], 'allow_post_content'
   end
+
+  should 'return only article fields defined in parameter' do
+    Article.destroy_all
+    article = fast_create(Article, :profile_id => user.person.id, :name => "Some thing")
+    params[:fields] = {:only => ['id', 'title']}
+    get "/api/v1/articles/?#{params.to_query}"
+    json = JSON.parse(last_response.body)
+    assert_equivalent ['id', 'title'], json["articles"].first.keys
+  end
+
+  should 'return all article fields except the ones defined in parameter' do
+    Article.destroy_all
+    article = fast_create(Article, :profile_id => user.person.id, :name => "Some thing")
+    params[:fields] = {:except => ['id', 'title']}
+    get "/api/v1/articles/?#{params.to_query}"
+    json = JSON.parse(last_response.body)
+    assert_not_includes json["articles"].first.keys, 'id'
+    assert_not_includes json["articles"].first.keys, 'title'
+  end
+
 end
@@ -100,7 +100,7 @@ class CommunitiesTest &lt; ActiveSupport::TestCase
     get "/api/v1/communities/#{community.id}?#{params.to_query}"
     json = JSON.parse(last_response.body)
     assert_equal community.id, json['community']['id']
-    assert_nil json['community']['members']
+    assert_nil json['community']['admins']
   end
   should 'get private community to logged member' do
@@ -108,6 +108,7 @@ class CommunitiesTest &lt; ActiveSupport::TestCase
     community = fast_create(Community, :environment_id => environment.id, :public_profile => false, :visible => true)
     community.add_member(person)
+    params[:optional_fields] = ['members']
     get "/api/v1/communities/#{community.id}?#{params.to_query}"
     json = JSON.parse(last_response.body)
     assert_equal community.id, json['community']['id']
@@ -333,4 +334,51 @@ class CommunitiesTest &lt; ActiveSupport::TestCase
     refute json['community']['additional_data'].has_key?('Rating')
   end
+  should 'not display members value by default' do
+    login_api
+    community = fast_create(Community, :environment_id => environment.id, :public_profile => false, :visible => true)
+    community.add_member(person)
+
+    get "/api/v1/communities/#{community.id}?#{params.to_query}"
+    json = JSON.parse(last_response.body)
+    assert_equal community.id, json['community']['id']
+    assert_nil json['community']['members']
+  end
+
+  should 'display members values if optional field parameter is passed' do
+    community = fast_create(Community, :environment_id => environment.id)
+
+    get "/api/v1/communities/#{community.id}?#{params.merge({:optional_fields => [:members]}).to_query}"
+    json = JSON.parse(last_response.body)
+    assert_equal community.id, json['community']['id']
+    assert_not_nil json['community']['members']
+  end
+
+  should 'display members values if optional_fields has members value as string in array' do
+    community = fast_create(Community, :environment_id => environment.id)
+
+    get "/api/v1/communities/#{community.id}?#{params.merge({:optional_fields => ['members']}).to_query}"
+    json = JSON.parse(last_response.body)
+    assert_equal community.id, json['community']['id']
+    assert_not_nil json['community']['members']
+  end
+
+  should 'display members values if optional_fields has members value in array' do
+    community = fast_create(Community, :environment_id => environment.id)
+
+    get "/api/v1/communities/#{community.id}?#{params.merge({:optional_fields => ['members', 'another']}).to_query}"
+    json = JSON.parse(last_response.body)
+    assert_equal community.id, json['community']['id']
+    assert_not_nil json['community']['members']
+  end
+
+  should 'display members values if optional_fields has members value as string' do
+    community = fast_create(Community, :environment_id => environment.id)
+
+    get "/api/v1/communities/#{community.id}?#{params.merge({:optional_fields => 'members'}).to_query}"
+    json = JSON.parse(last_response.body)
+    assert_equal community.id, json['community']['id']
+    assert_not_nil json['community']['members']
+  end
+
 end
@@ -233,7 +233,7 @@ class Api::HelpersTest &lt; ActiveSupport::TestCase
   should 'accept json as fields parameter when calling present partial' do
     model = mock
-    params[:fields] = {only: [:name, {user: [:login]}]}.to_json
+    params[:fields] = {only: [:name, {user: [:login]}]}
     expects(:present).with(model, {:only => ['name', {'user' => ['login']}]})
     present_partial(model, {})
   end
@@ -119,8 +119,8 @@ class PeopleTest &lt; ActiveSupport::TestCase
   should 'people endpoint filter by fields parameter with hierarchy for logged user' do
     login_api
-    fields = URI.encode({only: [:name, {user: [:login]}]}.to_json.to_str)
-    get "/api/v1/people?#{params.to_query}&fields=#{fields}"
+    params[:fields] = {only: [:name, {user: [:login]}]}
+    get "/api/v1/people?#{params.to_query}"
     json = JSON.parse(last_response.body)
     expected = {'people' => [{'name' => person.name, 'user' => {'login' => 'testapi'}}]}
     assert_equal expected, json
	@@ -18,7 +18,7 @@ module Api		@@ -18,7 +18,7 @@ module Api
18	communities = select_filtered_collection_of(environment, 'communities', params)	18	communities = select_filtered_collection_of(environment, 'communities', params)
19	communities = profiles_for_person(communities, current_person)	19	communities = profiles_for_person(communities, current_person)
20	communities = communities.by_location(params) # Must be the last. May return Exception obj	20	communities = communities.by_location(params) # Must be the last. May return Exception obj
21	- present communities, :with => Entities::Community, :current_person => current_person	21	+ present communities, :with => Entities::Community, :current_person => current_person, :params => params
22	end	22	end
23		23
24		24
	@@ -49,7 +49,7 @@ module Api		@@ -49,7 +49,7 @@ module Api
49		49
50	get ':id' do	50	get ':id' do
51	community = profiles_for_person(environment.communities, current_person).find_by_id(params[:id])	51	community = profiles_for_person(environment.communities, current_person).find_by_id(params[:id])
52	- present community, :with => Entities::Community, :current_person => current_person	52	+ present community, :with => Entities::Community, :current_person => current_person, :params => params
53	end	53	end
54		54
55	end	55	end
	@@ -773,12 +773,12 @@ class ArticlesTest < ActiveSupport::TestCase		@@ -773,12 +773,12 @@ class ArticlesTest < ActiveSupport::TestCase
773	end	773	end
774	end	774	end
775		775
776	- should 'only show article comments when show_comments is present' do	776	+ should 'only show article comments when optional_fields comments is present' do
777	person = fast_create(Person)	777	person = fast_create(Person)
778	article = fast_create(Article, :profile_id => person.id, :name => "Some thing")	778	article = fast_create(Article, :profile_id => person.id, :name => "Some thing")
779	article.comments.create!(:body => "another comment", :author => person)	779	article.comments.create!(:body => "another comment", :author => person)
780		780
781	- get "/api/v1/articles/#{article.id}/?#{params.merge(:show_comments => '1').to_query}"	781	+ get "/api/v1/articles/#{article.id}/?#{params.merge(:optional_fields => [:comments]).to_query}"
782	json = JSON.parse(last_response.body)	782	json = JSON.parse(last_response.body)
783	assert_includes json["article"].keys, "comments"	783	assert_includes json["article"].keys, "comments"
784	assert_equal json["article"]["comments"].first["body"], "another comment"	784	assert_equal json["article"]["comments"].first["body"], "another comment"
	@@ -805,4 +805,24 @@ class ArticlesTest < ActiveSupport::TestCase		@@ -805,4 +805,24 @@ class ArticlesTest < ActiveSupport::TestCase
805	json = JSON.parse(last_response.body)	805	json = JSON.parse(last_response.body)
806	assert_includes json["article"]["permissions"], 'allow_post_content'	806	assert_includes json["article"]["permissions"], 'allow_post_content'
807	end	807	end
		808	+
		809	+ should 'return only article fields defined in parameter' do
		810	+ Article.destroy_all
		811	+ article = fast_create(Article, :profile_id => user.person.id, :name => "Some thing")
		812	+ params[:fields] = {:only => ['id', 'title']}
		813	+ get "/api/v1/articles/?#{params.to_query}"
		814	+ json = JSON.parse(last_response.body)
		815	+ assert_equivalent ['id', 'title'], json["articles"].first.keys
		816	+ end
		817	+
		818	+ should 'return all article fields except the ones defined in parameter' do
		819	+ Article.destroy_all
		820	+ article = fast_create(Article, :profile_id => user.person.id, :name => "Some thing")
		821	+ params[:fields] = {:except => ['id', 'title']}
		822	+ get "/api/v1/articles/?#{params.to_query}"
		823	+ json = JSON.parse(last_response.body)
		824	+ assert_not_includes json["articles"].first.keys, 'id'
		825	+ assert_not_includes json["articles"].first.keys, 'title'
		826	+ end
		827	+
808	end	828	end
	@@ -100,7 +100,7 @@ class CommunitiesTest < ActiveSupport::TestCase		@@ -100,7 +100,7 @@ class CommunitiesTest < ActiveSupport::TestCase
100	get "/api/v1/communities/#{community.id}?#{params.to_query}"	100	get "/api/v1/communities/#{community.id}?#{params.to_query}"
101	json = JSON.parse(last_response.body)	101	json = JSON.parse(last_response.body)
102	assert_equal community.id, json['community']['id']	102	assert_equal community.id, json['community']['id']
103	- assert_nil json['community']['members']	103	+ assert_nil json['community']['admins']
104	end	104	end
105		105
106	should 'get private community to logged member' do	106	should 'get private community to logged member' do
	@@ -108,6 +108,7 @@ class CommunitiesTest < ActiveSupport::TestCase		@@ -108,6 +108,7 @@ class CommunitiesTest < ActiveSupport::TestCase
108	community = fast_create(Community, :environment_id => environment.id, :public_profile => false, :visible => true)	108	community = fast_create(Community, :environment_id => environment.id, :public_profile => false, :visible => true)
109	community.add_member(person)	109	community.add_member(person)
110		110
		111	+ params[:optional_fields] = ['members']
111	get "/api/v1/communities/#{community.id}?#{params.to_query}"	112	get "/api/v1/communities/#{community.id}?#{params.to_query}"
112	json = JSON.parse(last_response.body)	113	json = JSON.parse(last_response.body)
113	assert_equal community.id, json['community']['id']	114	assert_equal community.id, json['community']['id']
	@@ -333,4 +334,51 @@ class CommunitiesTest < ActiveSupport::TestCase		@@ -333,4 +334,51 @@ class CommunitiesTest < ActiveSupport::TestCase
333	refute json['community']['additional_data'].has_key?('Rating')	334	refute json['community']['additional_data'].has_key?('Rating')
334	end	335	end
335		336
		337	+ should 'not display members value by default' do
		338	+ login_api
		339	+ community = fast_create(Community, :environment_id => environment.id, :public_profile => false, :visible => true)
		340	+ community.add_member(person)
		341	+
		342	+ get "/api/v1/communities/#{community.id}?#{params.to_query}"
		343	+ json = JSON.parse(last_response.body)
		344	+ assert_equal community.id, json['community']['id']
		345	+ assert_nil json['community']['members']
		346	+ end
		347	+
		348	+ should 'display members values if optional field parameter is passed' do
		349	+ community = fast_create(Community, :environment_id => environment.id)
		350	+
		351	+ get "/api/v1/communities/#{community.id}?#{params.merge({:optional_fields => [:members]}).to_query}"
		352	+ json = JSON.parse(last_response.body)
		353	+ assert_equal community.id, json['community']['id']
		354	+ assert_not_nil json['community']['members']
		355	+ end
		356	+
		357	+ should 'display members values if optional_fields has members value as string in array' do
		358	+ community = fast_create(Community, :environment_id => environment.id)
		359	+
		360	+ get "/api/v1/communities/#{community.id}?#{params.merge({:optional_fields => ['members']}).to_query}"
		361	+ json = JSON.parse(last_response.body)
		362	+ assert_equal community.id, json['community']['id']
		363	+ assert_not_nil json['community']['members']
		364	+ end
		365	+
		366	+ should 'display members values if optional_fields has members value in array' do
		367	+ community = fast_create(Community, :environment_id => environment.id)
		368	+
		369	+ get "/api/v1/communities/#{community.id}?#{params.merge({:optional_fields => ['members', 'another']}).to_query}"
		370	+ json = JSON.parse(last_response.body)
		371	+ assert_equal community.id, json['community']['id']
		372	+ assert_not_nil json['community']['members']
		373	+ end
		374	+
		375	+ should 'display members values if optional_fields has members value as string' do
		376	+ community = fast_create(Community, :environment_id => environment.id)
		377	+
		378	+ get "/api/v1/communities/#{community.id}?#{params.merge({:optional_fields => 'members'}).to_query}"
		379	+ json = JSON.parse(last_response.body)
		380	+ assert_equal community.id, json['community']['id']
		381	+ assert_not_nil json['community']['members']
		382	+ end
		383	+
336	end	384	end
	@@ -233,7 +233,7 @@ class Api::HelpersTest < ActiveSupport::TestCase		@@ -233,7 +233,7 @@ class Api::HelpersTest < ActiveSupport::TestCase
233		233
234	should 'accept json as fields parameter when calling present partial' do	234	should 'accept json as fields parameter when calling present partial' do
235	model = mock	235	model = mock
236	- params[:fields] = {only: [:name, {user: [:login]}]}.to_json	236	+ params[:fields] = {only: [:name, {user: [:login]}]}
237	expects(:present).with(model, {:only => ['name', {'user' => ['login']}]})	237	expects(:present).with(model, {:only => ['name', {'user' => ['login']}]})
238	present_partial(model, {})	238	present_partial(model, {})
239	end	239	end