[MM-52541] Mark files as deleted along with thread (#23226)

* mark thread files as deleted

* add missing check

* improve query

* Stopped rendering post preview if post is deleted

* Fixed lint error

* Fixed test

* updated types

* Removed deleted post from other post's embed data

* Added tests

* Apply suggestions from code review

Co-authored-by: Daniel Espino García <larkox@gmail.com>

* lint fix

---------

Co-authored-by: Konstantinos Pittas <konstantinos.pittas@mattermost.com>
Co-authored-by: Harshil Sharma <harshilsharma63@gmail.com>
Co-authored-by: Mattermost Build <build@mattermost.com>
Co-authored-by: Harshil Sharma <18575143+harshilsharma63@users.noreply.github.com>
Co-authored-by: Daniel Espino García <larkox@gmail.com>
This commit is contained in:
Konstantinos Pittas
2023-06-02 13:30:33 +03:00
committed by GitHub
parent 341b34d08f
commit 4d41365fbe
10 changed files with 314 additions and 6 deletions

View File

@@ -2913,6 +2913,30 @@ func (s *SqlPostStore) deleteThread(transaction *sqlxTxWrapper, postId string, d
return errors.Wrapf(err, "failed to mark thread for root post %s as deleted", postId)
}
return s.deleteThreadFiles(transaction, postId, deleteAtTime)
}
func (s *SqlPostStore) deleteThreadFiles(transaction *sqlxTxWrapper, postID string, deleteAtTime int64) error {
var query sq.UpdateBuilder
if s.DriverName() == model.DatabaseDriverPostgres {
query = s.getQueryBuilder().Update("FileInfo").
Set("DeleteAt", deleteAtTime).
From("Posts")
} else {
query = s.getQueryBuilder().Update("FileInfo", "Posts").
Set("FileInfo.DeleteAt", deleteAtTime)
}
query = query.Where(sq.And{
sq.Expr("FileInfo.PostId = Posts.Id"),
sq.Eq{"Posts.RootId": postID},
})
_, err := transaction.ExecBuilder(query)
if err != nil {
return errors.Wrapf(err, "failed to mark files of thread post %s as deleted", postID)
}
return nil
}

View File

@@ -1303,6 +1303,88 @@ func testPostStoreDelete(t *testing.T, ss store.Store) {
// last reply at should be 0
require.Equal(t, int64(0), thread.LastReplyAt)
})
t.Run("thread with file attachments", func(t *testing.T) {
teamId := model.NewId()
channel, err := ss.Channel().Save(&model.Channel{
TeamId: teamId,
DisplayName: "DisplayName1",
Name: "channel" + model.NewId(),
Type: model.ChannelTypeOpen,
}, -1)
require.NoError(t, err)
// Create a root post
rootPost1, err := ss.Post().Save(&model.Post{
ChannelId: channel.Id,
UserId: model.NewId(),
Message: NewTestId(),
})
require.NoError(t, err)
// Create another root post
rootPost2, err := ss.Post().Save(&model.Post{
ChannelId: channel.Id,
UserId: model.NewId(),
Message: NewTestId(),
})
require.NoError(t, err)
// Reply to first root post with file attachments
replyPost1, err := ss.Post().Save(&model.Post{
ChannelId: rootPost1.ChannelId,
UserId: model.NewId(),
Message: NewTestId(),
RootId: rootPost1.Id,
})
require.NoError(t, err)
file11, err := ss.FileInfo().Save(&model.FileInfo{
Id: model.NewId(),
PostId: replyPost1.Id,
CreatorId: replyPost1.UserId,
Path: "file1.txt",
})
require.NoError(t, err)
file12, err := ss.FileInfo().Save(&model.FileInfo{
Id: model.NewId(),
PostId: replyPost1.Id,
CreatorId: replyPost1.UserId,
Path: "file2.png",
})
require.NoError(t, err)
// Reply to second root post with file attachments
replyPost2, err := ss.Post().Save(&model.Post{
ChannelId: rootPost2.ChannelId,
UserId: model.NewId(),
Message: NewTestId(),
RootId: rootPost2.Id,
})
require.NoError(t, err)
file21, err := ss.FileInfo().Save(&model.FileInfo{
Id: model.NewId(),
PostId: replyPost2.Id,
CreatorId: replyPost2.UserId,
Path: "file1.txt",
})
require.NoError(t, err)
// Delete the first root post
err = ss.Post().Delete(rootPost1.Id, model.GetMillis(), "")
require.NoError(t, err)
// Verify the reply post's files are deleted
_, err = ss.FileInfo().Get(file11.Id)
require.Error(t, err, "Deleted id should have failed")
require.IsType(t, &store.ErrNotFound{}, err)
_, err = ss.FileInfo().Get(file12.Id)
require.Error(t, err, "Deleted id should have failed")
require.IsType(t, &store.ErrNotFound{}, err)
// Verify the other reply post's files are NOT deleted
_, err = ss.FileInfo().Get(file21.Id)
require.NoError(t, err, "Not deleted id should have succeeded")
})
}
func testPostStorePermDelete1Level(t *testing.T, ss store.Store) {

View File

@@ -291,7 +291,6 @@ const ForwardPostModal = ({onExited, post, actions}: Props) => {
>
<PostMessagePreview
metadata={previewMetaData}
previewPost={previewMetaData.post}
handleFileDropdownOpened={noop}
preventClickAction={true}
previewFooterMessage={postPreviewFooterMessage}

View File

@@ -126,7 +126,6 @@ export default class PostBodyAdditionalContent extends React.PureComponent<Props
return (
<PostMessagePreview
metadata={embed.data}
previewPost={embed.data.post}
handleFileDropdownOpened={this.props.handleFileDropdownOpened}
/>
);

View File

@@ -668,3 +668,92 @@ exports[`PostMessagePreview should render correctly 1`] = `
`;
exports[`PostMessagePreview should render without preview 1`] = `""`;
exports[`PostMessagePreview show render without preview when preview posts becomes undefined after being defined 1`] = `
<PostAttachmentContainer
className="permalink"
link="/team1/pl/post_id"
>
<div
className="post-preview"
>
<div
className="post-preview__header"
>
<div
className="col col__name"
>
<div
className="post__img"
>
<span
className="profile-icon"
>
<Memo(Avatar)
className="avatar-post-preview"
size="sm"
url="/api/v4/users/user_1/image?_=0"
username="username1"
/>
</span>
</div>
</div>
<div
className="col col__name permalink--username"
>
<Connect(UserProfile)
disablePopover={true}
hasMention={true}
overwriteName=""
userId="user_1"
/>
</div>
<div
className="col d-flex align-items-center"
>
<Connect(injectIntl(Timestamp))
className="post-preview__time"
day="numeric"
units={
Array [
"now",
"minute",
"hour",
"day",
]
}
useTime={false}
/>
</div>
</div>
<Connect(PostMessageView)
maxHeight={105}
overflowType="ellipsis"
post={
Object {
"id": "post_id",
"message": "post message",
"metadata": Object {},
}
}
/>
<div
className="post__preview-footer"
>
<p>
<MemoizedFormattedMessage
defaultMessage="Only visible to users in ~{channel}"
id="post_message_preview.channel"
values={
Object {
"channel": "channel name",
}
}
/>
</p>
</div>
</div>
</PostAttachmentContainer>
`;
exports[`PostMessagePreview show render without preview when preview posts becomes undefined after being defined 2`] = `""`;

View File

@@ -8,7 +8,7 @@ import {bindActionCreators, Dispatch} from 'redux';
import {GlobalState} from 'types/store';
import {GenericAction} from 'mattermost-redux/types/actions';
import {Post, PostPreviewMetadata} from '@mattermost/types/posts';
import {PostPreviewMetadata} from '@mattermost/types/posts';
import {makeGetChannel} from 'mattermost-redux/selectors/entities/channels';
import {get} from 'mattermost-redux/selectors/entities/preferences';
@@ -29,7 +29,6 @@ import PostMessagePreview from './post_message_preview';
export type OwnProps = {
metadata: PostPreviewMetadata;
previewPost?: Post;
preventClickAction?: boolean;
previewFooterMessage?: string;
}
@@ -43,7 +42,7 @@ function makeMapStateToProps() {
let user = null;
let embedVisible = false;
let channelDisplayName = ownProps.metadata.channel_display_name;
const previewPost = getPost(state, ownProps.metadata.post_id) || ownProps.previewPost;
const previewPost = getPost(state, ownProps.metadata.post_id);
if (previewPost && previewPost.user_id) {
user = getUser(state, previewPost.user_id);

View File

@@ -65,6 +65,32 @@ describe('PostMessagePreview', () => {
expect(wrapper).toMatchSnapshot();
});
test('show render without preview when preview posts becomes undefined after being defined', () => {
const props = {...baseProps};
let wrapper = shallow(
<PostMessagePreview
{...props}
/>,
);
expect(wrapper).toMatchSnapshot();
let permalink = wrapper.find('.permalink');
expect(permalink.length).toBe(1);
// now we'll set the preview post to undefined. This happens when the
// previewed post is deleted.
props.previewPost = undefined;
wrapper = shallow(
<PostMessagePreview
{...props}
/>,
);
expect(wrapper).toMatchSnapshot();
permalink = wrapper.find('.permalink');
expect(permalink.length).toBe(0);
});
test('should not render bot icon', () => {
const postProps = {
override_icon_url: 'https://fakeicon.com/image.jpg',

View File

@@ -25,8 +25,10 @@ import {Constants} from 'utils/constants';
import {General} from 'mattermost-redux/constants';
import {OwnProps} from './index';
import {Post} from '@mattermost/types/posts';
export type Props = OwnProps & {
previewPost?: Post;
currentTeamUrl: string;
channelDisplayName?: string;
user: UserProfile | null;

View File

@@ -317,6 +317,59 @@ describe('posts', () => {
});
});
it('should remove deleted post from other post embeds', () => {
const post1 = {id: 'post1', message: 'Post 1'};
const post2 = {
id: 'post2',
message: 'Post 2',
metadata: {
embeds: [
{
type: 'permalink',
data: {
post_id: 'post1',
},
},
],
},
};
const post3 = {
id: 'post3',
message: 'Post 3',
metadata: {
embeds: [
{
type: 'permalink',
data: {
post_id: 'post1',
},
},
{
type: 'permalink',
data: {
post_id: 'post2',
},
},
],
},
};
const state = deepFreeze({
post1,
post2,
post3,
});
const nextState = reducers.handlePosts(state, {
type: PostTypes.POST_DELETED,
data: {id: 'post1'},
});
expect(nextState).not.toBe(state);
expect(nextState.post2.metadata.embeds.length).toBe(0);
expect(nextState.post3.metadata.embeds.length).toBe(1);
});
it('should not remove the rest of the thread when deleting a comment', () => {
const state = deepFreeze({
post1: {id: 'post1'},

View File

@@ -16,6 +16,8 @@ import {
PostOrderBlock,
MessageHistory,
PostAcknowledgement,
PostEmbed,
PostPreviewMetadata,
} from '@mattermost/types/posts';
import {UserProfile} from '@mattermost/types/users';
import {Reaction} from '@mattermost/types/reactions';
@@ -200,11 +202,44 @@ export function handlePosts(state: RelationOneToOne<Post, Post> = {}, action: Ge
},
};
// Remove any of its comments
for (const otherPost of Object.values(state)) {
// Remove any of its comments
if (otherPost.root_id === post.id) {
Reflect.deleteProperty(nextState, otherPost.id);
}
// a deleted post may exist in some other post's
// embeds when its link is mentioned in the post message.
// We need to remove the deleted post from post embeds of all posts
// to ensure the deleted post's contents cannot be retrieved from the store.
if (otherPost.metadata && otherPost.metadata.embeds && otherPost.metadata.embeds.length > 0) {
// This will become the post's new embeds array.
// We'll add everything other than the deleted post's embed here.
const newEmbeds: PostEmbed[] = [];
for (const embed of otherPost.metadata.embeds) {
if (embed.type === 'permalink' && (embed.data as PostPreviewMetadata).post_id === post.id) {
// skip if the embed is the deleted post
continue;
}
// include everything else
newEmbeds.push(embed);
}
// if newEmbeds changed, update post's embeds
if (newEmbeds.length !== otherPost.metadata.embeds.length) {
// Since otherPost refers to the post from store, its frozen un immutable.
// That's why cloning it and modifying required parts here.
nextState[otherPost.id] = {
...nextState[otherPost.id],
metadata: {
...nextState[otherPost.id].metadata,
embeds: newEmbeds,
},
};
}
}
}
return nextState;