axllent / mailpit

An email and SMTP testing tool with API for developers
https://mailpit.axllent.org
MIT License
6.17k stars 153 forks source link

Fix: Message view not updating when deleting messages from search #395

Closed avesst closed 1 day ago

avesst commented 1 day ago

When deleting messages from search with the API, the message view were never updated, while the message and unread count was.

axllent commented 1 day ago

Thank you, I had not noticed this before, but you're totally right!

I do not think this is the right place for this as it translates to a websocket message for every deleted message. This is OK if there is a relatively low number of messages being deleted, however someone deletes all 10,000 messages in a tag, it means every connected browser gets 10,00 individual delete messages.

I think that a better solution would be: a) To broadcast only after the dbLastAction = time.Now() (meaning the sql has already been executed) b) Add some logic to the broadcast that, if the len(ids) > 200, to broadcast a "prune" instead of individual messages to just tell every client to refresh their current view.

Example (directly after dbLastAction = time.Now()):

        // broadcast changes
        if len(ids) > 200 {
            websockets.Broadcast("prune", nil)
        } else {
            for _, id := range ids {
                d := struct {
                    ID string
                }{ID: id}
                websockets.Broadcast("delete", d)
            }
        }

What do you think?

avesst commented 1 day ago

Yeah you are correct. I'm not really a Go developer and had like 25 minutes between two meetings and quickly tried to get it done in that time, with this being my first time actually digging into the code base. The thought occurred to me that it would be a lot of calls but I tried it with 2000 e-mails and it seemed fine-ish, I however did not ponder the effects of multiple browsers being connected as in our use case it's rare that more than one person using Mailpit at once.

I made the change as you suggested.

axllent commented 1 day ago

Well done on finding the related code then, considering you're "not really a Go developer" :tada:

Thank you, I'll merge this in and it'll be included in the next release later this coming week (I try not release too often, so once a week is generally more than enough).