Commit 56f3aee23b7ebc5092f0d48b8dbddffb1785bba3
1 parent
92fbfd94
Exists in
theme-brasil-digital-from-staging
and in
4 other branches
api: better support to choose returned fields
Showing
8 changed files
with
65 additions
and
37 deletions
Show diff stats
lib/noosfero/api/entities.rb
| ... | ... | @@ -96,14 +96,14 @@ module Noosfero |
| 96 | 96 | class Person < Profile |
| 97 | 97 | root 'people', 'person' |
| 98 | 98 | expose :user, :using => UserBasic, documentation: {type: 'User', desc: 'The user data of a person' } |
| 99 | - expose :vote_count, if: lambda { |object, options| options[:fields].present? ? options[:fields].include?('vote_count') : false} | |
| 100 | - expose :comments_count, if: lambda { |object, options| options[:fields].present? ? options[:fields].include?('comments_count') : false} do |person, options| | |
| 99 | + expose :vote_count | |
| 100 | + expose :comments_count do |person, options| | |
| 101 | 101 | person.comments.count |
| 102 | 102 | end |
| 103 | - expose :following_articles_count, if: lambda { |object, options| options[:fields].present? ? options[:fields].include?('following_articles_count') : false} do |person, options| | |
| 103 | + expose :following_articles_count do |person, options| | |
| 104 | 104 | person.following_articles.count |
| 105 | 105 | end |
| 106 | - expose :articles_count, if: lambda { |object, options| options[:fields].present? ? options[:fields].include?('articles_count') : false} do |person, options| | |
| 106 | + expose :articles_count do |person, options| | |
| 107 | 107 | person.articles.count |
| 108 | 108 | end |
| 109 | 109 | ... | ... |
lib/noosfero/api/entity.rb
| ... | ... | @@ -22,18 +22,4 @@ class Noosfero::API::Entity < Grape::Entity |
| 22 | 22 | end |
| 23 | 23 | end |
| 24 | 24 | |
| 25 | - def self.fields_condition(fields) | |
| 26 | - lambda do |object, options| | |
| 27 | - return true if options[:fields].blank? | |
| 28 | - fields.map { |field| options[:fields].include?(field.to_s)}.grep(true).present? | |
| 29 | - end | |
| 30 | - end | |
| 31 | - | |
| 32 | - def self.expose(*args, &block) | |
| 33 | - hash = args.last.is_a?(Hash) ? args.pop : {} | |
| 34 | - hash.merge!({:if => fields_condition(args)}) if hash[:if].blank? | |
| 35 | - args << hash | |
| 36 | - super | |
| 37 | - end | |
| 38 | - | |
| 39 | 25 | end | ... | ... |
lib/noosfero/api/helpers.rb
| ... | ... | @@ -39,6 +39,20 @@ require_relative '../../find_by_contents' |
| 39 | 39 | @environment |
| 40 | 40 | end |
| 41 | 41 | |
| 42 | + def present_partial(model, options) | |
| 43 | + if(params[:fields].present?) | |
| 44 | + begin | |
| 45 | + fields = JSON.parse(params[:fields]) | |
| 46 | + if fields.present? | |
| 47 | + options.merge!(fields.symbolize_keys.slice(:only, :except)) | |
| 48 | + end | |
| 49 | + rescue | |
| 50 | + options[:only] = Array.wrap(params[:fields]) | |
| 51 | + end | |
| 52 | + end | |
| 53 | + present model, options | |
| 54 | + end | |
| 55 | + | |
| 42 | 56 | include FindByContents |
| 43 | 57 | |
| 44 | 58 | #################################################################### |
| ... | ... | @@ -99,12 +113,12 @@ require_relative '../../find_by_contents' |
| 99 | 113 | if !article.save |
| 100 | 114 | render_api_errors!(article.errors.full_messages) |
| 101 | 115 | end |
| 102 | - present article, :with => Entities::Article, :fields => params[:fields] | |
| 116 | + present_partial article, :with => Entities::Article | |
| 103 | 117 | end |
| 104 | 118 | |
| 105 | 119 | def present_article(asset) |
| 106 | 120 | article = find_article(asset.articles, params[:id]) |
| 107 | - present article, :with => Entities::Article, :fields => params[:fields] | |
| 121 | + present_partial article, :with => Entities::Article | |
| 108 | 122 | end |
| 109 | 123 | |
| 110 | 124 | def present_articles_for_asset(asset, method = 'articles') |
| ... | ... | @@ -113,12 +127,12 @@ require_relative '../../find_by_contents' |
| 113 | 127 | end |
| 114 | 128 | |
| 115 | 129 | def present_articles(articles) |
| 116 | - present articles, :with => Entities::Article, :fields => params[:fields] | |
| 130 | + present_partial articles, :with => Entities::Article | |
| 117 | 131 | end |
| 118 | 132 | |
| 119 | 133 | def present_articles_paginated(articles, per_page=nil) |
| 120 | 134 | articles = paginate(articles) |
| 121 | - present articles, :with => Entities::Article, :fields => params[:fields] | |
| 135 | + present_partial articles, :with => Entities::Article | |
| 122 | 136 | end |
| 123 | 137 | |
| 124 | 138 | def find_articles(asset, method = 'articles') |
| ... | ... | @@ -148,19 +162,19 @@ require_relative '../../find_by_contents' |
| 148 | 162 | if !task.save |
| 149 | 163 | render_api_errors!(task.errors.full_messages) |
| 150 | 164 | end |
| 151 | - present task, :with => Entities::Task, :fields => params[:fields] | |
| 165 | + present_partial task, :with => Entities::Task | |
| 152 | 166 | end |
| 153 | 167 | |
| 154 | 168 | def present_task(asset) |
| 155 | 169 | task = find_task(asset, params[:id]) |
| 156 | - present task, :with => Entities::Task, :fields => params[:fields] | |
| 170 | + present_partial task, :with => Entities::Task | |
| 157 | 171 | end |
| 158 | 172 | |
| 159 | 173 | def present_tasks(asset) |
| 160 | 174 | tasks = select_filtered_collection_of(asset, 'tasks', params) |
| 161 | 175 | tasks = tasks.select {|t| current_person.has_permission?(t.permission, asset)} |
| 162 | 176 | return forbidden! if tasks.empty? && !current_person.has_permission?(:perform_task, asset) |
| 163 | - present tasks, :with => Entities::Task, :fields => params[:fields] | |
| 177 | + present_partial tasks, :with => Entities::Task | |
| 164 | 178 | end |
| 165 | 179 | |
| 166 | 180 | def make_conditions_with_parameter(params = {}) | ... | ... |
lib/noosfero/api/v1/articles.rb
| ... | ... | @@ -58,7 +58,7 @@ module Noosfero |
| 58 | 58 | article = environment.articles.find(params[:id]) |
| 59 | 59 | return forbidden! unless article.allow_edit?(current_person) |
| 60 | 60 | article.update_attributes!(params[:article]) |
| 61 | - present article, :with => Entities::Article, :fields => params[:fields] | |
| 61 | + present_partial article, :with => Entities::Article | |
| 62 | 62 | end |
| 63 | 63 | |
| 64 | 64 | desc 'Report a abuse and/or violent content in a article by id' do |
| ... | ... | @@ -190,7 +190,7 @@ module Noosfero |
| 190 | 190 | article = find_article(environment.articles, params[:id]) |
| 191 | 191 | child = find_article(article.children, params[:child_id]) |
| 192 | 192 | child.hit |
| 193 | - present child, :with => Entities::Article, :fields => params[:fields] | |
| 193 | + present_partial child, :with => Entities::Article | |
| 194 | 194 | end |
| 195 | 195 | |
| 196 | 196 | desc 'Suggest a article to another profile' do |
| ... | ... | @@ -213,7 +213,7 @@ module Noosfero |
| 213 | 213 | unless suggest_article.save |
| 214 | 214 | render_api_errors!(suggest_article.article_object.errors.full_messages) |
| 215 | 215 | end |
| 216 | - present suggest_article, :with => Entities::Task, :fields => params[:fields] | |
| 216 | + present_partial suggest_article, :with => Entities::Task | |
| 217 | 217 | end |
| 218 | 218 | |
| 219 | 219 | # Example Request: |
| ... | ... | @@ -244,7 +244,7 @@ module Noosfero |
| 244 | 244 | if !article.save |
| 245 | 245 | render_api_errors!(article.errors.full_messages) |
| 246 | 246 | end |
| 247 | - present article, :with => Entities::Article, :fields => params[:fields] | |
| 247 | + present_partial article, :with => Entities::Article | |
| 248 | 248 | end |
| 249 | 249 | |
| 250 | 250 | end |
| ... | ... | @@ -271,7 +271,7 @@ module Noosfero |
| 271 | 271 | article = forbidden! |
| 272 | 272 | end |
| 273 | 273 | |
| 274 | - present article, :with => Entities::Article, :fields => params[:fields] | |
| 274 | + present_partial article, :with => Entities::Article | |
| 275 | 275 | else |
| 276 | 276 | |
| 277 | 277 | present_articles_for_asset(profile) | ... | ... |
lib/noosfero/api/v1/people.rb
| ... | ... | @@ -33,12 +33,12 @@ module Noosfero |
| 33 | 33 | get do |
| 34 | 34 | people = select_filtered_collection_of(environment, 'people', params) |
| 35 | 35 | people = people.visible_for_person(current_person) |
| 36 | - present people, :with => Entities::Person, :fields => params[:fields] | |
| 36 | + present_partial people, :with => Entities::Person | |
| 37 | 37 | end |
| 38 | 38 | |
| 39 | 39 | desc "Return the logged user information" |
| 40 | 40 | get "/me" do |
| 41 | - present current_person, :with => Entities::Person, :fields => params[:fields] | |
| 41 | + present_partial current_person, :with => Entities::Person | |
| 42 | 42 | end |
| 43 | 43 | |
| 44 | 44 | desc "Return the person information" | ... | ... |
lib/noosfero/api/v1/tasks.rb
| ... | ... | @@ -20,13 +20,13 @@ module Noosfero |
| 20 | 20 | get do |
| 21 | 21 | tasks = select_filtered_collection_of(environment, 'tasks', params) |
| 22 | 22 | tasks = tasks.select {|t| current_person.has_permission?(t.permission, environment)} |
| 23 | - present tasks, :with => Entities::Task, :fields => params[:fields] | |
| 23 | + present_partial tasks, :with => Entities::Task | |
| 24 | 24 | end |
| 25 | 25 | |
| 26 | 26 | desc "Return the task id" |
| 27 | 27 | get ':id' do |
| 28 | 28 | task = find_task(environment, params[:id]) |
| 29 | - present task, :with => Entities::Task, :fields => params[:fields] | |
| 29 | + present_partial task, :with => Entities::Task | |
| 30 | 30 | end |
| 31 | 31 | end |
| 32 | 32 | ... | ... |
test/unit/api/helpers_test.rb
| ... | ... | @@ -214,6 +214,26 @@ class APIHelpersTest < ActiveSupport::TestCase |
| 214 | 214 | #assert_equals [article1, article2], present_articles |
| 215 | 215 | end |
| 216 | 216 | |
| 217 | + should 'not touch in options when no fields parameter is passed' do | |
| 218 | + model = mock | |
| 219 | + expects(:present).with(model, {}) | |
| 220 | + present_partial(model, {}) | |
| 221 | + end | |
| 222 | + | |
| 223 | + should 'fallback to array when fields parameter is not a json when calling present partial' do | |
| 224 | + model = mock | |
| 225 | + params[:fields] = 'name' | |
| 226 | + expects(:present).with(model, {:only => ['name']}) | |
| 227 | + present_partial(model, {}) | |
| 228 | + end | |
| 229 | + | |
| 230 | + should 'accept json as fields parameter when calling present partial' do | |
| 231 | + model = mock | |
| 232 | + params[:fields] = {only: [:name, {user: [:login]}]}.to_json | |
| 233 | + expects(:present).with(model, {:only => ['name', {'user' => ['login']}]}) | |
| 234 | + present_partial(model, {}) | |
| 235 | + end | |
| 236 | + | |
| 217 | 237 | ###### Captcha tests ###### |
| 218 | 238 | |
| 219 | 239 | should 'do not test captcha when there are no settings' do | ... | ... |
test/unit/api/people_test.rb
| ... | ... | @@ -54,6 +54,14 @@ class PeopleTest < ActiveSupport::TestCase |
| 54 | 54 | assert_equal expected, json |
| 55 | 55 | end |
| 56 | 56 | |
| 57 | + should 'people endpoint filter by fields parameter with hierarchy' do | |
| 58 | + fields = {only: [:name, {user: [:login]}]}.to_json | |
| 59 | + get "/api/v1/people?#{params.to_query}&fields=#{fields}" | |
| 60 | + json = JSON.parse(last_response.body) | |
| 61 | + expected = {'people' => [{'name' => person.name, 'user' => {'login' => 'testapi'}}]} | |
| 62 | + assert_equal expected, json | |
| 63 | + end | |
| 64 | + | |
| 57 | 65 | |
| 58 | 66 | should 'get logged person' do |
| 59 | 67 | get "/api/v1/people/me?#{params.to_query}" |
| ... | ... | @@ -184,13 +192,13 @@ class PeopleTest < ActiveSupport::TestCase |
| 184 | 192 | |
| 185 | 193 | PERSON_ATTRIBUTES.map do |attribute| |
| 186 | 194 | |
| 187 | - define_method "test_should_not_expose_#{attribute}_attribute_in_person_enpoint_if_field_parameter_is_not_passed" do | |
| 188 | - get "/api/v1/people/me?#{params.to_query}" | |
| 195 | + define_method "test_should_not_expose_#{attribute}_attribute_in_person_enpoint_if_field_parameter_does_not_contain_the_attribute" do | |
| 196 | + get "/api/v1/people/me?#{params.to_query}&fields=name" | |
| 189 | 197 | json = JSON.parse(last_response.body) |
| 190 | 198 | assert_nil json['person'][attribute] |
| 191 | 199 | end |
| 192 | 200 | |
| 193 | - define_method "test_should_expose_#{attribute}_attribute_in_person_enpoints_only_if_field_parameter_is_passed" do | |
| 201 | + define_method "test_should_expose_#{attribute}_attribute_in_person_enpoints_if_field_parameter_is_passed" do | |
| 194 | 202 | get "/api/v1/people/me?#{params.to_query}&fields=#{attribute}" |
| 195 | 203 | json = JSON.parse(last_response.body) |
| 196 | 204 | assert_not_nil json['person'][attribute] | ... | ... |