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

Updated the deleted_at timestamp for deactivated direct message channel #15039

Conversation

Vars-07
Copy link

@Vars-07 Vars-07 commented Jul 15, 2020

Summary

When the user account is deactivated, direct message channels are not updated with deleted time

Ticket Link

https://mattermost.atlassian.net/browse/MM-26941

@hanzei
Copy link
Contributor

hanzei commented Jul 20, 2020

/update-branch

@hanzei
Copy link
Contributor

hanzei commented Jul 20, 2020

Hi @Vars-07,

Thanks for the PR 👍 Could you please run make store-layers and commit the changes to fix the CI error?

@hanzei hanzei added the Work In Progress Not yet ready for review label Jul 20, 2020
@Vars-07
Copy link
Author

Vars-07 commented Jul 20, 2020

@hanzei Updated. Have a look :)

@hanzei hanzei added 2: Dev Review Requires review by a developer and removed Work In Progress Not yet ready for review labels Jul 22, 2020
@hanzei hanzei added 3: QA Review Requires review by a QA tester. May occur at the same time as Dev Review EETests labels Jul 22, 2020
@mattermod mattermod removed the EETests label Jul 22, 2020
@lindalumitchell lindalumitchell added the Setup Cloud Test Server Setup an on-prem test server label Jul 24, 2020
@agnivade agnivade requested review from ashishbhate and removed request for mgdelacroix July 24, 2020 12:34
Copy link
Member

@agnivade agnivade left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working on this @Vars-07 ! I have some comments to improve this further.

app/user.go Outdated Show resolved Hide resolved
app/user.go Outdated Show resolved Hide resolved
app/user.go Outdated Show resolved Hide resolved
store/sqlstore/channel_store.go Outdated Show resolved Hide resolved
store/sqlstore/channel_store.go Outdated Show resolved Hide resolved
app/user.go Outdated Show resolved Hide resolved
store/sqlstore/channel_store.go Show resolved Hide resolved
@Vars-07 Vars-07 requested a review from agnivade July 26, 2020 17:53
app/user.go Outdated Show resolved Hide resolved
Copy link
Contributor

@ashishbhate ashishbhate left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good @Vars-07 ! Some small suggestions inline.

@agnivade
Copy link
Member

agnivade commented Aug 3, 2020

@jgilliam17 - Assigning to you since you filed the original issue and have a general context. The overall behavior is tested with unit tests.

@jgilliam17 jgilliam17 added the Setup Cloud Test Server Setup an on-prem test server label Aug 4, 2020
@mm-cloud-bot mm-cloud-bot removed the Setup Cloud Test Server Setup an on-prem test server label Aug 4, 2020
@jgilliam17 jgilliam17 added the Setup Cloud Test Server Setup an on-prem test server label Aug 4, 2020
@jgilliam17
Copy link

Thank you @Vars-07

I am seeing a deactivated DM removed from the LHS as another channel/DM is opened, without having to close deactivated DM. Deactivated DMs should remain on the LHS even when not open on the main channel view.
Let me know if this change is related to this fix. If not, I can file separately.
If you look at the images, on the old sidebar deactivated DM is present when open, on exp. sidebar DM is not shown on the LHS. When switching away to a different channel, on both old and exp sidebar, deactivated DM should still be present on the LHS.

Old sidebar Exp. Sidebar
Screen Shot 2020-08-05 at 10 39 15 AM Screen Shot 2020-08-05 at 10 39 36 AM

@Vars-07
Copy link
Author

Vars-07 commented Aug 6, 2020

Hi, @jgilliam17, yes that change of vanishing away archived/deactivated channel is not related to this fix. IMO that should be UI issue. you can file a separate Jira for that. Thanks

@jgilliam17
Copy link

/update-branch

@hmhealey
Copy link
Member

@Vars-07 Can you explain again what problem you were having with DMs not being deleted when one of their members is deactivated? I'm not sure if marking the DM as deleted is the correct way to fix that since deleting DMs might have unintended side effects such as the one @jgilliam17 reported. In the past, we've assumed that DMs and GMs are only hidden, never archived, so I'd be worried about changing that now

@mattermod
Copy link
Contributor

This PR has been automatically labelled "stale" because it hasn't had recent activity.
A core team member will check in on the status of the PR to help with questions.
Thank you for your contribution!

/cc @jasonblais @jfrerich

@jasonblais
Copy link
Contributor

Hi @Vars-07, let us know if you had thoughts or questions on Harrison's latest message. Would be happy to clarify :)

From @hmhealey:

@Vars-07 Can you explain again what problem you were having with DMs not being deleted when one of their members is deactivated? I'm not sure if marking the DM as deleted is the correct way to fix that since deleting DMs might have unintended side effects such as the one @jgilliam17 reported. In the past, we've assumed that DMs and GMs are only hidden, never archived, so I'd be worried about changing that now

@stylianosrigas stylianosrigas removed the Setup Cloud Test Server Setup an on-prem test server label Aug 26, 2020
@stylianosrigas
Copy link
Contributor

Removing the test server for maintenance purposes. Please add the label again if you need it. Sorry for any inconvenience.

@mm-cloud-bot
Copy link

Test server destroyed

@hanzei
Copy link
Contributor

hanzei commented Sep 22, 2020

@Vars-07 Do you have any questions about the feedback?
Please note that we will close this PR after another 10 days of inactivity. If you don't object, other contributors might complete this PR for you.

@Vars-07
Copy link
Author

Vars-07 commented Sep 22, 2020 via email

@jgilliam17
Copy link

Removing my name but, leaving the QA review label. Please add me again when PR is ready for re-review. Thanks.

@jgilliam17 jgilliam17 removed their request for review August 25, 2021 21:41
@jasonblais
Copy link
Contributor

Big thanks for your contribution!

However, it has been some time with no activity so we've marked your PR as inactive. Per our inactive contribution process, it is eligible to be assumed by another community member interested in working on the ticket.

If there are any areas we can help clarify or provide guidance, let us know! We'd be happy to help :)

@jasonblais jasonblais closed this Aug 26, 2021
@jasonblais jasonblais added Lifecycle/3:orphaned and removed Lifecycle/2:inactive 3: QA Review Requires review by a QA tester. May occur at the same time as Dev Review labels Aug 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

When the user account is deactivated, direct message channels are not updated with deleted time