Merge pull request #3739 from loomio/moar-perf

Add full discussion / motion serializers for single resource requests [WIP]

angular/core/components/thread_page/proposal_expanded/proposal_expanded.coffee

@@ -4,6 +4,7 @@ angular.module('loomioApp').directive 'proposalExpanded', ->
   templateUrl: 'generated/components/thread_page/proposal_expanded/proposal_expanded.html'
   replace: true
   controller: ($scope, Records, Session, AbilityService, TranslationService) ->
+    Records.proposals.findOrFetchById($scope.proposal.key)
     Records.votes.fetchByProposal($scope.proposal)
 
     $scope.collapse = ->

app/controllers/api/discussions_controller.rb

@@ -2,6 +2,7 @@ class API::DiscussionsController < API::RestfulController
   load_and_authorize_resource only: [:show, :mark_as_read, :dismiss, :move]
   load_resource only: [:create, :update, :star, :unstar, :set_volume]
   include UsesDiscussionReaders
+  include UsesFullSerializer
 
   def index
     load_and_authorize(:group, optional: true)
@@ -12,13 +13,13 @@ def index
   def dashboard
     raise CanCan::AccessDenied.new unless current_user.is_logged_in?
     instantiate_collection { |collection| collection_for_dashboard collection }
-    respond_with_collection(serializer: Dashboard::DiscussionSerializer)
+    respond_with_collection
   end
 
   def inbox
     raise CanCan::AccessDenied.new unless current_user.is_logged_in?
     instantiate_collection { |collection| collection_for_inbox collection }
-    respond_with_collection(serializer: Dashboard::DiscussionSerializer)
+    respond_with_collection
   end
 
   def move

app/controllers/api/motions_controller.rb

@@ -1,10 +1,7 @@
 class API::MotionsController < API::RestfulController
+  load_and_authorize_resource only: :show
   include UsesDiscussionReaders
-
-  def show
-    load_and_authorize(:motion)
-    respond_with_resource
-  end
+  include UsesFullSerializer
 
   def close
     @event = service.close_by_user(load_and_authorize(:motion, :close), current_user)

app/helpers/uses_full_serializer.rb

@@ -0,0 +1,8 @@
+module UsesFullSerializer
+  def resource_serializer
+    case action_name
+    when 'show' then "Full::#{controller_name.singularize.camelize}Serializer".constantize
+    else super
+    end
+  end
+end

app/serializers/discussion_serializer.rb

@@ -35,8 +35,7 @@ def scope
              :updated_at,
              :archived_at,
              :private,
-             :versions_count,
-             :mentioned_usernames
+             :versions_count
 
   attributes_from_reader :discussion_reader_id,
                          :read_items_count,
@@ -50,9 +49,8 @@ def scope
 
   has_one :author, serializer: UserSerializer, root: :users
   has_one :group, serializer: GroupSerializer, root: :groups
-  has_one :active_proposal, serializer: Dashboard::MotionSerializer, root: :proposals
+  has_one :active_proposal, serializer: MotionSerializer, root: :proposals
   has_one :active_proposal_vote, serializer: VoteSerializer, root: :votes
-  has_many :attachments, serializer: AttachmentSerializer, root: :attachments
 
   def include_active_proposal_vote?
     reader.present? && active_proposal.present?

app/serializers/events/version_serializer.rb

@@ -1,3 +1,8 @@
 class Events::VersionSerializer < Events::BaseSerializer
   has_one :eventable, polymorphic: true, serializer: ::VersionSerializer
+  has_many :attachments, serializer: AttachmentSerializer, root: :attachments
+
+  def attachments
+    object.eventable.item.attachments
+  end
 end

app/serializers/full/discussion_serializer.rb

@@ -0,0 +1,5 @@
+class Full::DiscussionSerializer < ::DiscussionSerializer
+  attributes :mentioned_usernames
+  has_one :active_proposal, serializer: Full::MotionSerializer, root: :proposals
+  has_many :attachments, serializer: AttachmentSerializer, root: :attachments
+end

app/serializers/full/motion_serializer.rb

@@ -0,0 +1,4 @@
+class Full::MotionSerializer < ::MotionSerializer
+  attributes :mentioned_usernames
+  has_many :attachments, serializer: AttachmentSerializer, root: :attachments
+end

app/serializers/motion_serializer.rb

@@ -18,12 +18,10 @@ class MotionSerializer < ActiveModel::Serializer
              :vote_counts,
              :activity_count,
              :group_id,
-             :discussion_id,
-             :mentioned_usernames
+             :discussion_id
 
   has_one :author, serializer: UserSerializer, root: :users
   has_one :outcome_author, serializer: UserSerializer, root: :users
-  has_many :attachments, serializer: AttachmentSerializer, root: :attachments
 
   def filter(keys)
     keys.delete(:outcome_author) unless object.outcome_author.present?

spec/controllers/api/motions_controller_spec.rb

@@ -16,6 +16,7 @@
     discussion_id: discussion.id
   }}
   let(:my_vote) { create :vote, motion: motion, author: user }
+  let(:attachment) { create :attachment }
 
   before do
     group.admins << user
@@ -163,6 +164,25 @@
         expect(response).to be_success
         expect(motion.reload.name).to eq motion_params[:name]
       end
+
+      it 'adds attachments' do
+        motion_params[:attachment_ids] = attachment.id
+        post :update, id: motion.id, motion: motion_params, format: :json
+        expect(motion.reload.attachments).to include attachment
+        json = JSON.parse(response.body)
+        attachment_ids = json['attachments'].map { |a| a['id'] }
+        expect(attachment_ids).to include attachment.id
+      end
+
+      it 'removes attachments' do
+        attachment.update(attachable: discussion)
+        motion_params[:attachment_ids] = []
+        post :update, id: motion.id, motion: motion_params, format: :json
+        expect(motion.reload.attachments).to be_empty
+        json = JSON.parse(response.body)
+        attachment_ids = json['attachments'].map { |a| a['id'] }
+        expect(attachment_ids).to_not include attachment.id
+      end
     end
 
     context 'failures' do