Delete messages from store for deleted conversation

[PVince81]

Fixes #4572

When a conversation is deleted through an action, also delete the matching messages for consistency.

This also prevents the corresponding vues to try to access the deleted conversation.

Note: the proposed code works by defining an action in messagesStore with the same name as the one in the conversationsStore. The dispatching of actions will trigger on all the stores, which then makes this magic happen :-)

I noticed also that "purgeConversations" is also not clearing the messagesStore but decided against adding it here, as it might have side effects. I'll raise a tech debt ticket later to discuss this unrelated bit.

[PVince81]

For reference, this is where the app sidebar deletes the conversation from store upon leaving: https://github.com/nextcloud/spreed/blob/bugfix/4572/clear-messages-when-conversation-delete/src/FilesSidebarTabApp.vue#L219

The issue was that the CallView + MessagesList were still rendered, so they were trying to render old messages that were not cleared which referred to the deleted conversation: https://github.com/nextcloud/spreed/blob/bugfix/4572/clear-messages-when-conversation-delete/src/components/MessagesList/MessagesGroup/Message/Message.vue#L282.

The fix prevents those message vues to be rendered in the first place and brings back the loading placeholder in that case.

[nickvergessen]

I noticed also that "purgeConversations" is also not clearing the messagesStore but decided against adding it here, as it might have side effects. I'll raise a tech debt ticket later to discuss this unrelated bit.

purgeConversation is done every 30 seconds, so don't do it there

[nickvergessen]

Note: the proposed code works by defining an action in messagesStore with the same name as the one in the conversationsStore. The dispatching of actions will trigger on all the stores, which then makes this magic happen :-)

That sounds rather hacky and like it could break as it could have been unintended. Wouldn't it be better to call a new action from this action instead?

[PVince81]

Note: the proposed code works by defining an action in messagesStore with the same name as the one in the conversationsStore. The dispatching of actions will trigger on all the stores, which then makes this magic happen :-)

That sounds rather hacky and like it could break as it could have been unintended. Wouldn't it be better to call a new action from this action instead?

It isn't hacky if it's part of the framework. It might be the reason why dispatch uses a string a action name instead of calling the function directly to allow for this.

However, I failed to find any article about this feature, nor best practices, so I really wonder why it's there and why nobody is using it...

So for the sake of readability I'll make the conversation store action call the message list store action to make it more explicit.

[PVince81]

Code adjusted and retested

[nickvergessen]

I still get TypeError: this.$refs.messageMain is undefined and don't see anything in the chat with the steps posted by daniel?

[PVince81]

damn, seems I had a cached state when I retested.

I'm now getting something different than what you see

vuex.esm.js:497 [vuex] unknown action type: deleteMessages

[PVince81]

regarding multiple action handlers with same name, so far I found a mention in https://hackernoon.com/correct-and-efficient-way-to-use-vuex-part-i-wf15432eu

The first thing to know is that store.dispatch can handle Promise returned by the triggered action handler and it also returns Promise. It's possible for a store.dispatch to trigger multiple action handlers in different modules. In such a case the returned value will be a Promise that resolves when all triggered handlers have been resolved.

and here by the Vue author himself, he calls these "Composable Action Flow" (see section 3): vuejs/vuex#236

[PVince81]

I've squashed the fix into the last commit.

It turns out I only renamed deleteConversation to deleteMessages in the mutation but forgot to do the same in the action.

[PVince81]

@nickvergessen can you retest ? Not sure about the error you saw, didn't see it last time

[nickvergessen]

I start to dislike the delete* method name as with real message deletion that is easy to mix up. should make clear its only removing from the store and not from DB, but same with conversation and others. so its okay for now

[PVince81]

I start to dislike the delete* method name as with real message deletion that is easy to mix up. should make clear its only removing from the store and not from DB, but same with conversation and others. so its okay for now

Yep. Went through the same dislike while working on this.