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

fix: shared post empty state #2615

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion __tests__/cron/checkAnalyticsReport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ it('should publish message for every post that needs analytics report', async ()
const posts = await con
.getRepository(Post)
.findBy({ sentAnalyticsReport: true });
expect(posts.length).toEqual(4);
expect(posts.length).toEqual(5);
expect(posts.map(({ id }) => id)).toEqual(
expect.arrayContaining(['p3', 'p4', 'p5']),
);
Expand Down
3 changes: 2 additions & 1 deletion __tests__/cron/updateTagsStr.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {
import { sourcesFixture } from '../fixture/source';
import { postsFixture, sharedPostsFixture } from '../fixture/post';
import { Checkpoint } from '../../src/entity/Checkpoint';
import { DataSource } from 'typeorm';
import { DataSource, Not } from 'typeorm';
import createOrGetConnection from '../../src/db';

let con: DataSource;
Expand Down Expand Up @@ -57,6 +57,7 @@ it('should update post tagsStr with the all recently updated keywords', async ()
const posts = await con.getRepository(Post).find({
select: ['id', 'tagsStr'],
order: { id: 'ASC' },
where: { id: Not('404') },
});
expect(posts).toMatchSnapshot();
});
10 changes: 6 additions & 4 deletions __tests__/cron/updateTrending.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { sourcesFixture } from '../fixture/source';
import { postsFixture, sharedPostsFixture } from '../fixture/post';

import { DeepPartial } from 'typeorm/common/DeepPartial';
import { DataSource } from 'typeorm';
import { DataSource, Not } from 'typeorm';
import createOrGetConnection from '../../src/db';

let con: DataSource;
Expand Down Expand Up @@ -51,9 +51,11 @@ it('should update the trending score of the relevant articles', async () => {
addViewsToPost(postsFixture[2].id, 30, halfHour),
]);
await expectSuccessfulCron(cron);
const posts = await con
.getRepository(Post)
.find({ select: ['id', 'trending'], order: { id: 'ASC' } });
const posts = await con.getRepository(Post).find({
select: ['id', 'trending'],
order: { id: 'ASC' },
where: { id: Not('404') },
});
expect(posts).toMatchSnapshot();
const trendingPost = await con.getRepository(Post).findOne({
select: ['id', 'lastTrending'],
Expand Down
3 changes: 2 additions & 1 deletion __tests__/cron/updateViews.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import cron from '../../src/cron/updateViews';
import { expectSuccessfulCron, saveFixtures } from '../helpers';
import { ArticlePost, Post, Source, View } from '../../src/entity';
import { sourcesFixture } from '../fixture/source';
import { DataSource } from 'typeorm';
import { DataSource, Not } from 'typeorm';
import createOrGetConnection from '../../src/db';

let con: DataSource;
Expand Down Expand Up @@ -62,6 +62,7 @@ it('should update views', async () => {
const posts = await con.getRepository(Post).find({
select: ['id', 'views', 'score', 'createdAt'],
order: { createdAt: 'ASC' },
where: { id: Not('404') },
});
expect(posts[0].views).toEqual(0);
expect(posts[1].views).toEqual(2);
Expand Down
29 changes: 16 additions & 13 deletions __tests__/workers/cdc/primary.ts
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ import {
relatedPostsFixture,
} from '../../fixture/post';
import { randomUUID } from 'crypto';
import { DataSource } from 'typeorm';
import { DataSource, Not } from 'typeorm';
import createOrGetConnection from '../../../src/db';
import { TypeOrmError } from '../../../src/errors';
import { SourceMemberRoles } from '../../../src/roles';
Expand Down Expand Up @@ -4855,7 +4855,7 @@ describe('source_post_moderation', () => {
it('should create freeform post', async () => {
const repo = con.getRepository(Post);
const before = await repo.find();
expect(before.length).toEqual(0);
expect(before.length).toEqual(1);
const after = {
...base,
status: SourcePostModerationStatus.Approved,
Expand Down Expand Up @@ -4886,7 +4886,7 @@ describe('source_post_moderation', () => {
await saveFixtures(con, User, badUsersFixture);
const repo = con.getRepository(Post);
const before = await repo.find();
expect(before.length).toEqual(0);
expect(before.length).toEqual(1);
const after = {
...base,
createdById: 'vordr',
Expand Down Expand Up @@ -4935,7 +4935,7 @@ describe('source_post_moderation', () => {
]);
const repo = con.getRepository(Post);
const before = await repo.find();
expect(before.length).toEqual(0);
expect(before.length).toEqual(1);
const after = {
...base,
status: SourcePostModerationStatus.Approved,
Expand All @@ -4959,7 +4959,7 @@ describe('source_post_moderation', () => {
it('should not create post if status did not change', async () => {
const repo = con.getRepository(Post);
const before = await repo.find();
expect(before.length).toEqual(0);
expect(before.length).toEqual(1);
const after = {
...base,
status: SourcePostModerationStatus.Approved,
Expand All @@ -4979,7 +4979,7 @@ describe('source_post_moderation', () => {
it('should not create share post when share post id is null', async () => {
const repo = con.getRepository(Post);
const before = await repo.find();
expect(before.length).toEqual(0);
expect(before.length).toEqual(1);
await mockUpdate({
type: PostType.Share,
status: SourcePostModerationStatus.Approved,
Expand Down Expand Up @@ -5054,7 +5054,7 @@ describe('source_post_moderation', () => {
.update({ id: 'b' }, { id: UNKNOWN_SOURCE });
const repo = con.getRepository(Post);
const before = await repo.find();
expect(before.length).toEqual(0);
expect(before.length).toEqual(1);
await mockUpdate({
sourceId: 'a',
type: PostType.Share,
Expand All @@ -5063,8 +5063,8 @@ describe('source_post_moderation', () => {
content: '# Sample',
contentHtml: '# Sample',
});
const unknown = await repo.findOneBy({ sourceId: UNKNOWN_SOURCE });
expect(unknown).toBeNull();
const unknown = await repo.findBy({ sourceId: UNKNOWN_SOURCE });
expect(unknown.length).toEqual(1);
const share = await repo.findOneBy({ sourceId: 'a' });
expect(share).toBeNull();
});
Expand All @@ -5075,7 +5075,7 @@ describe('source_post_moderation', () => {
.update({ id: 'b' }, { id: UNKNOWN_SOURCE });
const repo = con.getRepository(Post);
const before = await repo.find();
expect(before.length).toEqual(0);
expect(before.length).toEqual(1);
const after = {
...base,
sourceId: 'a',
Expand All @@ -5087,7 +5087,10 @@ describe('source_post_moderation', () => {
externalLink: 'https://daily.dev/blog-post/sauron',
};
await mockUpdate(after);
const unknown = await repo.findOneBy({ sourceId: UNKNOWN_SOURCE });
const unknown = await repo.findOneBy({
sourceId: UNKNOWN_SOURCE,
id: Not('404'),
});
expect(unknown).toBeTruthy();
expect(unknown.title).toEqual('Test');
const share = (await repo.findOneBy({
Expand Down Expand Up @@ -5116,7 +5119,7 @@ describe('source_post_moderation', () => {
},
]);
const before = await repo.find();
expect(before.length).toEqual(1);
expect(before.length).toEqual(2);
const after = {
...base,
sourceId: 'a',
Expand All @@ -5141,7 +5144,7 @@ describe('source_post_moderation', () => {
]);

const list = await con.getRepository(Post).find();
expect(list.length).toEqual(2); // to ensure nothing new was created other than the share post
expect(list.length).toEqual(3); // to ensure nothing new was created other than the share post
});

it('should update the content if post id is present', async () => {
Expand Down
23 changes: 22 additions & 1 deletion __tests__/workers/postBannedRep.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,11 @@ import { postsFixture } from '../fixture/post';
import { PostReport, ReputationEvent } from '../../src/entity';
import { DataSource, LessThan } from 'typeorm';
import createOrGetConnection from '../../src/db';
import { createSquadWelcomePost, updateFlagsStatement } from '../../src/common';
import {
createSquadWelcomePost,
DELETED_BY_WORKER,
updateFlagsStatement,
} from '../../src/common';
import { ReportReason } from '../../src/entity/common';

let con: DataSource;
Expand Down Expand Up @@ -61,6 +65,23 @@ it('should create a reputation event that increases reputation', async () => {
expect(events[1].amount).toEqual(100);
});

it('should not create a reputation event if deleted by worker', async () => {
const post = await con.getRepository(Post).findOneBy({ id: 'p1' });
await expectSuccessfulBackground(worker, {
post: {
...post,
flags: {
...post.flags,
deletedBy: DELETED_BY_WORKER,
},
},
});
const events = await con
.getRepository(ReputationEvent)
.find({ where: { targetId: 'p1', grantById: '' } });
expect(events.length).toEqual(0);
});

const createSharedPost = async (id = 'sp1', args: Partial<Post> = {}) => {
const post = await con.getRepository(Post).findOneBy({ id: 'p1' });
await con.getRepository(SharePost).save({
Expand Down
35 changes: 34 additions & 1 deletion __tests__/workers/postDeletedSharedPostCleanup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { Post, SharePost, Source } from '../../src/entity';
import { postsFixture, sharedPostsFixture } from '../fixture/post';
import { sourcesFixture } from '../fixture';
import { workers } from '../../src/workers';
import { DELETED_BY_WORKER } from '../../src/common';

let con: DataSource;
beforeEach(async () => {
Expand Down Expand Up @@ -34,6 +35,20 @@ beforeEach(async () => {
};
}),
);
await saveFixtures(
con,
SharePost,
sharedPostsFixture.map((item) => {
return {
...item,
id: `pdspc-nc-${item.id}`,
shortId: `pdspcns1`,
sharedPostId: `pdspc-p2`,
title: null,
titleHtml: null,
};
}),
);
});

describe('postDeletedSharedPostCleanup worker', () => {
Expand All @@ -49,7 +64,23 @@ describe('postDeletedSharedPostCleanup worker', () => {
expect(registeredWorker).toBeDefined();
});

it('should set shared post to not show on feed if post gets deleted', async () => {
it('should set shared post with no commentary to not show on feed and be shadow banned if post gets deleted', async () => {
await expectSuccessfulBackground(worker, {
post: {
id: 'pdspc-p2',
},
});
const sharedPost = await con.getRepository(SharePost).findOneBy({
id: 'pdspc-nc-squadP1',
});
expect(sharedPost?.sharedPostId).toBe('404');
expect(sharedPost?.deleted).toBe(true);
expect(sharedPost?.flags?.deletedBy).toBe(DELETED_BY_WORKER);
expect(sharedPost?.showOnFeed).toBe(false);
expect(sharedPost?.flags?.showOnFeed).toEqual(false);
});

it('should set shared post with commentary to not show on feed if post gets deleted', async () => {
await expectSuccessfulBackground(worker, {
post: {
id: 'pdspc-p1',
Expand All @@ -58,6 +89,8 @@ describe('postDeletedSharedPostCleanup worker', () => {
const sharedPost = await con.getRepository(SharePost).findOneBy({
id: 'pdspc-squadP1',
});
expect(sharedPost?.sharedPostId).toBe('404');
expect(sharedPost?.deleted).toBe(false);
expect(sharedPost?.showOnFeed).toBe(false);
expect(sharedPost?.flags?.showOnFeed).toEqual(false);
});
Expand Down
32 changes: 16 additions & 16 deletions __tests__/workers/postUpdated.ts
Original file line number Diff line number Diff line change
Expand Up @@ -445,8 +445,8 @@ it('should save a new post with basic information', async () => {
order: 0,
});
const posts = await con.getRepository(Post).find();
expect(posts.length).toEqual(3);
expect(posts[2]).toMatchSnapshot({
expect(posts.length).toEqual(4);
expect(posts[3]).toMatchSnapshot({
visible: true,
visibleAt: expect.any(Date),
createdAt: expect.any(Date),
Expand Down Expand Up @@ -476,8 +476,8 @@ it('should save a new post with with non-default language', async () => {
language: 'nb',
});
const posts = await con.getRepository(Post).find();
expect(posts.length).toEqual(3);
expect(posts[2]).toMatchObject({
expect(posts.length).toEqual(4);
expect(posts[3]).toMatchObject({
sourceId: 'a',
title: 'Title',
showOnFeed: true,
Expand All @@ -493,7 +493,7 @@ it('should set show on feed to true when order is missing', async () => {
source_id: 'a',
});
const posts = await con.getRepository(Post).find();
expect(posts.length).toEqual(3);
expect(posts.length).toEqual(4);
expect(posts[2].showOnFeed).toEqual(true);
});

Expand All @@ -506,8 +506,8 @@ it('should save a new post with showOnFeed information', async () => {
order: 1,
});
const posts = await con.getRepository(Post).find();
expect(posts.length).toEqual(3);
expect(posts[2].showOnFeed).toEqual(false);
expect(posts.length).toEqual(4);
expect(posts[3].showOnFeed).toEqual(false);
});

it('should save a new post with content curation', async () => {
Expand All @@ -521,7 +521,7 @@ it('should save a new post with content curation', async () => {
},
});
const posts = await con.getRepository(Post).find();
expect(posts.length).toEqual(3);
expect(posts.length).toEqual(4);
const post = await con
.getRepository(Post)
.findOneBy({ yggdrasilId: 'f99a445f-e2fb-48e8-959c-e02a17f5e816' });
Expand All @@ -540,9 +540,9 @@ it('save a post as public if source is public', async () => {
source_id: 'a',
});
const posts = await con.getRepository(Post).find();
expect(posts.length).toEqual(3);
expect(posts.length).toEqual(4);
expect(posts[2].private).toEqual(false);
expect(posts[2].flags.private).toEqual(false);
expect(posts[3].flags.private).toEqual(false);
});

it('save a post as private if source is private', async () => {
Expand All @@ -553,9 +553,9 @@ it('save a post as private if source is private', async () => {
source_id: 'p',
});
const posts = await con.getRepository(Post).find();
expect(posts.length).toEqual(3);
expect(posts[2].private).toBe(true);
expect(posts[2].flags.private).toBe(true);
expect(posts.length).toEqual(4);
expect(posts[3].private).toBe(true);
expect(posts[3].flags.private).toBe(true);
});

it('do not save post if source can not be found', async () => {
Expand Down Expand Up @@ -663,9 +663,9 @@ it('should save a new post with the relevant keywords', async () => {
},
});
const posts = await con.getRepository(Post).find();
expect(posts.length).toEqual(3);
expect(posts[2].scoutId).toEqual('1');
const tagsArray = posts[2].tagsStr.split(',');
expect(posts.length).toEqual(4);
expect(posts[3].scoutId).toEqual('1');
const tagsArray = posts[3].tagsStr.split(',');
['mongodb', 'alpinejs', 'ab-testing'].forEach((item) => {
expect(tagsArray).toContain(item);
});
Expand Down
6 changes: 6 additions & 0 deletions src/common/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,18 @@ import { ChangeObject } from '../types';

const REMOVE_SPECIAL_CHARACTERS_REGEX = /[^a-zA-Z0-9-_#.]/g;

export const DELETED_BY_WORKER = 'worker';

export const ghostUser = {
id: '404',
username: 'ghost',
name: 'Deleted user',
};

export const deletedPost = {
id: '404',
};

interface GetTimezonedIsoWeekProps {
date: Date;
timezone: string;
Expand Down
Loading
Loading