From 84799fd0a1763bfe36ffecf379fc23c95c8b0107 Mon Sep 17 00:00:00 2001
From: Subin T P <subinthattaparambil@gmail.com>
Date: Sun, 1 Dec 2019 10:16:51 +0530
Subject: [PATCH] [#247] Filters conversation by status and paginate
 conversations (#284)

* [#247] Filters conversation by status

* Fixes conversation finder specs

* [#248] Paginates conversation

* Use method name in description

* Move page to default param, add filters on frontend

* Fix code climate issues
---
 .../api/v1/conversations_controller.rb        |  1 -
 app/finders/conversation_finder.rb            | 41 ++++++++++---
 .../dashboard/api/inbox/conversation.js       |  6 +-
 .../dashboard/components/ChatList.vue         | 61 ++++++++-----------
 .../components/widgets/ChatTypeTabs.vue       |  6 +-
 .../widgets/conversation/ChatFilter.vue       | 14 +++--
 .../dashboard/i18n/locale/en/chatlist.json    |  2 +-
 .../store/modules/conversations/getters.js    |  1 +
 .../store/modules/conversations/index.js      | 27 ++++----
 .../api/v1/conversations/index.json.jbuilder  |  6 +-
 spec/factories/conversations.rb               |  2 +-
 spec/factories/inbox_members.rb               |  8 +++
 spec/finders/conversation_finder_spec.rb      | 49 +++++++++++++++
 13 files changed, 155 insertions(+), 69 deletions(-)
 create mode 100644 spec/factories/inbox_members.rb
 create mode 100644 spec/finders/conversation_finder_spec.rb

diff --git a/app/controllers/api/v1/conversations_controller.rb b/app/controllers/api/v1/conversations_controller.rb
index 1c03e8ee0..cc179a75d 100644
--- a/app/controllers/api/v1/conversations_controller.rb
+++ b/app/controllers/api/v1/conversations_controller.rb
@@ -11,7 +11,6 @@ class Api::V1::ConversationsController < Api::BaseController
     result = conversation_finder.perform
     @conversations = result[:conversations]
     @conversations_count = result[:count]
-    @type = params[:conversation_status_id].to_i
   end
 
   def show
diff --git a/app/finders/conversation_finder.rb b/app/finders/conversation_finder.rb
index 026b9658f..cc41bb557 100644
--- a/app/finders/conversation_finder.rb
+++ b/app/finders/conversation_finder.rb
@@ -6,6 +6,8 @@ class ConversationFinder
   ASSIGNEE_TYPES_BY_ID = ASSIGNEE_TYPES.invert
   ASSIGNEE_TYPES_BY_ID.default = :me
 
+  DEFAULT_STATUS = 'open'.freeze
+
   # assumptions
   # inbox_id if not given, take from all conversations, else specific to inbox
   # assignee_type if not given, take 'me'
@@ -15,7 +17,7 @@ class ConversationFinder
   # {conversations: [array of conversations], count: {open: count, resolved: count}}
 
   # params
-  # assignee_type_id, inbox_id, :conversation_status_id,
+  # assignee_type_id, inbox_id, :status
 
   def initialize(current_user, params)
     @current_user = current_user
@@ -27,12 +29,21 @@ class ConversationFinder
     set_inboxes
     set_assignee_type
 
-    find_all_conversations # find all with the inbox
-    filter_by_assignee_type # filter by assignee
-    open_count, resolved_count = set_count_for_all_conversations # fetch count for both before filtering by status
+    find_all_conversations
+    filter_by_status
 
-    { conversations: @conversations.latest,
-      count: { open: open_count, resolved: resolved_count } }
+    mine_count, unassigned_count, all_count = set_count_for_all_conversations
+
+    filter_by_assignee_type
+
+    {
+      conversations: conversations,
+      count: {
+        mine_count: mine_count,
+        unassigned_count: unassigned_count,
+        all_count: all_count
+      }
+    }
   end
 
   private
@@ -69,7 +80,23 @@ class ConversationFinder
     @conversations
   end
 
+  def filter_by_status
+    @conversations = @conversations.where(status: params[:status] || DEFAULT_STATUS)
+  end
+
   def set_count_for_all_conversations
-    [@conversations.open.count, @conversations.resolved.count]
+    [
+      @conversations.assigned_to(current_user).count,
+      @conversations.unassigned.count,
+      @conversations.count
+    ]
+  end
+
+  def current_page
+    params[:page]
+  end
+
+  def conversations
+    current_page ? @conversations.latest.page(current_page) : @conversations.latest
   end
 end
diff --git a/app/javascript/dashboard/api/inbox/conversation.js b/app/javascript/dashboard/api/inbox/conversation.js
index c43f41c70..b444d0e17 100644
--- a/app/javascript/dashboard/api/inbox/conversation.js
+++ b/app/javascript/dashboard/api/inbox/conversation.js
@@ -6,12 +6,12 @@ class ConversationApi extends ApiClient {
     super('conversations');
   }
 
-  get({ inboxId, convStatus, assigneeStatus }) {
+  get({ inboxId, status, assigneeType }) {
     return axios.get(this.url, {
       params: {
         inbox_id: inboxId,
-        conversation_status_id: convStatus,
-        assignee_type_id: assigneeStatus,
+        status,
+        assignee_type_id: assigneeType,
       },
     });
   }
diff --git a/app/javascript/dashboard/components/ChatList.vue b/app/javascript/dashboard/components/ChatList.vue
index f3b357110..89106b915 100644
--- a/app/javascript/dashboard/components/ChatList.vue
+++ b/app/javascript/dashboard/components/ChatList.vue
@@ -1,7 +1,5 @@
 <template>
   <div class="conversations-sidebar  medium-4 columns">
-    <!-- <SearchBox></SearchBox> -->
-
     <div class="chat-list__top">
       <h1 class="page-title">
         {{ getInboxName }}
@@ -14,10 +12,11 @@
       :active-tab-index="activeAssigneeTab"
       class="tab--chat-type"
       @chatTabChange="getDataForTab"
-    ></chat-type-tabs>
+    >
+    </chat-type-tabs>
 
     <p
-      v-if="!chatListLoading && !getChatsForTab(activeStatusTab).length"
+      v-if="!chatListLoading && !getChatsForTab(activeStatus).length"
       class="content-box"
     >
       {{ $t('CHAT_LIST.LIST.404') }}
@@ -33,7 +32,7 @@
       class="conversations-list"
     >
       <conversation-card
-        v-for="chat in getChatsForTab(activeStatusTab)"
+        v-for="chat in getChatsForTab(activeStatus)"
         :key="chat.id"
         :chat="chat"
       />
@@ -56,13 +55,12 @@ import wootConstants from '../constants';
 export default {
   mixins: [timeMixin, conversationMixin],
   props: ['conversationInbox', 'pageTitle'],
-  data: () => ({
-    chats: null,
-    activeStatusTab: 0,
-    activeAssigneeTab: 0,
-    toggleType: true,
-    allMessageType: 2,
-  }),
+  data() {
+    return {
+      activeAssigneeTab: 0,
+      activeStatus: 0,
+    };
+  },
   mounted() {
     this.$watch('$store.state.route', () => {
       if (this.$store.state.route.name !== 'inbox_conversation') {
@@ -85,17 +83,8 @@ export default {
       chatListLoading: 'getChatListLoadingStatus',
       currentUserID: 'getCurrentUserID',
       activeInbox: 'getSelectedInbox',
+      convStats: 'getConvTabStats',
     }),
-    convStats() {
-      const mineCount = this.mineChatsList.length;
-      const unAssignedCount = this.unAssignedChatsList.length;
-      const allCount = this.allChatList.length;
-      return {
-        mineCount,
-        unAssignedCount,
-        allCount,
-      };
-    },
     assigneeTabItems() {
       return this.$t('CHAT_LIST.ASSIGNEE_TYPE_TABS').map((item, index) => ({
         id: index,
@@ -120,24 +109,27 @@ export default {
   methods: {
     fetchData() {
       if (this.chatLists.length === 0) {
-        this.$store.dispatch('fetchAllConversations', {
-          inboxId: this.conversationInbox ? this.conversationInbox : undefined,
-          assigneeStatus: this.allMessageType,
-          convStatus: this.activeStatusTab,
-        });
+        this.fetchConversations();
       }
     },
+    fetchConversations() {
+      this.$store.dispatch('fetchAllConversations', {
+        inboxId: this.conversationInbox ? this.conversationInbox : undefined,
+        assigneeType: this.activeAssigneeTab,
+        status: this.activeStatus ? 'resolved' : 'open',
+      });
+    },
     getDataForTab(index) {
-      this.activeAssigneeTab = index;
-      if (!(index in this.chatLists)) {
-        // this.$store.dispatch('fetchList', {
-        //   inbox: this.conversationInbox,
-        //   type: index,
-        // });
+      if (this.activeAssigneeTab !== index) {
+        this.activeAssigneeTab = index;
+        this.fetchConversations();
       }
     },
     getDataForStatusTab(index) {
-      this.activeStatusTab = index;
+      if (this.activeStatus !== index) {
+        this.activeStatus = index;
+        this.fetchConversations();
+      }
     },
     getChatsForTab() {
       let copyList = [];
@@ -154,7 +146,6 @@ export default {
         (a, b) =>
           this.lastMessage(b).created_at - this.lastMessage(a).created_at
       );
-
       return sorted;
     },
   },
diff --git a/app/javascript/dashboard/components/widgets/ChatTypeTabs.vue b/app/javascript/dashboard/components/widgets/ChatTypeTabs.vue
index a636072ad..3615543c6 100644
--- a/app/javascript/dashboard/components/widgets/ChatTypeTabs.vue
+++ b/app/javascript/dashboard/components/widgets/ChatTypeTabs.vue
@@ -32,8 +32,10 @@ export default {
   },
   methods: {
     onTabChange(selectedTabIndex) {
-      this.$emit('chatTabChange', selectedTabIndex);
-      this.tabsIndex = selectedTabIndex;
+      if (selectedTabIndex !== this.tabsIndex) {
+        this.$emit('chatTabChange', selectedTabIndex);
+        this.tabsIndex = selectedTabIndex;
+      }
     },
   },
 };
diff --git a/app/javascript/dashboard/components/widgets/conversation/ChatFilter.vue b/app/javascript/dashboard/components/widgets/conversation/ChatFilter.vue
index 589b38a02..6fc3975f3 100644
--- a/app/javascript/dashboard/components/widgets/conversation/ChatFilter.vue
+++ b/app/javascript/dashboard/components/widgets/conversation/ChatFilter.vue
@@ -1,7 +1,12 @@
-
 <template>
-  <select class="status--filter" v-model="activeIndex" @change="onTabChange()">
-    <option v-for="(item, index) in $t('CHAT_LIST.CHAT_STAUTUS_ITEMS')" :value="item['VALUE']">{{item["TEXT"]}}</option>
+  <select v-model="activeIndex" class="status--filter" @change="onTabChange()">
+    <option
+      v-for="item in $t('CHAT_LIST.CHAT_STATUS_ITEMS')"
+      :key="item['VALUE']"
+      :value="item['VALUE']"
+    >
+      {{ item['TEXT'] }}
+    </option>
   </select>
 </template>
 
@@ -10,8 +15,7 @@ export default {
   data: () => ({
     activeIndex: 0,
   }),
-  mounted() {
-  },
+  mounted() {},
   methods: {
     onTabChange() {
       this.$store.dispatch('setChatFilter', this.activeIndex);
diff --git a/app/javascript/dashboard/i18n/locale/en/chatlist.json b/app/javascript/dashboard/i18n/locale/en/chatlist.json
index c4f54ff70..c0bc10057 100644
--- a/app/javascript/dashboard/i18n/locale/en/chatlist.json
+++ b/app/javascript/dashboard/i18n/locale/en/chatlist.json
@@ -25,7 +25,7 @@
       "ALL": 2
     },
 
-    "CHAT_STAUTUS_ITEMS": [
+    "CHAT_STATUS_ITEMS": [
       { "TEXT": "Open", "VALUE": 0 },
       { "TEXT": "Resolved", "VALUE": 1 }
     ],
diff --git a/app/javascript/dashboard/store/modules/conversations/getters.js b/app/javascript/dashboard/store/modules/conversations/getters.js
index d2cdc4fc3..383d5c4ef 100644
--- a/app/javascript/dashboard/store/modules/conversations/getters.js
+++ b/app/javascript/dashboard/store/modules/conversations/getters.js
@@ -47,6 +47,7 @@ const getters = {
   },
   getChatStatusFilter: ({ chatStatusFilter }) => chatStatusFilter,
   getSelectedInbox: ({ currentInbox }) => currentInbox,
+  getConvTabStats: ({ convTabStats }) => convTabStats,
 };
 
 export default getters;
diff --git a/app/javascript/dashboard/store/modules/conversations/index.js b/app/javascript/dashboard/store/modules/conversations/index.js
index 9a2f30bea..5e9d17380 100644
--- a/app/javascript/dashboard/store/modules/conversations/index.js
+++ b/app/javascript/dashboard/store/modules/conversations/index.js
@@ -6,8 +6,6 @@ import wootConstants from '../../../constants';
 import getters, { getSelectedChatConversation } from './getters';
 import actions from './actions';
 
-// const chatType = 'all';
-// initial state
 const state = {
   allConversations: [],
   convTabStats: {
@@ -30,10 +28,18 @@ const state = {
 
 // mutations
 const mutations = {
-  [types.default.SET_ALL_CONVERSATION](_state, chatList) {
-    _state.allConversations.push(...chatList);
+  [types.default.SET_ALL_CONVERSATION](_state, conversationList) {
+    const newAllConversations = [..._state.allConversations];
+    conversationList.forEach(conversation => {
+      const indexInCurrentList = newAllConversations.findIndex(
+        c => c.id === conversation.id
+      );
+      if (indexInCurrentList < 0) {
+        newAllConversations.push(conversation);
+      }
+    });
+    _state.allConversations = newAllConversations;
   },
-
   [types.default.EMPTY_ALL_CONVERSATION](_state) {
     _state.allConversations = [];
     _state.selectedChat = {
@@ -45,7 +51,6 @@ const mutations = {
       dataFetched: false,
     };
   },
-
   [types.default.SET_ALL_MESSAGES_LOADED](_state) {
     const [chat] = getSelectedChatConversation(_state);
     Vue.set(chat, 'allMessagesLoaded', true);
@@ -71,14 +76,14 @@ const mutations = {
   [types.default.SET_CONV_TAB_META](
     _state,
     {
-      overdue_count: overdueCount,
+      mine_count: mineCount,
+      unassigned_count: unAssignedCount,
       all_count: allCount,
-      open_count: openCount,
     } = {}
   ) {
-    Vue.set(_state.convTabStats, 'overdueCount', overdueCount);
-    Vue.set(_state.convTabStats, 'allConvCount', allCount);
-    Vue.set(_state.convTabStats, 'openCount', openCount);
+    Vue.set(_state.convTabStats, 'mineCount', mineCount);
+    Vue.set(_state.convTabStats, 'allCount', allCount);
+    Vue.set(_state.convTabStats, 'unAssignedCount', unAssignedCount);
   },
 
   [types.default.CURRENT_CHAT_WINDOW](_state, activeChat) {
diff --git a/app/views/api/v1/conversations/index.json.jbuilder b/app/views/api/v1/conversations/index.json.jbuilder
index fefa3c61a..0f73c85ef 100644
--- a/app/views/api/v1/conversations/index.json.jbuilder
+++ b/app/views/api/v1/conversations/index.json.jbuilder
@@ -1,8 +1,8 @@
 json.data do
   json.meta do
-    json.open_count @conversations_count[:open]
-    json.resolved_count @conversations_count[:closed]
-    json.conversation_type @type
+    json.mine_count @conversations_count[:mine_count]
+    json.unassigned_count @conversations_count[:unassigned_count]
+    json.all_count @conversations_count[:all_count]
   end
 
   json.payload do
diff --git a/spec/factories/conversations.rb b/spec/factories/conversations.rb
index e6e3da4aa..d1dee0df9 100644
--- a/spec/factories/conversations.rb
+++ b/spec/factories/conversations.rb
@@ -3,7 +3,7 @@
 FactoryBot.define do
   factory :conversation do
     status { 'open' }
-    display_id { SecureRandom.uuid }
+    display_id { rand(10_000_000) }
     user_last_seen_at { Time.current }
     agent_last_seen_at { Time.current }
     locked { false }
diff --git a/spec/factories/inbox_members.rb b/spec/factories/inbox_members.rb
new file mode 100644
index 000000000..3ee68ebfc
--- /dev/null
+++ b/spec/factories/inbox_members.rb
@@ -0,0 +1,8 @@
+# frozen_string_literal: true
+
+FactoryBot.define do
+  factory :inbox_member do
+    user
+    inbox
+  end
+end
diff --git a/spec/finders/conversation_finder_spec.rb b/spec/finders/conversation_finder_spec.rb
new file mode 100644
index 000000000..e31d17267
--- /dev/null
+++ b/spec/finders/conversation_finder_spec.rb
@@ -0,0 +1,49 @@
+require 'rails_helper'
+
+describe ::ConversationFinder do
+  subject(:conversation_finder) { described_class.new(user_1, params) }
+
+  let!(:account) { create(:account) }
+  let!(:user_1) { create(:user, account: account) }
+  let!(:user_2) { create(:user, account: account) }
+  let!(:inbox) { create(:inbox, account: account) }
+
+  before do
+    create(:inbox_member, user: user_1, inbox: inbox)
+    create(:inbox_member, user: user_2, inbox: inbox)
+    create(:complete_conversation, account: account, inbox: inbox, assignee: user_1)
+    create(:complete_conversation, account: account, inbox: inbox, assignee: user_1)
+    create(:complete_conversation, account: account, inbox: inbox, assignee: user_1, status: 'resolved')
+    create(:complete_conversation, account: account, inbox: inbox, assignee: user_2)
+  end
+
+  describe '#perform' do
+    context 'with status' do
+      let(:params) { { status: 'open', assignee_type_id: 0 } }
+
+      it 'filter conversations by status' do
+        result = conversation_finder.perform
+        expect(result[:conversations].count).to be 2
+      end
+    end
+
+    context 'with assignee' do
+      let(:params) { { assignee_type_id: 2 } }
+
+      it 'filter conversations by assignee' do
+        result = conversation_finder.perform
+        expect(result[:conversations].count).to be 3
+      end
+    end
+
+    context 'with pagination' do
+      let(:params) { { status: 'open', assignee_type_id: 0, page: 1 } }
+
+      it 'returns paginated conversations' do
+        create_list(:complete_conversation, 50, account: account, inbox: inbox, assignee: user_1)
+        result = conversation_finder.perform
+        expect(result[:conversations].count).to be 25
+      end
+    end
+  end
+end