From 3b23aa79134171ab6334c8dda747feb353016cb7 Mon Sep 17 00:00:00 2001 From: Pranav Raj S Date: Mon, 3 Aug 2020 13:40:20 +0530 Subject: [PATCH] Bugfix: Use server timestamp to set agent_last_seen_at (#1114) --- .../v1/accounts/conversations_controller.rb | 7 +----- .../dashboard/api/inbox/conversation.js | 6 ++--- .../widgets/conversation/MessagesView.vue | 7 +----- .../conversation/labels/LabelBox.vue | 3 +++ .../store/modules/conversations/actions.js | 15 ++++++++---- .../store/modules/conversations/index.js | 4 +++- .../specs/conversations/actions.spec.js | 24 +++++++++++++++++++ .../specs/conversations/mutations.spec.js | 7 ++++++ .../update_last_seen.json.jbuilder | 1 + ...20200802170002_reset_agent_last_seen_at.rb | 7 ++++++ db/schema.rb | 2 +- .../accounts/conversations_controller_spec.rb | 5 +--- 12 files changed, 62 insertions(+), 26 deletions(-) create mode 100644 app/views/api/v1/accounts/conversations/update_last_seen.json.jbuilder create mode 100644 db/migrate/20200802170002_reset_agent_last_seen_at.rb diff --git a/app/controllers/api/v1/accounts/conversations_controller.rb b/app/controllers/api/v1/accounts/conversations_controller.rb index 8d8099fdc..74f206e00 100644 --- a/app/controllers/api/v1/accounts/conversations_controller.rb +++ b/app/controllers/api/v1/accounts/conversations_controller.rb @@ -44,9 +44,8 @@ class Api::V1::Accounts::ConversationsController < Api::V1::Accounts::BaseContro end def update_last_seen - @conversation.agent_last_seen_at = parsed_last_seen_at + @conversation.agent_last_seen_at = DateTime.now.utc @conversation.save! - head :ok end private @@ -56,10 +55,6 @@ class Api::V1::Accounts::ConversationsController < Api::V1::Accounts::BaseContro Rails.configuration.dispatcher.dispatch(event, Time.zone.now, conversation: @conversation, user: user) end - def parsed_last_seen_at - DateTime.strptime(params[:agent_last_seen_at].to_s, '%s') - end - def conversation @conversation ||= Current.account.conversations.find_by(display_id: params[:id]) end diff --git a/app/javascript/dashboard/api/inbox/conversation.js b/app/javascript/dashboard/api/inbox/conversation.js index 3f1b2e199..5a9173a7a 100644 --- a/app/javascript/dashboard/api/inbox/conversation.js +++ b/app/javascript/dashboard/api/inbox/conversation.js @@ -29,10 +29,8 @@ class ConversationApi extends ApiClient { ); } - markMessageRead({ id, lastSeen }) { - return axios.post(`${this.url}/${id}/update_last_seen`, { - agent_last_seen_at: lastSeen, - }); + markMessageRead({ id }) { + return axios.post(`${this.url}/${id}/update_last_seen`); } toggleTyping({ conversationId, status }) { diff --git a/app/javascript/dashboard/components/widgets/conversation/MessagesView.vue b/app/javascript/dashboard/components/widgets/conversation/MessagesView.vue index 1951588dd..551d82b83 100644 --- a/app/javascript/dashboard/components/widgets/conversation/MessagesView.vue +++ b/app/javascript/dashboard/components/widgets/conversation/MessagesView.vue @@ -218,12 +218,7 @@ export default { }, makeMessagesRead() { - if (this.getUnreadCount !== 0 && this.getMessages !== undefined) { - this.$store.dispatch('markMessagesRead', { - id: this.currentChat.id, - lastSeen: this.getMessages.messages.last().created_at, - }); - } + this.$store.dispatch('markMessagesRead', { id: this.currentChat.id }); }, }, }; diff --git a/app/javascript/dashboard/routes/dashboard/conversation/labels/LabelBox.vue b/app/javascript/dashboard/routes/dashboard/conversation/labels/LabelBox.vue index 1a2353899..f637f6c26 100644 --- a/app/javascript/dashboard/routes/dashboard/conversation/labels/LabelBox.vue +++ b/app/javascript/dashboard/routes/dashboard/conversation/labels/LabelBox.vue @@ -106,6 +106,9 @@ export default { this.isEditing = false; }, async fetchLabels(conversationId) { + if (!conversationId) { + return; + } this.$store.dispatch('conversationLabels/get', conversationId); }, }, diff --git a/app/javascript/dashboard/store/modules/conversations/actions.js b/app/javascript/dashboard/store/modules/conversations/actions.js index b237a822d..e4d33f805 100644 --- a/app/javascript/dashboard/store/modules/conversations/actions.js +++ b/app/javascript/dashboard/store/modules/conversations/actions.js @@ -170,11 +170,18 @@ const actions = { }, markMessagesRead: async ({ commit }, data) => { - setTimeout(() => { - commit(types.default.MARK_MESSAGE_READ, data); - }, 4000); try { - await ConversationApi.markMessageRead(data); + const { + data: { id, agent_last_seen_at: lastSeen }, + } = await ConversationApi.markMessageRead(data); + setTimeout( + () => + commit(types.default.MARK_MESSAGE_READ, { + id, + lastSeen, + }), + 4000 + ); } catch (error) { // Handle error } diff --git a/app/javascript/dashboard/store/modules/conversations/index.js b/app/javascript/dashboard/store/modules/conversations/index.js index 81380b547..107c94b15 100644 --- a/app/javascript/dashboard/store/modules/conversations/index.js +++ b/app/javascript/dashboard/store/modules/conversations/index.js @@ -134,7 +134,9 @@ export const mutations = { [types.default.MARK_MESSAGE_READ](_state, { id, lastSeen }) { const [chat] = _state.allConversations.filter(c => c.id === id); - chat.agent_last_seen_at = lastSeen; + if (chat) { + chat.agent_last_seen_at = lastSeen; + } }, [types.default.CHANGE_CHAT_STATUS_FILTER](_state, data) { diff --git a/app/javascript/dashboard/store/modules/specs/conversations/actions.spec.js b/app/javascript/dashboard/store/modules/specs/conversations/actions.spec.js index d9b4ad753..2c470207d 100644 --- a/app/javascript/dashboard/store/modules/specs/conversations/actions.spec.js +++ b/app/javascript/dashboard/store/modules/specs/conversations/actions.spec.js @@ -153,4 +153,28 @@ describe('#actions', () => { expect(commit.mock.calls).toEqual([[types.default.ADD_MESSAGE, message]]); }); }); + + describe('#markMessagesRead', () => { + beforeEach(() => { + jest.useFakeTimers(); + }); + + it('sends correct mutations if api is successful', async () => { + const lastSeen = new Date().getTime() / 1000; + axios.post.mockResolvedValue({ + data: { id: 1, agent_last_seen_at: lastSeen }, + }); + await actions.markMessagesRead({ commit }, { id: 1 }); + jest.runAllTimers(); + expect(commit).toHaveBeenCalledTimes(1); + expect(commit.mock.calls).toEqual([ + [types.default.MARK_MESSAGE_READ, { id: 1, lastSeen }], + ]); + }); + it('sends correct mutations if api is unsuccessful', async () => { + axios.post.mockRejectedValue({ message: 'Incorrect header' }); + await actions.markMessagesRead({ commit }, { id: 1 }); + expect(commit.mock.calls).toEqual([]); + }); + }); }); diff --git a/app/javascript/dashboard/store/modules/specs/conversations/mutations.spec.js b/app/javascript/dashboard/store/modules/specs/conversations/mutations.spec.js index d4d364f8f..f8d6283d8 100644 --- a/app/javascript/dashboard/store/modules/specs/conversations/mutations.spec.js +++ b/app/javascript/dashboard/store/modules/specs/conversations/mutations.spec.js @@ -20,6 +20,13 @@ describe('#mutations', () => { { id: 1, agent_last_seen_at: lastSeen }, ]); }); + + it('doesnot send any mutation if chat doesnot exist', () => { + const state = { allConversations: [] }; + const lastSeen = new Date().getTime() / 1000; + mutations[types.MARK_MESSAGE_READ](state, { id: 1, lastSeen }); + expect(state.allConversations).toEqual([]); + }); }); describe('#CLEAR_CURRENT_CHAT_WINDOW', () => { diff --git a/app/views/api/v1/accounts/conversations/update_last_seen.json.jbuilder b/app/views/api/v1/accounts/conversations/update_last_seen.json.jbuilder new file mode 100644 index 000000000..2d39b121c --- /dev/null +++ b/app/views/api/v1/accounts/conversations/update_last_seen.json.jbuilder @@ -0,0 +1 @@ +json.partial! 'api/v1/conversations/partials/conversation.json.jbuilder', conversation: @conversation diff --git a/db/migrate/20200802170002_reset_agent_last_seen_at.rb b/db/migrate/20200802170002_reset_agent_last_seen_at.rb new file mode 100644 index 000000000..b6d4198a7 --- /dev/null +++ b/db/migrate/20200802170002_reset_agent_last_seen_at.rb @@ -0,0 +1,7 @@ +class ResetAgentLastSeenAt < ActiveRecord::Migration[6.0] + def change + # rubocop:disable Rails/SkipsModelValidations + ::Conversation.where('agent_last_seen_at > ?', DateTime.now.utc).update_all(agent_last_seen_at: DateTime.now.utc) + # rubocop:enable Rails/SkipsModelValidations + end +end diff --git a/db/schema.rb b/db/schema.rb index 0addc1f8e..2e32788d6 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2020_07_19_171437) do +ActiveRecord::Schema.define(version: 2020_08_02_170002) do # These are extensions that must be enabled in order to support this database enable_extension "pg_stat_statements" diff --git a/spec/controllers/api/v1/accounts/conversations_controller_spec.rb b/spec/controllers/api/v1/accounts/conversations_controller_spec.rb index af9eeaa4f..17ad27118 100644 --- a/spec/controllers/api/v1/accounts/conversations_controller_spec.rb +++ b/spec/controllers/api/v1/accounts/conversations_controller_spec.rb @@ -177,15 +177,12 @@ RSpec.describe 'Conversations API', type: :request do let(:agent) { create(:user, account: account, role: :agent) } it 'updates last seen' do - params = { agent_last_seen_at: '-1' } - post "/api/v1/accounts/#{account.id}/conversations/#{conversation.display_id}/update_last_seen", headers: agent.create_new_auth_token, - params: params, as: :json expect(response).to have_http_status(:success) - expect(conversation.reload.agent_last_seen_at).to eq(DateTime.strptime(params[:agent_last_seen_at].to_s, '%s')) + expect(conversation.reload.agent_last_seen_at).not_to eq nil end end end