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: reject reason on notification #2485

Merged
merged 6 commits into from
Nov 25, 2024
Merged

fix: reject reason on notification #2485

merged 6 commits into from
Nov 25, 2024

Conversation

sshanzel
Copy link
Member

No description provided.

This comment has been minimized.

This comment has been minimized.

@@ -136,7 +137,7 @@ export const notificationTitleMap: Record<
source_post_approved: (ctx: NotificationPostContext) =>
`Woohoo! Your post has been approved and is now live in ${ctx.source.name}. Check it out!`,
source_post_rejected: (ctx: NotificationPostModerationContext) =>
`Your post in ${ctx.source.name} was not approved for the following reason: ${ctx.post.rejectionReason}. Please review the feedback and consider making changes before resubmitting.`,
`Your post in ${ctx.source.name} was not approved for the following reason: ${rejectReason[ctx.post.rejectionReason!]}. Please review the feedback and consider making changes before resubmitting.`,
Copy link
Contributor

Choose a reason for hiding this comment

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

For Other it will also show the commentary somewhere?

Copy link
Member Author

@sshanzel sshanzel Nov 25, 2024

Choose a reason for hiding this comment

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

Yes. Should be part of the description already.

Copy link
Contributor

Choose a reason for hiding this comment

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

should we fallback to other, just for safety or even throw error for some invalid value?

Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe we can fallback to Other. Wdyt?

Copy link
Contributor

Choose a reason for hiding this comment

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

sounds good to me

@@ -136,7 +137,7 @@ export const notificationTitleMap: Record<
source_post_approved: (ctx: NotificationPostContext) =>
`Woohoo! Your post has been approved and is now live in ${ctx.source.name}. Check it out!`,
source_post_rejected: (ctx: NotificationPostModerationContext) =>
`Your post in ${ctx.source.name} was not approved for the following reason: ${ctx.post.rejectionReason}. Please review the feedback and consider making changes before resubmitting.`,
`Your post in ${ctx.source.name} was not approved for the following reason: ${rejectReason[ctx.post.rejectionReason!]}. Please review the feedback and consider making changes before resubmitting.`,
Copy link
Contributor

Choose a reason for hiding this comment

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

should we fallback to other, just for safety or even throw error for some invalid value?

This comment has been minimized.

This comment has been minimized.

Copy link

pulumi bot commented Nov 25, 2024

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

Resource Changes

    Name                                            Type                           Operation
~   vpc-native-personalized-digest-deployment       kubernetes:apps/v1:Deployment  update
~   vpc-native-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-check-analytics-report-cron          kubernetes:batch/v1:CronJob    update
~   vpc-native-generate-search-invites-cron         kubernetes:batch/v1:CronJob    update
~   vpc-native-daily-digest-cron                    kubernetes:batch/v1:CronJob    update
~   vpc-native-update-current-streak-cron           kubernetes:batch/v1:CronJob    update
~   vpc-native-clean-zombie-users-cron              kubernetes:batch/v1:CronJob    update
~   vpc-native-sync-subscription-with-cio-cron      kubernetes:batch/v1:CronJob    update
-   vpc-native-api-migration-60954948               kubernetes:batch/v1:Job        delete
~   vpc-native-private-deployment                   kubernetes:apps/v1:Deployment  update
~   vpc-native-bg-deployment                        kubernetes:apps/v1:Deployment  update
~   vpc-native-temporal-deployment                  kubernetes:apps/v1:Deployment  update
~   vpc-native-update-views-cron                    kubernetes:batch/v1:CronJob    update
~   vpc-native-update-highlighted-views-cron        kubernetes:batch/v1:CronJob    update
~   vpc-native-update-source-tag-view-cron          kubernetes:batch/v1:CronJob    update
~   vpc-native-update-source-public-threshold-cron  kubernetes:batch/v1:CronJob    update
~   vpc-native-ws-deployment                        kubernetes:apps/v1:Deployment  update
~   vpc-native-update-trending-cron                 kubernetes:batch/v1:CronJob    update
~   vpc-native-clean-zombie-images-cron             kubernetes:batch/v1:CronJob    update
~   vpc-native-clean-zombie-user-companies-cron     kubernetes:batch/v1:CronJob    update
-   vpc-native-api-migration-b10a43d3               kubernetes:batch/v1:Job        delete
~   vpc-native-update-tags-str-cron                 kubernetes:batch/v1:CronJob    update
~   vpc-native-personalized-digest-cron             kubernetes:batch/v1:CronJob    update
+   vpc-native-api-migration-42e243dd               kubernetes:batch/v1:Job        create
~   vpc-native-hourly-notification-cron             kubernetes:batch/v1:CronJob    update
~   vpc-native-calculate-top-readers-cron           kubernetes:batch/v1:CronJob    update

@sshanzel sshanzel merged commit 5f95c1b into main Nov 25, 2024
8 checks passed
@sshanzel sshanzel deleted the WT-646 branch November 25, 2024 10:19
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.

3 participants