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

feat: source category entity #2119

Merged
merged 13 commits into from
Aug 14, 2024
Merged

feat: source category entity #2119

merged 13 commits into from
Aug 14, 2024

Conversation

sshanzel
Copy link
Member

@sshanzel sshanzel commented Aug 8, 2024

  • Introduce source category entity.
  • Add categoryId column on the source entity.
  • update sources query to filter by category

MI-483

@sshanzel sshanzel requested review from capJavert and a team as code owners August 8, 2024 15:24

This comment has been minimized.

@@ -275,7 +275,7 @@ const createPost = async ({
data.score = Math.floor(postCreatedAt.getTime() / (1000 * 60));
data.origin = data?.scoutId
? PostOrigin.CommunityPicks
: (data.origin ?? PostOrigin.Crawler);
Copy link
Member Author

@sshanzel sshanzel Aug 8, 2024

Choose a reason for hiding this comment

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

Lint was screaming about this locally, now CI screaming otherwise :hidethepain:

This comment has been minimized.

Copy link
Contributor

@capJavert capJavert left a comment

Choose a reason for hiding this comment

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

Looking forward to categories on sources! Some comments to standardize, nothing breaking really.

src/entity/Source.ts Outdated Show resolved Hide resolved
src/migration/1723118739478-SourceCategory.ts Outdated Show resolved Hide resolved
src/migration/1723118739478-SourceCategory.ts Outdated Show resolved Hide resolved
src/schema/sources.ts Outdated Show resolved Hide resolved
src/entity/sources/SourceCategory.ts Outdated Show resolved Hide resolved
src/entity/sources/SourceCategory.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@rebelchris rebelchris left a comment

Choose a reason for hiding this comment

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

Couple queries besides what Ante logged

__tests__/sources.ts Outdated Show resolved Hide resolved
__tests__/sources.ts Outdated Show resolved Hide resolved
src/entity/sources/SourceCategory.ts Show resolved Hide resolved

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

Copy link
Contributor

@rebelchris rebelchris left a comment

Choose a reason for hiding this comment

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

Non blocking remarks left :)

@@ -266,6 +266,9 @@ const obj = new GraphORM({
},
},
},
SourceCategory: {
requiredColumns: ['createdAt'],
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this one required only?

Copy link
Contributor

Choose a reason for hiding this comment

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

Its due to paginated query being ordered by date so it needs to be included in graphorm context always

Copy link
Member Author

Choose a reason for hiding this comment

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

The createdAt is needed for the pagination, technically you can have everything missing and not break the query.

src/migration/1723579275223-SourceCategory.ts Show resolved Hide resolved

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

Copy link

pulumi bot commented Aug 14, 2024

🍹 The Update (preview) for dailydotdev/api/prod was successful.

Resource Changes

    Name                                        Type                           Operation
~   vpc-native-check-analytics-report-cron      kubernetes:batch/v1:CronJob    update
~   vpc-native-private-deployment               kubernetes:apps/v1:Deployment  update
~   vpc-native-update-tags-str-cron             kubernetes:batch/v1:CronJob    update
~   vpc-native-update-views-cron                kubernetes:batch/v1:CronJob    update
~   vpc-native-sync-subscription-with-cio-cron  kubernetes:batch/v1:CronJob    update
+   vpc-native-api-migration-aa537e13           kubernetes:batch/v1:Job        create
~   vpc-native-update-highlighted-views-cron    kubernetes:batch/v1:CronJob    update
~   vpc-native-bg-deployment                    kubernetes:apps/v1:Deployment  update
~   vpc-native-update-trending-cron             kubernetes:batch/v1:CronJob    update
~   vpc-native-personalized-digest-cron         kubernetes:batch/v1:CronJob    update
~   vpc-native-temporal-deployment              kubernetes:apps/v1:Deployment  update
~   vpc-native-clean-zombie-users-cron          kubernetes:batch/v1:CronJob    update
~   vpc-native-generate-search-invites-cron     kubernetes:batch/v1:CronJob    update
~   vpc-native-deployment                       kubernetes:apps/v1:Deployment  update
~   vpc-native-update-current-streak-cron       kubernetes:batch/v1:CronJob    update
~   vpc-native-clean-zombie-images-cron         kubernetes:batch/v1:CronJob    update
~   vpc-native-hourly-notification-cron         kubernetes:batch/v1:CronJob    update
~   vpc-native-daily-digest-cron                kubernetes:batch/v1:CronJob    update
~   vpc-native-ws-deployment                    kubernetes:apps/v1:Deployment  update
~   vpc-native-personalized-digest-deployment   kubernetes:apps/v1:Deployment  update
~   vpc-native-update-tag-recommendations-cron  kubernetes:batch/v1:CronJob    update
~   vpc-native-generic-referral-reminder-cron   kubernetes:batch/v1:CronJob    update
-   vpc-native-api-migration-a462949b           kubernetes:batch/v1:Job        delete
~   vpc-native-update-source-tag-view-cron      kubernetes:batch/v1:CronJob    update

@sshanzel sshanzel merged commit 362d6ef into main Aug 14, 2024
8 checks passed
@sshanzel sshanzel deleted the MI-483 branch August 14, 2024 16:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants