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

PNI products inheriting share image #7655

Merged
merged 4 commits into from
Oct 22, 2021
Merged

PNI products inheriting share image #7655

merged 4 commits into from
Oct 22, 2021

Conversation

danielfmiranda
Copy link
Collaborator

@danielfmiranda danielfmiranda commented Oct 21, 2021

Closes #7165
Related PRs/issues #

Link to sample test pages and Metadata preview:

Test PNI homepage, with custom meta description and image:
https://foundation-s-pni-share--4dl0xc.mofostaging.net/en/privacynotincluded/
Screen Shot 2021-10-20 at 6 38 19 PM

Test PNI Product, with custom meta description and image:
https://foundation-s-pni-share--4dl0xc.mofostaging.net/en/privacynotincluded/minute-dinner-also/
image

Test PNI product with no custom metadata set, should inherit from PNI homepage:
https://foundation-s-pni-share--4dl0xc.mofostaging.net/en/privacynotincluded/gas-word-mr-front-involve-street-hold/
image

Steps to test:

  1. Copy and paste the links above in a messaging app of your choice, and the metadata should match the images next to each URL.
  2. Please note that the third link, has no metadata set, which means it is inheriting the metadata set from the PNI home page. If we were to set either description or image, it would appear here.
  3. If everything is working as expected, testing is complete!

Checklist

Changes in Models:

@mofodevops mofodevops temporarily deployed to foundation-s-pni-share--4dl0xc October 21, 2021 01:05 Inactive
@github-actions
Copy link

This PR introduces visual differences. Click here to inspect the diffs.

@kristinashu
Copy link

Hi @danielfmiranda, this all looks and is an improvement. I'm also sorry tho because I think I mixed up this ticket. I thought this one was about how the custom category share image/text isn't working. And for products, there has been a more recent request in #7625.

What you have is an improvement so I think you could merge it but I'm not sure if you want to add the broken category issue to this PR or a new one?

For the category issue, I have uploaded a custom image for Entertainment:

image

But when I share it, I see the main image and text:

image

Sorry for the mix up, but I'm guessing what broke the Product and Category share info is related?

Copy link
Contributor

@Pomax Pomax left a comment

Choose a reason for hiding this comment

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

good catch

@danielfmiranda
Copy link
Collaborator Author

Hi @kristinashu, no problem! the category share data fix has actually been merged into the q3-pni-2021 branch through this pr: #7606, which from what I understand, should be merged into the main branch very soon.

This PR however does fix an issue for PNI Products and Category Pages where instead of having the PNI metadata, they would instead show the Foundation Site Metadata.

With this PR, they will now check their parent pages for metadata, and if it is set, it will display those. However, if the parent page, like the PNI homepage for example, does not have any metadata set, it will then default to the foundation metadata.

I will merge this branch in, and then take care of #7625 shortly. Once I finish that ticket, the metadata process should go:

Product Page Metadata > PNI metadata > Foundation Site Metadata
Category Page Metadata > PNI Metadata > Foundation Site Metadata

@danielfmiranda danielfmiranda temporarily deployed to foundation-s-pni-share--4dl0xc October 21, 2021 17:29 Inactive
@kristinashu
Copy link

Ah good, I knew there was a category ticket somewhere but couldn't find it!

@github-actions
Copy link

This PR introduces visual differences. Click here to inspect the diffs.

@danielfmiranda danielfmiranda temporarily deployed to foundation-s-pni-share--4dl0xc October 21, 2021 20:54 Inactive
@github-actions
Copy link

This PR introduces visual differences. Click here to inspect the diffs.

@danielfmiranda danielfmiranda temporarily deployed to foundation-s-pni-share--4dl0xc October 21, 2021 23:18 Inactive
@github-actions
Copy link

This PR introduces visual differences. Click here to inspect the diffs.

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.

PNI product pages should show PNI OG image
4 participants