From 56f3aee23b7ebc5092f0d48b8dbddffb1785bba3 Mon Sep 17 00:00:00 2001 From: Victor Costa Date: Mon, 23 Nov 2015 11:57:27 -0300 Subject: [PATCH] api: better support to choose returned fields --- lib/noosfero/api/entities.rb | 8 ++++---- lib/noosfero/api/entity.rb | 14 -------------- lib/noosfero/api/helpers.rb | 28 +++++++++++++++++++++------- lib/noosfero/api/v1/articles.rb | 10 +++++----- lib/noosfero/api/v1/people.rb | 4 ++-- lib/noosfero/api/v1/tasks.rb | 4 ++-- test/unit/api/helpers_test.rb | 20 ++++++++++++++++++++ test/unit/api/people_test.rb | 14 +++++++++++--- 8 files changed, 65 insertions(+), 37 deletions(-) diff --git a/lib/noosfero/api/entities.rb b/lib/noosfero/api/entities.rb index b1a1592..f71d6cb 100644 --- a/lib/noosfero/api/entities.rb +++ b/lib/noosfero/api/entities.rb @@ -96,14 +96,14 @@ module Noosfero class Person < Profile root 'people', 'person' expose :user, :using => UserBasic, documentation: {type: 'User', desc: 'The user data of a person' } - expose :vote_count, if: lambda { |object, options| options[:fields].present? ? options[:fields].include?('vote_count') : false} - expose :comments_count, if: lambda { |object, options| options[:fields].present? ? options[:fields].include?('comments_count') : false} do |person, options| + expose :vote_count + expose :comments_count do |person, options| person.comments.count end - expose :following_articles_count, if: lambda { |object, options| options[:fields].present? ? options[:fields].include?('following_articles_count') : false} do |person, options| + expose :following_articles_count do |person, options| person.following_articles.count end - expose :articles_count, if: lambda { |object, options| options[:fields].present? ? options[:fields].include?('articles_count') : false} do |person, options| + expose :articles_count do |person, options| person.articles.count end diff --git a/lib/noosfero/api/entity.rb b/lib/noosfero/api/entity.rb index 767a379..51b55ab 100644 --- a/lib/noosfero/api/entity.rb +++ b/lib/noosfero/api/entity.rb @@ -22,18 +22,4 @@ class Noosfero::API::Entity < Grape::Entity end end - def self.fields_condition(fields) - lambda do |object, options| - return true if options[:fields].blank? - fields.map { |field| options[:fields].include?(field.to_s)}.grep(true).present? - end - end - - def self.expose(*args, &block) - hash = args.last.is_a?(Hash) ? args.pop : {} - hash.merge!({:if => fields_condition(args)}) if hash[:if].blank? - args << hash - super - end - end diff --git a/lib/noosfero/api/helpers.rb b/lib/noosfero/api/helpers.rb index 168d17b..02295b3 100644 --- a/lib/noosfero/api/helpers.rb +++ b/lib/noosfero/api/helpers.rb @@ -39,6 +39,20 @@ require_relative '../../find_by_contents' @environment end + def present_partial(model, options) + if(params[:fields].present?) + begin + fields = JSON.parse(params[:fields]) + if fields.present? + options.merge!(fields.symbolize_keys.slice(:only, :except)) + end + rescue + options[:only] = Array.wrap(params[:fields]) + end + end + present model, options + end + include FindByContents #################################################################### @@ -99,12 +113,12 @@ require_relative '../../find_by_contents' if !article.save render_api_errors!(article.errors.full_messages) end - present article, :with => Entities::Article, :fields => params[:fields] + present_partial article, :with => Entities::Article end def present_article(asset) article = find_article(asset.articles, params[:id]) - present article, :with => Entities::Article, :fields => params[:fields] + present_partial article, :with => Entities::Article end def present_articles_for_asset(asset, method = 'articles') @@ -113,12 +127,12 @@ require_relative '../../find_by_contents' end def present_articles(articles) - present articles, :with => Entities::Article, :fields => params[:fields] + present_partial articles, :with => Entities::Article end def present_articles_paginated(articles, per_page=nil) articles = paginate(articles) - present articles, :with => Entities::Article, :fields => params[:fields] + present_partial articles, :with => Entities::Article end def find_articles(asset, method = 'articles') @@ -148,19 +162,19 @@ require_relative '../../find_by_contents' if !task.save render_api_errors!(task.errors.full_messages) end - present task, :with => Entities::Task, :fields => params[:fields] + present_partial task, :with => Entities::Task end def present_task(asset) task = find_task(asset, params[:id]) - present task, :with => Entities::Task, :fields => params[:fields] + present_partial task, :with => Entities::Task end def present_tasks(asset) tasks = select_filtered_collection_of(asset, 'tasks', params) tasks = tasks.select {|t| current_person.has_permission?(t.permission, asset)} return forbidden! if tasks.empty? && !current_person.has_permission?(:perform_task, asset) - present tasks, :with => Entities::Task, :fields => params[:fields] + present_partial tasks, :with => Entities::Task end def make_conditions_with_parameter(params = {}) diff --git a/lib/noosfero/api/v1/articles.rb b/lib/noosfero/api/v1/articles.rb index a0a3331..a37b196 100644 --- a/lib/noosfero/api/v1/articles.rb +++ b/lib/noosfero/api/v1/articles.rb @@ -58,7 +58,7 @@ module Noosfero article = environment.articles.find(params[:id]) return forbidden! unless article.allow_edit?(current_person) article.update_attributes!(params[:article]) - present article, :with => Entities::Article, :fields => params[:fields] + present_partial article, :with => Entities::Article end desc 'Report a abuse and/or violent content in a article by id' do @@ -190,7 +190,7 @@ module Noosfero article = find_article(environment.articles, params[:id]) child = find_article(article.children, params[:child_id]) child.hit - present child, :with => Entities::Article, :fields => params[:fields] + present_partial child, :with => Entities::Article end desc 'Suggest a article to another profile' do @@ -213,7 +213,7 @@ module Noosfero unless suggest_article.save render_api_errors!(suggest_article.article_object.errors.full_messages) end - present suggest_article, :with => Entities::Task, :fields => params[:fields] + present_partial suggest_article, :with => Entities::Task end # Example Request: @@ -244,7 +244,7 @@ module Noosfero if !article.save render_api_errors!(article.errors.full_messages) end - present article, :with => Entities::Article, :fields => params[:fields] + present_partial article, :with => Entities::Article end end @@ -271,7 +271,7 @@ module Noosfero article = forbidden! end - present article, :with => Entities::Article, :fields => params[:fields] + present_partial article, :with => Entities::Article else present_articles_for_asset(profile) diff --git a/lib/noosfero/api/v1/people.rb b/lib/noosfero/api/v1/people.rb index 74bdcf1..08c421d 100644 --- a/lib/noosfero/api/v1/people.rb +++ b/lib/noosfero/api/v1/people.rb @@ -33,12 +33,12 @@ module Noosfero get do people = select_filtered_collection_of(environment, 'people', params) people = people.visible_for_person(current_person) - present people, :with => Entities::Person, :fields => params[:fields] + present_partial people, :with => Entities::Person end desc "Return the logged user information" get "/me" do - present current_person, :with => Entities::Person, :fields => params[:fields] + present_partial current_person, :with => Entities::Person end desc "Return the person information" diff --git a/lib/noosfero/api/v1/tasks.rb b/lib/noosfero/api/v1/tasks.rb index feec86e..23d6f4a 100644 --- a/lib/noosfero/api/v1/tasks.rb +++ b/lib/noosfero/api/v1/tasks.rb @@ -20,13 +20,13 @@ module Noosfero get do tasks = select_filtered_collection_of(environment, 'tasks', params) tasks = tasks.select {|t| current_person.has_permission?(t.permission, environment)} - present tasks, :with => Entities::Task, :fields => params[:fields] + present_partial tasks, :with => Entities::Task end desc "Return the task id" get ':id' do task = find_task(environment, params[:id]) - present task, :with => Entities::Task, :fields => params[:fields] + present_partial task, :with => Entities::Task end end diff --git a/test/unit/api/helpers_test.rb b/test/unit/api/helpers_test.rb index eeae0d6..ba651ea 100644 --- a/test/unit/api/helpers_test.rb +++ b/test/unit/api/helpers_test.rb @@ -214,6 +214,26 @@ class APIHelpersTest < ActiveSupport::TestCase #assert_equals [article1, article2], present_articles end + should 'not touch in options when no fields parameter is passed' do + model = mock + expects(:present).with(model, {}) + present_partial(model, {}) + end + + should 'fallback to array when fields parameter is not a json when calling present partial' do + model = mock + params[:fields] = 'name' + expects(:present).with(model, {:only => ['name']}) + present_partial(model, {}) + end + + should 'accept json as fields parameter when calling present partial' do + model = mock + params[:fields] = {only: [:name, {user: [:login]}]}.to_json + expects(:present).with(model, {:only => ['name', {'user' => ['login']}]}) + present_partial(model, {}) + end + ###### Captcha tests ###### should 'do not test captcha when there are no settings' do diff --git a/test/unit/api/people_test.rb b/test/unit/api/people_test.rb index dfae0b5..5483511 100644 --- a/test/unit/api/people_test.rb +++ b/test/unit/api/people_test.rb @@ -54,6 +54,14 @@ class PeopleTest < ActiveSupport::TestCase assert_equal expected, json end + should 'people endpoint filter by fields parameter with hierarchy' do + fields = {only: [:name, {user: [:login]}]}.to_json + get "/api/v1/people?#{params.to_query}&fields=#{fields}" + json = JSON.parse(last_response.body) + expected = {'people' => [{'name' => person.name, 'user' => {'login' => 'testapi'}}]} + assert_equal expected, json + end + should 'get logged person' do get "/api/v1/people/me?#{params.to_query}" @@ -184,13 +192,13 @@ class PeopleTest < ActiveSupport::TestCase PERSON_ATTRIBUTES.map do |attribute| - define_method "test_should_not_expose_#{attribute}_attribute_in_person_enpoint_if_field_parameter_is_not_passed" do - get "/api/v1/people/me?#{params.to_query}" + define_method "test_should_not_expose_#{attribute}_attribute_in_person_enpoint_if_field_parameter_does_not_contain_the_attribute" do + get "/api/v1/people/me?#{params.to_query}&fields=name" json = JSON.parse(last_response.body) assert_nil json['person'][attribute] end - define_method "test_should_expose_#{attribute}_attribute_in_person_enpoints_only_if_field_parameter_is_passed" do + define_method "test_should_expose_#{attribute}_attribute_in_person_enpoints_if_field_parameter_is_passed" do get "/api/v1/people/me?#{params.to_query}&fields=#{attribute}" json = JSON.parse(last_response.body) assert_not_nil json['person'][attribute] -- libgit2 0.21.2