From d84ae80074b660737a4681040a6149c16075f822 Mon Sep 17 00:00:00 2001 From: Sam Date: Mon, 27 May 2013 10:22:37 +1000 Subject: [PATCH] Simplify user action make it more idiomatic --- app/controllers/user_actions_controller.rb | 15 ++++--- app/models/user_action.rb | 42 ++++++------------- app/serializers/user_action_serializer.rb | 27 ++++++++++++ .../user_actions_controller_spec.rb | 22 ++++++++++ 4 files changed, 70 insertions(+), 36 deletions(-) create mode 100644 app/serializers/user_action_serializer.rb create mode 100644 spec/controllers/user_actions_controller_spec.rb diff --git a/app/controllers/user_actions_controller.rb b/app/controllers/user_actions_controller.rb index b30dcb37b..92a7ec398 100644 --- a/app/controllers/user_actions_controller.rb +++ b/app/controllers/user_actions_controller.rb @@ -14,12 +14,15 @@ class UserActionsController < ApplicationController ignore_private_messages: params[:filter] ? false : true } - if opts[:action_types] == [UserAction::GOT_PRIVATE_MESSAGE] || - opts[:action_types] == [UserAction::NEW_PRIVATE_MESSAGE] - render json: UserAction.private_message_stream(opts[:action_types][0], opts) - else - render json: UserAction.stream(opts) - end + stream = + if opts[:action_types] == [UserAction::GOT_PRIVATE_MESSAGE] || + opts[:action_types] == [UserAction::NEW_PRIVATE_MESSAGE] + UserAction.private_message_stream(opts[:action_types][0], opts) + else + UserAction.stream(opts) + end + + render_serialized(stream, UserActionSerializer, root: "user_actions") end def show diff --git a/app/models/user_action.rb b/app/models/user_action.rb index 7c8cc07bb..6a240c865 100644 --- a/app/models/user_action.rb +++ b/app/models/user_action.rb @@ -78,7 +78,7 @@ SQL # The weird thing is that target_post_id can be null, so it makes everything # ever so more complex. Should we allow this, not sure. - builder = SqlBuilder.new(" + builder = UserAction.sql_builder(" SELECT t.title, a.action_type, a.created_at, t.id topic_id, a.user_id AS target_user_id, au.name AS target_name, au.username AS target_username, @@ -105,16 +105,16 @@ LEFT JOIN categories c on c.id = t.category_id if action_id builder.where("a.id = :id", id: action_id.to_i) - data = builder.exec.to_a else builder.where("a.user_id = :user_id", user_id: user_id.to_i) builder.where("a.action_type in (:action_types)", action_types: action_types) if action_types && action_types.length > 0 - builder.order_by("a.created_at desc") - builder.offset(offset.to_i) - builder.limit(limit.to_i) - data = builder.exec.to_a + builder + .order_by("a.created_at desc") + .offset(offset.to_i) + .limit(limit.to_i) end - normalize_builder_data(data) + + builder.exec.to_a end # slightly different to standard stream, it collapses replies @@ -123,7 +123,7 @@ LEFT JOIN categories c on c.id = t.category_id user_id = opts[:user_id] return [] unless opts[:guardian].can_see_private_messages?(user_id) - builder = SqlBuilder.new(" + builder = UserAction.sql_builder(" SELECT t.title, :action_type action_type, p.created_at, t.id topic_id, :user_id AS target_user_id, au.name AS target_name, au.username AS target_username, @@ -145,12 +145,10 @@ ORDER BY p.created_at desc /*limit*/ ") - builder.offset((opts[:offset] || 0).to_i) - builder.limit((opts[:limit] || 60).to_i) - - data = builder.exec(user_id: user_id, action_type: action_type).to_a - - normalize_builder_data(data) + builder + .offset((opts[:offset] || 0).to_i) + .limit((opts[:limit] || 60).to_i) + .exec(user_id: user_id, action_type: action_type).to_a end def self.log_action!(hash) @@ -209,25 +207,9 @@ ORDER BY p.created_at desc end protected - - def self.normalize_builder_data(data) - data.each do |row| - row["action_type"] = row["action_type"].to_i - row["created_at"] = DateTime.parse(row["created_at"]) - # we should probably cache the excerpts in the db at some point - row["excerpt"] = PrettyText.excerpt(row["cooked"],300) if row["cooked"] - row["cooked"] = nil - row["avatar_template"] = User.avatar_template(row["email"]) - row["acting_avatar_template"] = User.avatar_template(row["acting_email"]) - row.delete("email") - row.delete("acting_email") - row["slug"] = Slug.for(row["title"]) - end - end def self.apply_common_filters(builder,user_id,guardian,ignore_private_messages=false) - unless guardian.can_see_deleted_posts? builder.where("p.deleted_at is null and p2.deleted_at is null and t.deleted_at is null") end diff --git a/app/serializers/user_action_serializer.rb b/app/serializers/user_action_serializer.rb new file mode 100644 index 000000000..617ee6679 --- /dev/null +++ b/app/serializers/user_action_serializer.rb @@ -0,0 +1,27 @@ +class UserActionSerializer < ApplicationSerializer + + attributes :action_type, :created_at, :excerpt, + :avatar_template, :acting_avatar_template, + :slug, :topic_id, :target_user_id, :target_name, + :target_username, :post_number, :reply_to_post_number, + :username, :name, :user_id, :acting_username, + :acting_name, :acting_user_id + + + def excerpt + PrettyText.excerpt(object.cooked,300) if object.cooked + end + + def avatar_template + User.avatar_template(object.email) + end + + def acting_avatar_template + User.avatar_template(object.acting_email) + end + + def slug + Slug.for(object.title) + end + +end diff --git a/spec/controllers/user_actions_controller_spec.rb b/spec/controllers/user_actions_controller_spec.rb new file mode 100644 index 000000000..763cdbf3e --- /dev/null +++ b/spec/controllers/user_actions_controller_spec.rb @@ -0,0 +1,22 @@ +require 'spec_helper' + +describe UserActionsController do + context 'index' do + + it 'renders list correctly' do + ActiveRecord::Base.observers.enable :all + post = Fabricate(:post) + + xhr :get, :index, username: post.user.username + + response.status.should == 200 + parsed = JSON.parse(response.body) + actions = parsed["user_actions"] + actions.length.should == 1 + action = actions[0] + action["acting_name"].should == post.user.name + action["email"].should be_nil + action["post_number"].should == "1" + end + end +end