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

Deleting user doesn't delete messages #2700

Closed
dandv opened this issue Mar 30, 2016 · 11 comments · Fixed by #11236
Closed

Deleting user doesn't delete messages #2700

dandv opened this issue Mar 30, 2016 · 11 comments · Fixed by #11236
Assignees

Comments

@dandv
Copy link

dandv commented Mar 30, 2016

0.21.0, commit hash ...dac592

  1. Log in as user X and leave some messages in #general
  2. In a different tab, log in as an admin and delete user X. Notice the warning dialog saying that the user's messages will be deleted.

A third user who was watching #general reports that user X's messages are still there. Hard reloading the page no longer shows the deleted user's messages.
When I wanted to join #general as the admin user, the preview mode still showed the messages.

Possibly related: #1149, #739

@engelgabriel
Copy link
Member

We dont send the "remove" message for each deleted message on this event, as it may be a lot of traffic. We could maybe send the delete user event to all logged users, so they can delete the messaged on the client.

@engelgabriel engelgabriel added this to the Important milestone Mar 30, 2016
@engelgabriel
Copy link
Member

It would be something similar to the UPDATE AVATAR :)

@andreas-bulling
Copy link

I am not sure I understand the logic behind this - why delete all messages of a deleted user at all/by default? Couldn't it be that some messages the user posted are still of value to others even if the user himself is not active any more? I would at least leave the decision to also delete all messages to the admin so he can do this on a case-by-case basis.

@dytec
Copy link

dytec commented Mar 30, 2016

@andreas-bulling +1

@engelgabriel
Copy link
Member

In that case you should not DELETE the user, but just DEACTIVATE instead. If you REALLY want to delete the user record, we have to delete related references to keep DB integrity.

@andreas-bulling
Copy link

@engelgabriel that makes sense. What does DEACTIVATE involve - the user is not able to login anymore etc?

@engelgabriel
Copy link
Member

Yes, the user looses access completely.

@imtrobin
Copy link

When the user is deactivated, we can still find them on the member list, is there option to not show them?

@Hudell
Copy link
Contributor

Hudell commented Mar 12, 2018

PR #9947 adds the option to delete an user without removing their messages.

@engelgabriel
Copy link
Member

#6535

@vynmera
Copy link
Contributor

vynmera commented Jun 25, 2018

This will be implemented in #11236 :)

ggazzo pushed a commit that referenced this issue Jul 20, 2018
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)
@theorenck theorenck removed this from the Mid-term milestone Dec 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants