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: use shared and content title and image for post types #2477

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

capJavert
Copy link
Contributor

@capJavert capJavert commented Nov 21, 2024

  • no posts in email was due to outdated templates being scheduled on our email platform, it was a temporary issue and it should not happen now that everything scheduled before code change was deploy
  • no title is because of shared posts, we can show shared post title
  • no image, freeform post without image (we should technically load first image from content)
    • also case for shared post, we would load image of a shared post

Closes dailydotdev/daily#1600

Comment on lines +604 to +608
export const findPostImageFromContent = ({
post,
}: {
post: Pick<FreeformPost, 'content'>;
}): string | undefined => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

get first post image from content

Comment on lines +174 to +176
if (depth > maxDepth) {
return undefined;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

since this is recursive (due to markdown structure being tree) I added depth so we don't go too deep and affect performance.

By default its 2 level, which should be more then enough for markdown content we can create in our editor.

@capJavert capJavert changed the title fix: use shared and contnet title and image for post types fix: use shared and content title and image for post types Nov 21, 2024
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.

Cool :)

@@ -88,8 +93,13 @@ const getPostsTemplateData = ({
const summary = post.summary || '';

return {
post_title: post.title,
post_image: mapCloudinaryUrl(post.image || pickImageUrl(post)),
post_title: post.title || post.sharedPostTitle,
Copy link
Contributor

Choose a reason for hiding this comment

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

I thought I already added this, but maybe it was for another email.

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.

🐛 BUG: Personal update email not showing post info.
2 participants