Skip to content

Commit

Permalink
Merge pull request #12632 from nextcloud/feat/noid/delete-message-rea…
Browse files Browse the repository at this point in the history 
…ctive

fix(chat): promptly update messages list in sidebar, store and chat
  • Loading branch information
@Antreesy
Antreesy authored Jul 3, 2024
2 parents a3c275c + 4b5daa2 commit 372d728
Show file tree
Hide file tree
Showing 3 changed files with 62 additions and 124 deletions.
40 changes: 11 additions & 29 deletions src/components/MessagesList/MessagesList.vue
View file
Original file line number Diff line number Diff line change
Expand Up @@ -406,49 +406,31 @@ export default {
},

softUpdateAuthorGroups(oldGroups, newGroups, dateTimestamp) {
const newGroupIdSet = new Set(this.messagesList.map(message => message.id))
const newGroupsKeys = Object.keys(newGroups)
const groupIds = new Set([...Object.keys(oldGroups), ...Object.keys(newGroups)])

Object.entries(oldGroups).forEach(([id, oldGroup]) => {
if (newGroupsKeys.includes(id)) {
// Group is presented, will be handled in the next loop
return
}

if (oldGroup.messages.some(message => !newGroupIdSet.has(message.id))) {
// Delete groups of normal and temporary messages,
// if at least one message from the group is no longer in the store
groupIds.forEach(id => {
if (oldGroups[id] && !newGroups[id]) {
// group no longer exists, remove
this.$delete(this.messagesGroupedByDateByAuthor[dateTimestamp], id)
}
})
Object.entries(newGroups).forEach(([id, newGroup]) => {
if (!oldGroups[id]) {
const oldId = Object.keys(oldGroups)
.find(key => +id < +key && oldGroups[key].nextMessageId <= newGroup.nextMessageId)
if (oldId) {
// newGroup includes oldGroup and more old messages, remove oldGroup
this.$delete(this.messagesGroupedByDateByAuthor[dateTimestamp], oldId)
}
// newGroup is not presented in the list, add it
this.$set(this.messagesGroupedByDateByAuthor[dateTimestamp], id, newGroup)
} else if (!this.areGroupsIdentical(newGroup, oldGroups[id])) {
// newGroup includes oldGroup and more recent messages
this.$set(this.messagesGroupedByDateByAuthor[dateTimestamp], id, newGroup)
} else if ((newGroups[id] && !oldGroups[id]) || !this.areGroupsIdentical(newGroups[id], oldGroups[id])) {
// group did not exist before, or group differs from previous state, add
this.$set(this.messagesGroupedByDateByAuthor[dateTimestamp], id, newGroups[id])
}
})
},

areGroupsIdentical(group1, group2) {
// Compare plain values
if (group1.messages.length !== group2.messages.length
|| JSON.stringify(group1.messages) !== JSON.stringify(group2.messages)
|| group1.dateSeparator !== group2.dateSeparator
|| group1.previousMessageId !== group2.previousMessageId
|| group1.nextMessageId !== group2.nextMessageId) {
return false
}

// Check for temporary messages, replaced with messages from server
return group1.messages.every((message, index) => group2.messages[index].id === message.id)
// Compare ids and stringified messages (look for temporary, edited, deleted messages, replaced from server)
return group1.messages.every((message, index) => group2.messages[index].id === message.id
&& JSON.stringify(group2.messages[index]) === JSON.stringify(message))
},

removeExpiredMessagesFromStore() {
Expand Down
21 changes: 5 additions & 16 deletions src/store/messagesStore.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -290,13 +290,7 @@ const mutations = {
if (!state.messages[token]) {
Vue.set(state.messages, token, {})
}
if (state.messages[token][message.id]) {
Vue.set(state.messages[token], message.id,
Object.assign({}, state.messages[token][message.id], message)
)
} else {
Vue.set(state.messages[token], message.id, message)
}
Vue.set(state.messages[token], message.id, message)
},
/**
* Deletes a message from the store.
Expand Down Expand Up @@ -541,15 +535,6 @@ const actions = {
context.commit('addMessage', { token, message: message.parent })
}

// update conversation lastMessage, if it was edited
if (message.systemMessage === 'message_edited'
&& message.parent.id === context.getters.conversation(token).lastMessage.id) {
context.dispatch('updateConversationLastMessage', {
token,
lastMessage: message.parent,
})
}

const reactionsStore = useReactionsStore()
if (message.systemMessage === 'message_deleted') {
reactionsStore.resetReactions(token, message.parent.id)
Expand All @@ -558,6 +543,10 @@ const actions = {
}

if (message.systemMessage === 'message_edited' || message.systemMessage === 'message_deleted') {
// update conversation lastMessage, if it was edited or deleted
if (message.parent.id === context.getters.conversation(token).lastMessage.id) {
context.dispatch('updateConversationLastMessage', { token, lastMessage: message.parent })
}
// Check existing messages for having a deleted / edited message as parent, and update them
context.getters.messagesList(token)
.filter(storedMessage => storedMessage.parent?.id === message.parent.id && JSON.stringify(storedMessage.parent) !== JSON.stringify(message.parent))
Expand Down
125 changes: 46 additions & 79 deletions src/store/messagesStore.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,9 +68,23 @@ jest.mock('@nextcloud/capabilities', () => ({

describe('messagesStore', () => {
const TOKEN = 'XXTOKENXX'
const conversation = {
token: TOKEN,
lastMessage: {
id: 123,
},
}

let localVue = null
let testStoreConfig
let store = null
let getActorIdMock
let getUserIdMock
let getActorTypeMock
let getDisplayNameMock
let conversationMock
let updateConversationLastMessageMock
let updateConversationLastReadMessageMock
let updateConversationLastActiveAction
let reactionsStore

Expand All @@ -80,10 +94,26 @@ describe('messagesStore', () => {
setActivePinia(createPinia())
reactionsStore = useReactionsStore()

testStoreConfig = cloneDeep(messagesStore)
testStoreConfig = cloneDeep(storeConfig)

getActorIdMock = jest.fn().mockReturnValue(() => 'actor-id-1')
getUserIdMock = jest.fn().mockReturnValue(() => 'actor-id-1')
getActorTypeMock = jest.fn().mockReturnValue(() => ATTENDEE.ACTOR_TYPE.USERS)
getDisplayNameMock = jest.fn().mockReturnValue(() => 'actor-display-name-1')
conversationMock = jest.fn().mockReturnValue(conversation)
updateConversationLastMessageMock = jest.fn()
updateConversationLastReadMessageMock = jest.fn()
updateConversationLastActiveAction = jest.fn()
testStoreConfig.actions.updateConversationLastActive = updateConversationLastActiveAction

testStoreConfig.modules.actorStore.getters.getActorId = getActorIdMock
testStoreConfig.modules.actorStore.getters.getUserId = getUserIdMock
testStoreConfig.modules.actorStore.getters.getActorType = getActorTypeMock
testStoreConfig.modules.actorStore.getters.getDisplayName = getDisplayNameMock
testStoreConfig.modules.conversationsStore.getters.conversation = jest.fn().mockReturnValue(conversationMock)
testStoreConfig.modules.conversationsStore.actions.updateConversationLastMessage = updateConversationLastMessageMock
testStoreConfig.modules.conversationsStore.actions.updateConversationLastReadMessage = updateConversationLastReadMessageMock
testStoreConfig.modules.conversationsStore.actions.updateConversationLastActive = updateConversationLastActiveAction
testStoreConfig.modules.pollStore.getters.debounceGetPollData = jest.fn()

store = new Vuex.Store(testStoreConfig)
})
Expand All @@ -104,11 +134,8 @@ describe('messagesStore', () => {
})

test('doesn\'t add specific messages to the store', () => {
testStoreConfig = cloneDeep(storeConfig)
testStoreConfig.modules.pollStore.getters.debounceGetPollData = jest.fn()
reactionsStore.resetReactions = jest.fn()
reactionsStore.processReaction = jest.fn()
store = new Vuex.Store(testStoreConfig)

const messages = [{
id: 2,
Expand Down Expand Up @@ -229,9 +256,7 @@ describe('messagesStore', () => {
let message

beforeEach(() => {
testStoreConfig = cloneDeep(storeConfig)
reactionsStore.resetReactions = jest.fn()
store = new Vuex.Store(testStoreConfig)

message = {
id: 10,
Expand Down Expand Up @@ -361,24 +386,8 @@ describe('messagesStore', () => {

describe('edit message', () => {
let message
let conversationMock
let updateConversationLastMessageMock

beforeEach(() => {
const conversation = {
token: TOKEN,
lastMessage: {
id: 10,
},
}

testStoreConfig = cloneDeep(messagesStore)
conversationMock = jest.fn().mockReturnValue(conversation)
updateConversationLastMessageMock = jest.fn()
testStoreConfig.getters.conversation = jest.fn().mockReturnValue(conversationMock)
testStoreConfig.actions.updateConversationLastMessage = updateConversationLastMessageMock
store = new Vuex.Store(testStoreConfig)

message = {
id: 10,
token: TOKEN,
Expand Down Expand Up @@ -470,28 +479,13 @@ describe('messagesStore', () => {

describe('temporary messages', () => {
let mockDate
let getActorIdMock
let getActorTypeMock
let getDisplayNameMock
let chatExtraStore

beforeEach(() => {
mockDate = new Date('2020-01-01 20:00:00')
jest.spyOn(global, 'Date')
.mockImplementation(() => mockDate)

testStoreConfig = cloneDeep(messagesStore)
chatExtraStore = useChatExtrasStore()

getActorIdMock = jest.fn().mockReturnValue(() => 'actor-id-1')
getActorTypeMock = jest.fn().mockReturnValue(() => ATTENDEE.ACTOR_TYPE.USERS)
getDisplayNameMock = jest.fn().mockReturnValue(() => 'actor-display-name-1')
testStoreConfig.getters.getActorId = getActorIdMock
testStoreConfig.getters.getActorType = getActorTypeMock
testStoreConfig.getters.getDisplayName = getDisplayNameMock
testStoreConfig.actions.updateConversationLastActive = updateConversationLastActiveAction

store = new Vuex.Store(testStoreConfig)
})

test('creates temporary message', async () => {
Expand Down Expand Up @@ -640,8 +634,10 @@ describe('messagesStore', () => {
expect(updateConversationLastActiveAction).toHaveBeenCalledWith(expect.anything(), TOKEN)

// add again just replaces it
temporaryMessage.message = 'replaced'
store.dispatch('addTemporaryMessage', { token: TOKEN, message: temporaryMessage })
store.dispatch('addTemporaryMessage', {
token: TOKEN,
message: { ...temporaryMessage, message: 'replaced' }
})

expect(store.getters.messagesList(TOKEN)).toMatchObject([{
id: 'temp-1577908800000',
Expand Down Expand Up @@ -757,31 +753,9 @@ describe('messagesStore', () => {
})

describe('last read message markers', () => {
let conversationMock
let getUserIdMock
let updateConversationLastReadMessageMock

beforeEach(() => {
const conversation = {
lastMessage: {
id: 123,
},
}

testStoreConfig = cloneDeep(messagesStore)

getUserIdMock = jest.fn()
conversationMock = jest.fn().mockReturnValue(conversation)
updateConversationLastReadMessageMock = jest.fn()
testStoreConfig.getters.conversation = jest.fn().mockReturnValue(conversationMock)
testStoreConfig.getters.getUserId = jest.fn().mockReturnValue(getUserIdMock)
testStoreConfig.actions.updateConversationLastReadMessage = updateConversationLastReadMessageMock
testStoreConfig.actions.addConversation = jest.fn()

const response = generateOCSResponse({ payload: conversation })
updateLastReadMessage.mockResolvedValue(response)

store = new Vuex.Store(testStoreConfig)
})

test('stores visual last read message id per token', () => {
Expand All @@ -793,7 +767,7 @@ describe('messagesStore', () => {
})

test('clears last read message', async () => {
getUserIdMock.mockReturnValue('user-1')
getUserIdMock.mockReturnValue(() => 'user-1')

store.dispatch('setVisualLastReadMessageId', { token: TOKEN, id: 100 })
await store.dispatch('clearLastReadMessage', {
Expand All @@ -813,7 +787,7 @@ describe('messagesStore', () => {
})

test('clears last read message for federated conversation', async () => {
getUserIdMock.mockReturnValue('federated-user-1')
getUserIdMock.mockReturnValue(() => 'federated-user-1')
conversationMock.mockReturnValue({
lastMessage: {},
remoteServer: 'nextcloud.com',
Expand All @@ -835,7 +809,7 @@ describe('messagesStore', () => {
})

test('clears last read message and update visually', async () => {
getUserIdMock.mockReturnValue('user-1')
getUserIdMock.mockReturnValue(() => 'user-1')

store.dispatch('setVisualLastReadMessageId', { token: TOKEN, id: 100 })
await store.dispatch('clearLastReadMessage', {
Expand All @@ -855,7 +829,7 @@ describe('messagesStore', () => {
})

test('clears last read message for guests', async () => {
getUserIdMock.mockReturnValue(null)
getUserIdMock.mockReturnValue(() => null)

store.dispatch('setVisualLastReadMessageId', { token: TOKEN, id: 100 })
await store.dispatch('clearLastReadMessage', {
Expand All @@ -875,7 +849,7 @@ describe('messagesStore', () => {
})

test('updates last read message', async () => {
getUserIdMock.mockReturnValue('user-1')
getUserIdMock.mockReturnValue(() => 'user-1')
const response = generateOCSResponse({
payload: {
unreadMessages: 0,
Expand Down Expand Up @@ -903,7 +877,7 @@ describe('messagesStore', () => {
})

test('updates last read message and update visually', async () => {
getUserIdMock.mockReturnValue('user-1')
getUserIdMock.mockReturnValue(() => 'user-1')
const response = generateOCSResponse({
payload: {
unreadMessages: 0,
Expand Down Expand Up @@ -931,7 +905,7 @@ describe('messagesStore', () => {
})

test('updates last read message for guests', async () => {
getUserIdMock.mockReturnValue(null)
getUserIdMock.mockReturnValue(() => null)

store.dispatch('setVisualLastReadMessageId', { token: TOKEN, id: 100 })
await store.dispatch('updateLastReadMessage', {
Expand Down Expand Up @@ -1790,7 +1764,7 @@ describe('messagesStore', () => {
lastMessage: { id: 100 },
lastReadMessage: 50,
})
getUserIdMock.mockReturnValue('current-user')
getUserIdMock.mockReturnValue(() => 'current-user')

const temporaryMessage = {
id: 'temp-123',
Expand Down Expand Up @@ -1985,11 +1959,10 @@ describe('messagesStore', () => {
*/
function setupWithValues(lastKnownMessageId, lastConversationMessageId) {
store.dispatch('setLastKnownMessageId', { token: TOKEN, id: 123 })
const conversationMock = jest.fn().mockReturnValue({
conversationMock.mockReturnValue({
token: TOKEN,
lastMessage: { id: lastConversationMessageId },
})
testStoreConfig.getters.conversation = jest.fn().mockReturnValue(conversationMock)
store = new Vuex.Store(testStoreConfig)
store.dispatch('setLastKnownMessageId', { token: TOKEN, id: lastKnownMessageId })
}

Expand All @@ -2015,12 +1988,6 @@ describe('messagesStore', () => {
let messageExpected

beforeEach(() => {
localVue = createLocalVue()
localVue.use(Vuex)

testStoreConfig = cloneDeep(storeConfig)
store = new Vuex.Store(testStoreConfig)

message1 = {
id: 1,
token: TOKEN,
Expand Down

0 comments on commit 372d728

Please sign in to comment.