Commit d86bcb8f826b4dd67d1ba18485a2c84b717735fd
Exists in
staging
and in
4 other branches
Merge branch 'show_scrap_activity' into 'master'
Show scrap activity Today an scrap activity crash /activities api endpoint See merge request !986
Showing
6 changed files
with
162 additions
and
28 deletions
Show diff stats
app/api/entities.rb
| ... | ... | @@ -295,12 +295,19 @@ module Api |
| 295 | 295 | |
| 296 | 296 | class Activity < Entity |
| 297 | 297 | root 'activities', 'activity' |
| 298 | - expose :id, :params, :verb, :created_at, :updated_at, :comments_count, :visible | |
| 298 | + expose :id, :created_at, :updated_at | |
| 299 | 299 | expose :user, :using => Profile |
| 300 | + | |
| 300 | 301 | expose :target do |activity, opts| |
| 301 | 302 | type_map = {Profile => ::Profile, ArticleBase => ::Article}.find {|h| activity.target.kind_of?(h.last)} |
| 302 | 303 | type_map.first.represent(activity.target) unless type_map.nil? |
| 303 | 304 | end |
| 305 | + expose :params, :if => lambda { |activity, options| activity.kind_of?(ActionTracker::Record)} | |
| 306 | + expose :content, :if => lambda { |activity, options| activity.kind_of?(Scrap)} | |
| 307 | + expose :verb do |activity, options| | |
| 308 | + activity.kind_of?(Scrap) ? 'leave_scrap' : activity.verb | |
| 309 | + end | |
| 310 | + | |
| 304 | 311 | end |
| 305 | 312 | |
| 306 | 313 | class Role < Entity | ... | ... |
app/api/helpers.rb
| ... | ... | @@ -187,6 +187,28 @@ module Api |
| 187 | 187 | present_partial tasks, :with => Entities::Task |
| 188 | 188 | end |
| 189 | 189 | |
| 190 | + ########################### | |
| 191 | + # Activities # | |
| 192 | + ########################### | |
| 193 | + def find_activities(asset, method_or_relation = 'activities') | |
| 194 | + | |
| 195 | + not_found! if asset.blank? || asset.secret || !asset.visible | |
| 196 | + forbidden! if !asset.display_private_info_to?(current_person) | |
| 197 | + | |
| 198 | + activities = select_filtered_collection_of(asset, method_or_relation, params) | |
| 199 | + activities = activities.map(&:activity) | |
| 200 | + activities | |
| 201 | + end | |
| 202 | + | |
| 203 | + def present_activities_for_asset(asset, method_or_relation = 'activities') | |
| 204 | + tasks = find_activities(asset, method_or_relation) | |
| 205 | + present_activities(tasks) | |
| 206 | + end | |
| 207 | + | |
| 208 | + def present_activities(activities) | |
| 209 | + present_partial activities, :with => Entities::Activity, :current_person => current_person | |
| 210 | + end | |
| 211 | + | |
| 190 | 212 | def make_conditions_with_parameter(params = {}) |
| 191 | 213 | parsed_params = parser_params(params) |
| 192 | 214 | conditions = {} |
| ... | ... | @@ -210,7 +232,7 @@ module Api |
| 210 | 232 | order = 'RANDOM()' |
| 211 | 233 | else |
| 212 | 234 | field_name, direction = params[:order].split(' ') |
| 213 | - assoc_class = extract_associated_classname(method_or_relation) | |
| 235 | + assoc_class = extract_associated_classname(object, method_or_relation) | |
| 214 | 236 | if !field_name.blank? and assoc_class |
| 215 | 237 | if assoc_class.attribute_names.include? field_name |
| 216 | 238 | if direction.present? and ['ASC','DESC'].include? direction.upcase |
| ... | ... | @@ -223,12 +245,12 @@ module Api |
| 223 | 245 | return order |
| 224 | 246 | end |
| 225 | 247 | |
| 226 | - def make_timestamp_with_parameters_and_method(params, method_or_relation) | |
| 248 | + def make_timestamp_with_parameters_and_method(object, method_or_relation, params) | |
| 227 | 249 | timestamp = nil |
| 228 | 250 | if params[:timestamp] |
| 229 | 251 | datetime = DateTime.parse(params[:timestamp]) |
| 230 | - table_name = extract_associated_tablename(method_or_relation) | |
| 231 | - assoc_class = extract_associated_classname(method_or_relation) | |
| 252 | + table_name = extract_associated_tablename(object, method_or_relation) | |
| 253 | + assoc_class = extract_associated_classname(object, method_or_relation) | |
| 232 | 254 | date_atrr = assoc_class.attribute_names.include?('updated_at') ? 'updated_at' : 'created_at' |
| 233 | 255 | timestamp = "#{table_name}.#{date_atrr} >= '#{datetime}'" |
| 234 | 256 | end |
| ... | ... | @@ -258,7 +280,7 @@ module Api |
| 258 | 280 | def select_filtered_collection_of(object, method_or_relation, params) |
| 259 | 281 | conditions = make_conditions_with_parameter(params) |
| 260 | 282 | order = make_order_with_parameters(object,method_or_relation,params) |
| 261 | - timestamp = make_timestamp_with_parameters_and_method(params, method_or_relation) | |
| 283 | + timestamp = make_timestamp_with_parameters_and_method(object, method_or_relation, params) | |
| 262 | 284 | |
| 263 | 285 | objects = is_a_relation?(method_or_relation) ? method_or_relation : object.send(method_or_relation) |
| 264 | 286 | objects = by_reference(objects, params) |
| ... | ... | @@ -406,15 +428,15 @@ module Api |
| 406 | 428 | end |
| 407 | 429 | private |
| 408 | 430 | |
| 409 | - def extract_associated_tablename(method_or_relation) | |
| 410 | - extract_associated_classname(method_or_relation).table_name | |
| 431 | + def extract_associated_tablename(object, method_or_relation) | |
| 432 | + extract_associated_classname(object, method_or_relation).table_name | |
| 411 | 433 | end |
| 412 | 434 | |
| 413 | - def extract_associated_classname(method_or_relation) | |
| 435 | + def extract_associated_classname(object, method_or_relation) | |
| 414 | 436 | if is_a_relation?(method_or_relation) |
| 415 | 437 | method_or_relation.blank? ? '' : method_or_relation.first.class |
| 416 | 438 | else |
| 417 | - method_or_relation.to_s.singularize.camelize.constantize | |
| 439 | + object.send(method_or_relation).table_name.singularize.camelize.constantize | |
| 418 | 440 | end |
| 419 | 441 | end |
| 420 | 442 | ... | ... |
app/api/v1/activities.rb
| ... | ... | @@ -5,13 +5,8 @@ module Api |
| 5 | 5 | resource :profiles do |
| 6 | 6 | |
| 7 | 7 | get ':id/activities' do |
| 8 | - profile = Profile.find_by id: params[:id] | |
| 9 | - | |
| 10 | - not_found! if profile.blank? || profile.secret || !profile.visible | |
| 11 | - forbidden! if !profile.display_private_info_to?(current_person) | |
| 12 | - | |
| 13 | - activities = profile.activities.map(&:activity) | |
| 14 | - present activities, :with => Entities::Activity, :current_person => current_person | |
| 8 | + profile = environment.profiles.find_by id: params[:id] | |
| 9 | + present_activities_for_asset(profile) | |
| 15 | 10 | end |
| 16 | 11 | end |
| 17 | 12 | end | ... | ... |
app/models/scrap.rb
test/api/activities_test.rb
| ... | ... | @@ -8,7 +8,7 @@ class ActivitiesTest < ActiveSupport::TestCase |
| 8 | 8 | end |
| 9 | 9 | |
| 10 | 10 | should 'get own activities' do |
| 11 | - create_activity(person) | |
| 11 | + create_activity(:target => person) | |
| 12 | 12 | |
| 13 | 13 | get "/api/v1/profiles/#{person.id}/activities?#{params.to_query}" |
| 14 | 14 | json = JSON.parse(last_response.body) |
| ... | ... | @@ -19,7 +19,7 @@ class ActivitiesTest < ActiveSupport::TestCase |
| 19 | 19 | |
| 20 | 20 | should 'not get private community activities' do |
| 21 | 21 | community = fast_create(Community, :public_profile => false) |
| 22 | - create_activity(community) | |
| 22 | + create_activity(:target => community) | |
| 23 | 23 | |
| 24 | 24 | get "/api/v1/profiles/#{community.id}/activities?#{params.to_query}" |
| 25 | 25 | json = JSON.parse(last_response.body) |
| ... | ... | @@ -40,7 +40,7 @@ class ActivitiesTest < ActiveSupport::TestCase |
| 40 | 40 | |
| 41 | 41 | should 'get community activities for member' do |
| 42 | 42 | community = fast_create(Community) |
| 43 | - create_activity(community) | |
| 43 | + create_activity(:target => community) | |
| 44 | 44 | community.add_member(person) |
| 45 | 45 | |
| 46 | 46 | get "/api/v1/profiles/#{community.id}/activities?#{params.to_query}" |
| ... | ... | @@ -50,7 +50,7 @@ class ActivitiesTest < ActiveSupport::TestCase |
| 50 | 50 | |
| 51 | 51 | should 'not get other person activities' do |
| 52 | 52 | other_person = fast_create(Person) |
| 53 | - create_activity(other_person) | |
| 53 | + create_activity(:target => other_person) | |
| 54 | 54 | |
| 55 | 55 | get "/api/v1/profiles/#{other_person.id}/activities?#{params.to_query}" |
| 56 | 56 | json = JSON.parse(last_response.body) |
| ... | ... | @@ -61,7 +61,7 @@ class ActivitiesTest < ActiveSupport::TestCase |
| 61 | 61 | should 'get friend activities' do |
| 62 | 62 | other_person = fast_create(Person) |
| 63 | 63 | other_person.add_friend(person) |
| 64 | - create_activity(other_person) | |
| 64 | + create_activity(:target => other_person) | |
| 65 | 65 | |
| 66 | 66 | get "/api/v1/profiles/#{other_person.id}/activities?#{params.to_query}" |
| 67 | 67 | json = JSON.parse(last_response.body) |
| ... | ... | @@ -70,16 +70,123 @@ class ActivitiesTest < ActiveSupport::TestCase |
| 70 | 70 | |
| 71 | 71 | should 'get activities for non logged user in a public community' do |
| 72 | 72 | community = fast_create(Community) |
| 73 | - create_activity(community) | |
| 73 | + create_activity(:target => community) | |
| 74 | 74 | community.add_member(person) |
| 75 | 75 | get "/api/v1/profiles/#{community.id}/activities?#{params.to_query}" |
| 76 | 76 | json = JSON.parse(last_response.body) |
| 77 | 77 | assert_equivalent community.activities.map(&:activity).map(&:id), json["activities"].map{|c| c["id"]} |
| 78 | 78 | end |
| 79 | 79 | |
| 80 | - def create_activity(target) | |
| 81 | - activity = ActionTracker::Record.create! :verb => :leave_scrap, :user => person, :target => target | |
| 82 | - ProfileActivity.create! profile_id: target.id, activity: activity | |
| 80 | + should 'not crash api if an scrap activity is in the list' do | |
| 81 | + create_activity(:target => person) | |
| 82 | + create(Scrap, :sender_id => person.id, :receiver_id => person.id) | |
| 83 | + | |
| 84 | + assert_nothing_raised NoMethodError do | |
| 85 | + get "/api/v1/profiles/#{person.id}/activities?#{params.to_query}" | |
| 86 | + end | |
| 87 | + end | |
| 88 | + | |
| 89 | + should 'scrap activity be returned in acitivities list' do | |
| 90 | + create_activity(:target => person) | |
| 91 | + create(Scrap, :sender_id => person.id, :receiver_id => person.id) | |
| 92 | + | |
| 93 | + get "/api/v1/profiles/#{person.id}/activities?#{params.to_query}" | |
| 94 | + json = JSON.parse(last_response.body) | |
| 95 | + | |
| 96 | + assert_equivalent person.activities.map(&:activity).map(&:id), json["activities"].map{|c| c["id"]} | |
| 97 | + end | |
| 98 | + | |
| 99 | + should 'always return the activity verb parameter' do | |
| 100 | + ActionTracker::Record.destroy_all | |
| 101 | + ProfileActivity.destroy_all | |
| 102 | + create_activity(:target => person) | |
| 103 | + get "/api/v1/profiles/#{person.id}/activities?#{params.to_query}" | |
| 104 | + json = JSON.parse(last_response.body) | |
| 105 | + assert_equal 'create_article', json["activities"].last['verb'] | |
| 106 | + end | |
| 107 | + | |
| 108 | + should 'scrap activity return leave_scrap verb' do | |
| 109 | + ActionTracker::Record.destroy_all | |
| 110 | + create(TinyMceArticle, :name => 'Tracked Article 1', :profile_id => person.id) | |
| 111 | + create(Scrap, :sender_id => person.id, :receiver_id => person.id) | |
| 112 | + get "/api/v1/profiles/#{person.id}/activities?#{params.to_query}" | |
| 113 | + json = JSON.parse(last_response.body) | |
| 114 | + assert_equivalent ['create_article', 'leave_scrap'], json["activities"].map{|a|a['verb']} | |
| 115 | + end | |
| 116 | + | |
| 117 | + should 'the content be returned in scrap activities' do | |
| 118 | + ActionTracker::Record.destroy_all | |
| 119 | + content = 'some content' | |
| 120 | + create(Scrap, :sender_id => person.id, :receiver_id => person.id, :content => content) | |
| 121 | + get "/api/v1/profiles/#{person.id}/activities?#{params.to_query}" | |
| 122 | + json = JSON.parse(last_response.body) | |
| 123 | + assert_equal content, json["activities"].last['content'] | |
| 124 | + end | |
| 125 | + | |
| 126 | + should 'not return the content in other kind of activities except scrap' do | |
| 127 | + ActionTracker::Record.destroy_all | |
| 128 | + create_activity(:target => person) | |
| 129 | + get "/api/v1/profiles/#{person.id}/activities?#{params.to_query}" | |
| 130 | + json = JSON.parse(last_response.body) | |
| 131 | + assert_nil json["activities"].last['content'] | |
| 132 | + end | |
| 133 | + | |
| 134 | + should 'list activities with pagination' do | |
| 135 | + ActionTracker::Record.destroy_all | |
| 136 | + a1 = create_activity(:target => person) | |
| 137 | + a2 = create_activity(:target => person) | |
| 138 | + | |
| 139 | + params[:page] = 1 | |
| 140 | + params[:per_page] = 1 | |
| 141 | + get "/api/v1/profiles/#{person.id}/activities?#{params.to_query}" | |
| 142 | + json_page_one = JSON.parse(last_response.body) | |
| 143 | + | |
| 144 | + params[:page] = 2 | |
| 145 | + params[:per_page] = 1 | |
| 146 | + get "/api/v1/profiles/#{person.id}/activities?#{params.to_query}" | |
| 147 | + json_page_two = JSON.parse(last_response.body) | |
| 148 | + | |
| 149 | + assert_includes json_page_one["activities"].map { |a| a["id"] }, a2.id | |
| 150 | + assert_not_includes json_page_one["activities"].map { |a| a["id"] }, a1.id | |
| 151 | + | |
| 152 | + assert_includes json_page_two["activities"].map { |a| a["id"] }, a1.id | |
| 153 | + assert_not_includes json_page_two["activities"].map { |a| a["id"] }, a2.id | |
| 154 | + end | |
| 155 | + | |
| 156 | + should 'list only 20 elements by page if no limit param is passed' do | |
| 157 | + ActionTracker::Record.destroy_all | |
| 158 | + 1.upto(25).map do | |
| 159 | + create_activity(:target => person) | |
| 160 | + end | |
| 161 | + get "/api/v1/profiles/#{person.id}/activities?#{params.to_query}" | |
| 162 | + json = JSON.parse(last_response.body) | |
| 163 | + assert_equal 20, json["activities"].length | |
| 164 | + end | |
| 165 | + | |
| 166 | + should 'list activities with timestamp' do | |
| 167 | + ActionTracker::Record.destroy_all | |
| 168 | + a1 = create_activity(:target => person) | |
| 169 | + a2 = create_activity(:target => person) | |
| 170 | + a2.updated_at = Time.zone.now | |
| 171 | + a2.save | |
| 172 | + | |
| 173 | + a1.updated_at = Time.zone.now + 3.hours | |
| 174 | + a1.save! | |
| 175 | + | |
| 176 | + | |
| 177 | + params[:timestamp] = Time.zone.now + 1.hours | |
| 178 | + get "/api/v1/profiles/#{person.id}/activities?#{params.to_query}" | |
| 179 | + json = JSON.parse(last_response.body) | |
| 180 | + | |
| 181 | + assert_includes json["activities"].map { |a| a["id"] }, a1.id | |
| 182 | + assert_not_includes json["activities"].map { |a| a["id"] }, a2.id | |
| 183 | + end | |
| 184 | + | |
| 185 | + | |
| 186 | + | |
| 187 | + def create_activity(params = {}) | |
| 188 | + params[:verb] ||= 'create_article' | |
| 189 | + ActionTracker::Record.create!(params.merge(:user => person)) | |
| 83 | 190 | end |
| 84 | 191 | |
| 85 | 192 | end | ... | ... |
test/test_helper.rb
| ... | ... | @@ -87,8 +87,8 @@ class ActiveSupport::TestCase |
| 87 | 87 | norm1 = enum1.group_by{|e|e}.values |
| 88 | 88 | norm2 = enum2.group_by{|e|e}.values |
| 89 | 89 | assert_equal norm1.size, norm2.size, "Size mismatch: #{enum1.inspect} vs #{enum2.inspect}" |
| 90 | - assert_equal [], norm1 - norm2 | |
| 91 | - assert_equal [], norm2 - norm1 | |
| 90 | + assert_equal [], norm1 - norm2, "Arrays #{norm1} and #{norm2} are not equivalents" | |
| 91 | + assert_equal [], norm2 - norm1, "Arrays #{norm1} and #{norm2} are not equivalents" | |
| 92 | 92 | end |
| 93 | 93 | |
| 94 | 94 | def assert_mandatory(object, attribute, test_value = 'some random string') | ... | ... |