Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optimize conversations update on Frontend #9832

Merged
merged 1 commit into from
Jun 22, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 30 additions & 2 deletions src/store/conversationsStore.js
Original file line number Diff line number Diff line change
Expand Up @@ -203,14 +203,42 @@ const mutations = {

const actions = {
/**
* Add a conversation to the store and index the displayName.
* Add a conversation to the store and index the displayName
*
* @param {object} context default store context;
* @param {object} conversation the conversation;
*/
addConversation(context, conversation) {
context.commit('addConversation', conversation)

context.dispatch('postAddConversation', conversation)
},

/**
* Add conversation to the store only, if it was changed according to lastActivity and modifiedSince
*
* @param {object} context dispatch context
* @param {object} payload mutation payload
* @param {object} payload.conversation the conversation
* @param {number|0} payload.modifiedSince timestamp of last state or 0 if unknown
*/
addConversationIfChanged(context, { conversation, modifiedSince }) {
if (conversation.lastActivity >= modifiedSince) {
context.commit('addConversation', conversation)
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
}
} else {
// Smart update
}

Don't we want to apply smartUpdate here to cover rest cases (full re-fetch at least)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we use the smartUpdate approach in else, then we do not need if branch, as smartUpdate covers all possible updates anyway


context.dispatch('postAddConversation', conversation)
},

/**
* Post-actions after adding a conversation:
* - Get user status from 1-1 conversations
* - Add current user to the new conversation's participants
*
* @param {object} context dispatch context
* @param {object} conversation the conversation
*/
postAddConversation(context, conversation) {
if (conversation.type === CONVERSATION.TYPE.ONE_TO_ONE && conversation.status) {
emit('user_status:status.updated', {
status: conversation.status,
Expand Down Expand Up @@ -649,7 +677,7 @@ const actions = {
dispatch('purgeConversationsStore')
}
response.data.ocs.data.forEach(conversation => {
dispatch('addConversation', conversation)
dispatch('addConversationIfChanged', { conversation, modifiedSince })
})
return response
} catch (error) {
Expand Down
54 changes: 54 additions & 0 deletions src/store/conversationsStore.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ describe('conversationsStore', () => {
participantFlags: PARTICIPANT.CALL_FLAG.DISCONNECTED,
participantType: PARTICIPANT.TYPE.USER,
lastPing: 600,
lastActivity: 1672531200, // 2023-01-01T00:00:00.000Z
sessionId: 'session-id-1',
attendeeId: 'attendee-id-1',
actorType: ATTENDEE.ACTOR_TYPE.USERS,
Expand Down Expand Up @@ -242,10 +243,12 @@ describe('conversationsStore', () => {
{
token: 'one_token',
attendeeId: 'attendee-id-1',
lastActivity: 1675209600, // 2023-02-01T00:00:00.000Z
},
{
token: 'another_token',
attendeeId: 'attendee-id-2',
lastActivity: 1675209600, // 2023-02-01T00:00:00.000Z
},
]

Expand All @@ -271,6 +274,57 @@ describe('conversationsStore', () => {
expect(updateTalkVersionHashAction).toHaveBeenCalledWith(expect.anything(), response)
})

test('fetches all conversations and update only new without purging when modifiedSince is present', async () => {
const oldConversation1 = {
token: 'tokenOne',
attendeeId: 'attendee-id-1',
lastActivity: 1672531200, // 2023-01-01T00:00:00.000Z
}
const oldConversation2 = {
token: 'tokenTwo',
attendeeId: 'attendee-id-2',
lastActivity: 1672531200, // 2023-01-01T00:00:00.000Z
}

// Add initial conversations
store.dispatch('addConversation', oldConversation1)
store.dispatch('addConversation', oldConversation2)

// Fetch new conversation
// The same lastActivity, as oldConversation
const newConversation1 = {
token: 'tokenOne',
attendeeId: 'attendee-id-1',
lastActivity: 1672531200, // 2023-01-01T00:00:00.000Z
}
// Has new activity
const newConversation2 = {
token: 'tokenTwo',
attendeeId: 'attendee-id-2',
lastActivity: 1675209600, // 2023-02-01T00:00:00.000Z
}
const modifiedSince = 1675209600 // 2023-02-01T00:00:00.000Z

const response = {
data: {
ocs: {
data: [newConversation1, newConversation2],
},
},
}

fetchConversations.mockResolvedValue(response)

await store.dispatch('fetchConversations', { modifiedSince })

expect(fetchConversations).toHaveBeenCalledWith({ params: { modifiedSince } })
// conversationsList is actual to the response
expect(store.getters.conversationsList).toEqual([newConversation1, newConversation2])
// Only old conversation with new activity should be actually replaced with new objects
expect(store.state.conversationsStore.conversations[oldConversation1.token]).toStrictEqual(oldConversation1)
expect(store.state.conversationsStore.conversations[oldConversation2.token]).toStrictEqual(newConversation2)
})

test('fetch conversation failure checks for maintenance mode', async () => {
const response = { status: 503 }
fetchConversation.mockRejectedValue({ response })
Expand Down