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.

Sign up for GitHub

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

Jump to bottom

Deleting a user should instantly remove messages from that user #9374

Closed
robbyoconnor opened this issue Jan 10, 2018 · 6 comments · Fixed by #11236
Closed

Deleting a user should instantly remove messages from that user #9374

robbyoconnor opened this issue Jan 10, 2018 · 6 comments · Fixed by #11236
Labels
feat: message type: improvement

Comments

@robbyoconnor
Copy link
Contributor

Description:

If I delete a user, the messages are removed, but this isn't reflected until clients reload the channel(s).

Server Setup Information:

  • Version of Rocket.Chat Server: 0.60.3
  • Operating System: Ubuntu 16.04
  • Deployment Method(snap/docker/tar/etc): docker
  • Number of Running Instances: 1
  • DB Replicaset Oplog: n/a
  • Node Version: 8

Steps to Reproduce:

  1. Create extra user
  2. Type some messages in a channel
  3. Delete extra user

Expected behavior:

I would expect all messages to be instantly removed but they are not.

Actual behavior:

Messages remain until the client is reloaded.
@geekgonecrazy
Copy link
Member

This one could be tough.. when a user deletes a single message it makes sense to send an event. But let's just imagine what would happen if you sent events to every client connected when deleting... Say my user.

For one the server would likely chew through some CPU removing my user as is. But then imagine it had to fire events to inform all connected users of each message being deleted.

So we can't use the same delete as we do now. We'd have to pretty much create a new one just for this type of bulk update type of thing.

@robbyoconnor
Copy link
Contributor Author

I'm not sure how slack handles this type of thing, but it's surely possible..

@geekgonecrazy
Copy link
Member

For sure possible. Might be able to just have client side react and remove messages when it's alerted to the user being deleted.

@robbyoconnor
Copy link
Contributor Author

Maybe for the current active channel only? Loading the others shouldn't be affected...assuming you don't keep a local cache of messages... if you do...then this is easier, just invalidate the cache.

@Chris-Duke
Copy link

Would it be preferable to have this as an option, as there could be a valid reason to want to go back to view messages from a certain user?

Imagine having conversations between 3 or more people in a chat room, then if a user and their messages get deleted, the conversation wouldn't make sense.

The reason I'd be in favour of having this as an option, would be for auditing reasons.

@vynmera vynmera mentioned this issue Jun 22, 2018
Merged
42 tasks
@vynmera
Copy link
Contributor

vynmera commented Jun 25, 2018

This will be fixed in #11236.

ggazzo pushed a commit that referenced this issue Jul 20, 2018
@vynmera @ggazzo
[NEW][BREAK] Message retention policy and pruning (#11236)
829c5d1 
Closes #6749
Closes #8321
Closes #9374
Closes #2700
Closes #2639
Closes #2355 
Closes #1861
Closes #8757
Closes #7228
Closes #10870
Closes #6193 
Closes #11299
Closes #11468
Closes #9317
Closes #11300 (will incorporate a fix to this PR's issue)
Closes #11046 (will incorporate a fix to this PR's issue)
Contributes to #5944 
Contributes to #11475
_...and possibly more!_

This PR makes deleting messages (automatically and manually) a lot easier on Rocket.Chat.

- [X] Implement a bulk message deletion notification, to quickly push large message deletions to users without reload
  - [X] Use it in `rooms.cleanHistory`
  - [X] Use it in user deletions
- [X] Completely remove cleanChannelHistory as required by v0.67
  - [X] Remove server method `cleanChannelHistory`
  - [X] Remove REST API `channels.cleanHistory`
- [x] Implement a sidebar option to clean history
  - [x] Basic implementation
  - [x] Allow excluding pinned messages
  - [x] Allow attachment-only mode
  - [x] Allow specifying user(s) to narrow down to
    - [x] Also update REST API
    - [x] Also update docs
  - [x] Break the deletion into multiple different requests, so the client can keep track of progress
  - [x] Clear animation / progress bar for deleting
- [x] Retention policy
  - [X] Global, set by admin
    - [X] Global timer that runs every second and deletes messages over the set limit
      - [X] Can change its timer's resolution to prevent insane CPU overhead
    - [X] Admin can decide what room types to target (channels, groups and/or DMs)
    - [X] Allow excluding pinned messages
    - [X] Allow attachment-only mode
  - [x] Per-channel, set by those with a new permission
    - [x] Disabled when master switch off
    - [x] Set in channel info
    - [x] Can override global policy with a switch that requires `edit-privileged-setting`
    - [x] Allow excluding pinned messages
    - [x] Allow attachment-only mode
    - [x] Uses same global timer for cleanup
  - [X] Message at start of channel history / in channel info if there is a retention policy set
  - [x] Message in channel info if there is a retention policy set on that channel specifically
- [X] Make cleaning history also delete files (completely!)
  - [X] Manual purging
  - [X] Automatic purging
- [x] Make other deletions also delete files
  - [x] User deletion
    - [X] Own messages
    - [x] DMs with them's partner messages
  - [x] Room deletion
- [x] Cleanup
- [x] Finish related [docs](https://github.com/RocketChat/docs/pull/815)
- [x] Link to the docs in the settings

Please suggest any cool changes/additions! Any support is greatly appreciated.

**Breaking change:** This PR removes REST API endpoint `channels.cleanHistory` and Meteor callable `cleanChannelHistory` as per the protocol specified for them.

![bzzzzzzzz](https://user-images.githubusercontent.com/39674991/41799087-56d1dea0-7670-11e8-94c0-bc534b1f832d.png)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
feat: message type: improvement
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[NEW] Message retention policy and pruning vynmera/Rocket.Chat
4 participants
@robbyoconnor @geekgonecrazy @Chris-Duke @vynmera