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

fix: add allowed_content_curation flag to feed config #2123

Merged
merged 7 commits into from
Aug 13, 2024
Merged

fix: add allowed_content_curation flag to feed config #2123

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
f6e9635
fix: add allowed_content_curation flag to feed config
rebelchris Aug 9, 2024
84925a2
fix: rename to includeContentCuration
rebelchris Aug 9, 2024
3139ea2
Merge branch 'main' into AS-505-allowed_content_curation
rebelchris Aug 9, 2024
4ef49de
fix: remove blocked content curations
rebelchris Aug 13, 2024
27ae3e1
Merge branch 'main' into AS-505-allowed_content_curation
rebelchris Aug 13, 2024
2d0038c
Merge branch 'main' into AS-505-allowed_content_curation
rebelchris Aug 13, 2024
bb16555
fix: test case
rebelchris Aug 13, 2024
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
45 changes: 45 additions & 0 deletions __tests__/integrations/feed.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,13 @@ describe('FeedPreferencesConfigGenerator', () => {
defaultEnabledState: true,
options: { type: PostType.Article },
},
{
title: 'News',
group: 'content_curation',
description: '',
defaultEnabledState: true,
options: { type: 'news' },
},
]);
await con.getRepository(FeedAdvancedSettings).save([
{ feedId: '1', advancedSettingsId: 1, enabled: false },
Expand Down Expand Up @@ -222,6 +229,44 @@ describe('FeedPreferencesConfigGenerator', () => {
});
});

it('should generate feed config with blocked content curation', async () => {
await con
.getRepository(FeedAdvancedSettings)
.save([{ feedId: '1', advancedSettingsId: 3, enabled: false }]);
const generator: FeedConfigGenerator = new FeedPreferencesConfigGenerator(
config,
{
includeBlockedContentCuration: true,
},
);

const actual = await generator.generate(ctx, {
user_id: '1',
page_size: 2,
offset: 3,
});
expect(actual).toEqual({
config: {
allowed_content_curations: [
'release',
'opinion',
'listicle',
'comparison',
'tutorial',
'story',
'meme',
],
blocked_content_curations: ['news'],
feed_config_name: FeedConfigName.Personalise,
fresh_page_size: '1',
offset: 3,
page_size: 2,
total_pages: 20,
user_id: '1',
},
});
});

it('should generate feed config with blocked tags and sources', async () => {
const generator: FeedConfigGenerator = new FeedPreferencesConfigGenerator(
config,
Expand Down
13 changes: 13 additions & 0 deletions src/integrations/feed/configs.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,16 @@ type Options = {
};

type BaseConfig = Partial<Omit<FeedConfig, 'user_id' | 'page_size' | 'offset'>>;
const AllowedContentCurationTypes = [
'news',
'release',
'opinion',
'listicle',
'comparison',
'tutorial',
'story',
'meme',
] as const;

function getDefaultConfig(
baseConfig: BaseConfig,
Expand Down Expand Up @@ -88,6 +98,9 @@ const addFiltersToConfig = ({
opts.includeBlockedContentCuration
rebelchris marked this conversation as resolved.
Show resolved Hide resolved
) {
baseConfig.blocked_content_curations = filters.blockedContentCuration;
Copy link
Member

Choose a reason for hiding this comment

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

I think we don't need this anymore now

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What about the lofn provider? (And maybe the one that uses the digest?)

@vpol you probably know best, can we remove it?

Copy link
Contributor

@vpol vpol Aug 13, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

lofn does not use/parse any of the filters at all.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

And with your fix nothing else uses the old format right so removing it is fine? 🙏

Copy link
Contributor

Choose a reason for hiding this comment

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

we still support it on the feed side, but if you are not going to use it - it's ok.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed :)

baseConfig.allowed_content_curations = AllowedContentCurationTypes.filter(
(type) => !filters.blockedContentCuration.includes(type),
);
}

return baseConfig;
Expand Down
1 change: 1 addition & 0 deletions src/integrations/feed/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ export type FeedConfig = {
blocked_sources?: string[];
allowed_post_types?: string[];
blocked_content_curations?: string[];
allowed_content_curations?: string[];
squad_ids?: string[];
providers?: Record<string, FeedProvider>;
source_types?: ('machine' | 'squad')[];
Expand Down
Loading