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

7441 category share data #7606

Merged
merged 11 commits into from
Oct 19, 2021
Merged

7441 category share data #7606

merged 11 commits into from
Oct 19, 2021

Conversation

danielfmiranda
Copy link
Collaborator

@danielfmiranda danielfmiranda commented Oct 12, 2021

Closes #7441
Related PRs/issues #

Link to sample test page: https://foundation-s-7441-categ-8ryjva.herokuapp.com/en/privacynotincluded/categories/health-exercise/

Expected Results:
Screen Shot 2021-10-12 at 5 36 12 PM

Steps to test:

  1. For testing purposes, I have added a custom image to the "Health And Exercise" category on the staging app already. If you would like to upload a picture yourself in the CMS for a category, please feel free to do so!
  2. Visit the page above, which is the Entertainment category PNI page
  3. Copy and paste the URL and send it through slack, or the other platform of your choice.
  4. If the preview for the page looks like the picture above, testing is complete!

Checklist

Changes in Models:

  • [Are my changes backward-compatible]

network-api/networkapi/wagtailpages/pagemodels/products.py Outdated Show resolved Hide resolved
Comment on lines 1609 to 1612
if category.share_image:
setattr(self, 'search_image_id', category.share_image_id)
if category.description:
setattr(self, 'search_description', category.description)
Copy link
Contributor

Choose a reason for hiding this comment

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

I know this is still marked as WIP, but wanted to flag early that we will want to make this work for localized categories too.
I think relying on category.localized should work here?

@mofodevops mofodevops temporarily deployed to foundation-s-7441-categ-6pnfbc October 13, 2021 00:04 Inactive
@mofodevops mofodevops temporarily deployed to foundation-s-7441-categ-8ryjva October 13, 2021 00:04 Inactive
@mofodevops mofodevops temporarily deployed to foundation-s-7441-categ-8ryjva October 13, 2021 00:11 Inactive
@mofodevops mofodevops temporarily deployed to foundation-s-7441-categ-8ryjva October 13, 2021 00:22 Inactive
@danielfmiranda danielfmiranda requested review from TheoChevalier, Pomax and kristinashu and removed request for Pomax October 13, 2021 01:22
@mofodevops mofodevops temporarily deployed to foundation-s-7441-categ-8ryjva October 13, 2021 01:36 Inactive
@danielfmiranda
Copy link
Collaborator Author

Hi @TheoChevalier and @Pomax I have a quick question regarding localization.

I have added the feedback that Theo suggested, but noticed that on my local and test environment, I will receive a 500 error saying "Locale matching query does not exist."

Would there be a way to test this on the test environment?
I have tried to create a new locale category on the test environment but am unable to find the option to do so.

Also, I have implemented your feedback Theo, however I am unsure if I am using it correctly:

       if category.share_image:
            setattr(self, 'search_image_id', category.localized.share_image_id)
        if category.description:
            setattr(self, 'search_description', category.localized.description)

From my understanding, if the category has either of these things, then we want to set the metadata to the localised version?

Thanks in advance!

@danielfmiranda danielfmiranda changed the title WIP: 7441 category share data 7441 category share data Oct 13, 2021
@kristinashu
Copy link

Hi @danielfmiranda, should this be working? When I click on https://foundation-s-7441-categ-8ryjva.herokuapp.com/en/privacynotincluded/categories/entertainment/, I see Server Error (500).

@danielfmiranda
Copy link
Collaborator Author

Copy link

@kristinashu kristinashu left a comment

Choose a reason for hiding this comment

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

Thank you, this is working well!

@TheoChevalier
Copy link
Contributor

Hi @TheoChevalier and @Pomax I have a quick question regarding localization.

I have added the feedback that Theo suggested, but noticed that on my local and test environment, I will receive a 500 error saying "Locale matching query does not exist."

Would there be a way to test this on the test environment? I have tried to create a new locale category on the test environment but am unable to find the option to do so.

Also, I have implemented your feedback Theo, however I am unsure if I am using it correctly:

       if category.share_image:
            setattr(self, 'search_image_id', category.localized.share_image_id)
        if category.description:
            setattr(self, 'search_description', category.localized.description)

From my understanding, if the category has either of these things, then we want to set the metadata to the localised version?

Thanks in advance!

Hey @danielfmiranda, thanks! Your code looks good, and I was able to test locally it works.
The 500 error is not due to this PR — after adding a locale in Settings > Locale, you need to run inv manage sync_locale_trees in order to create all the page + snippet aliases for the new locale.

To test in the review app you’d need to:

  • Add a new locale in Settings > Locale, synced with English.
  • Run python network-api/manage.py sync_locale_trees as an Heroku command
  • Translate a product category snippet
  • Edit the translation / override the image

@danielfmiranda danielfmiranda temporarily deployed to foundation-s-7441-categ-8ryjva October 15, 2021 17:03 Inactive
@danielfmiranda danielfmiranda temporarily deployed to foundation-s-7441-categ-8ryjva October 18, 2021 20:31 Inactive
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.

just one small note

FieldPanel('description'),
FieldPanel('featured'),
FieldPanel('hidden'),
FieldPanel('slug'),
Copy link
Contributor

Choose a reason for hiding this comment

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

if we're adding explicit panels, we can hide this autogenerated field by simply omitting it from the panel list.

@danielfmiranda danielfmiranda temporarily deployed to foundation-s-7441-categ-8ryjva October 18, 2021 23:52 Inactive
@danielfmiranda danielfmiranda temporarily deployed to foundation-s-7441-categ-8ryjva October 19, 2021 00:25 Inactive
@danielfmiranda
Copy link
Collaborator Author

Thanks for the catch @Pomax! I have removed the "slug" field from the panels and re requested review

@Pomax Pomax merged commit 3c67027 into pni-q3-2021 Oct 19, 2021
@Pomax Pomax deleted the 7441-category-share-data branch October 19, 2021 15:15
Pomax pushed a commit that referenced this pull request Oct 19, 2021
… slider (#7402)

CI looks stuck - overriding

added external link icon and removed price/dollar-sign (#7394)

* added external link icon and removed price/dollar-sign

* Update product.scss

* Update product.scss

Co-authored-by: Daniel Miranda <daniel@mozillafoundation.org>

Update continous-integration.yml

Update prod-like-ci.yml

added new date format to match figma file (#7395)

* added new date format to match figma file

Co-authored-by: Pomax <pomax@nihongoresources.com>
Co-authored-by: Daniel Miranda <daniel@mozillafoundation.org>

Updated order of items, as well as renamed Updates section to News (#7400)

* Updated order of items, as well as renamed Updates section to News

Co-authored-by: Pomax <pomax@nihongoresources.com>
Co-authored-by: Daniel Miranda <daniel@mozillafoundation.org>

Create README.md

Delete README.md

7374 - Added Category Bubbles

Added padding

hide the softwareproductpage child page type (#7419)

removed company contact information fields from template and cms/models (#7401)

* removed company contact information fields from template and cms/models

Co-authored-by: Pomax <pomax@nihongoresources.com>
Co-authored-by: Daniel Miranda <daniel@mozillafoundation.org>

prettier

7294 - PNI category name untranslated in page title (#7455)

* 7294 - PNI category name untranslated in page title

linting fix

* Update network-api/networkapi/wagtailpages/pagemodels/products.py

Co-authored-by: Pomax <pomax@nihongoresources.com>

Co-authored-by: Pomax <pomax@nihongoresources.com>

updated dotted background bar to be around comments (#7450)

Co-authored-by: Daniel Miranda <daniel@mozillafoundation.org>

7374 - Added Category Bubbles

Added padding

7387-update typesetting styling

added mozilla says section (#7446)

* rebase

* updated migrations

* Update product_page.html

* Update network-api/networkapi/wagtailpages/templates/buyersguide/product_page.html

Co-authored-by: Théo Chevalier <theo.chevalier11@gmail.com>

* Update product_page.html

* updated rebase and commits

Co-authored-by: Daniel Miranda <daniel@mozillafoundation.org>
Co-authored-by: Théo Chevalier <theo.chevalier11@gmail.com>

ai product fields (#7488)

* added fields to be used in the AI panel

* Update products.py

Co-authored-by: Daniel Miranda <daniel@mozillafoundation.org>

Localize PNI category name in page title (#7478)

* Localize PNI category name in page title

* Line too long

7492 additional ai fields (#7513)

* added new fields, however, need to ask whether or not we want the helptext fields to go along or if they are static:

* updated AI fields, as well as reordered them in the code and CMS to match new PNI product page template

Co-authored-by: Daniel Miranda <daniel@mozillafoundation.org>

7389 section hrs (#7452)

* added new section dividers for news and related products

* updated spacing between news heading and section

Co-authored-by: Daniel Miranda <daniel@mozillafoundation.org>

update the ProductUpdate meta class to include ordering (#7530)

Localize category badge (#7565)

7472 - Search - update placeholder text

added new lead in paragraph under the comments heading (#7572)

* added new lead in paragraph under the comments heading

* Update product_page.html

Co-authored-by: Daniel Miranda <daniel@mozillafoundation.org>

7382 - product tabs component

Added sticky

Fixes

update

Design revisions

Refactor

Addressed

fixes

fixes

7377 mozilla researched (#7539)

* added new template to house all of the research details

* Update research_details.html

* New localization approach for Mozilla researched (#7553)

Co-authored-by: Daniel Miranda <daniel@mozillafoundation.org>
Co-authored-by: Théo Chevalier <theo.chevalier11@gmail.com>

7473 change "all" category on mobile to a more indicative text (#7570)

fix

7379 people voted (#7541)

* added migrations for mozilla researched

* merge with q3 branch

* merge with q3 branch

* updated migrations

* saving to ask design for what to do on the medium breakpoint for the mozilla says and researched section.

* updated to use tailwind

* added new template to house all of the research details

* updated migrations

* Update research_details.html

* 7379-people-voted

* New localization approach for Mozilla researched (#7553)

* New localization approach for Mozilla researched

* Remove extra spaces and simplify string

* added requested changes from design and review

* New localization approach for People voted (#7554)

* New localization approach for People voted

* remove extra spaces

* Simplify string

* Update most_voted_rating.html

* spacing

* Update research_details.html

* comments

* removed % sign, implemented feedback from review

* Removed rogue space

* Update network-api/networkapi/wagtailpages/templates/fragments/most_voted_rating.html

Co-authored-by: Théo Chevalier <theo.chevalier11@gmail.com>

* merging with q3

Co-authored-by: Daniel Miranda <daniel@mozillafoundation.org>
Co-authored-by: Théo Chevalier <theo.chevalier11@gmail.com>
Co-authored-by: Pomax <pomax@nihongoresources.com>

7393 - Update Content Width

Fixed

7380 tips to protect free text (#7576)

* tips to protect yourself using richtext field

Co-authored-by: Daniel Miranda <daniel@mozillafoundation.org>
Co-authored-by: Pomax <pomax@nihongoresources.com>

updated blurb and worst case to rich text fields with 5000 char limit (#7605)

Co-authored-by: Daniel Miranda <daniel@mozillafoundation.org>

7470/7474 relocate search bar (#7591)

* removed unneeded JS, updated CSS, and relocated search bar to hero section of the page

* updated use of variable for color white

Co-authored-by: Daniel Miranda <daniel@mozillafoundation.org>
Co-authored-by: Pomax <pomax@nihongoresources.com>

7383 - Product page - add "information" links to certain fields

Update network-api/networkapi/wagtailpages/templates/fragments/product_criterion_primary_info.html

Co-authored-by: Théo Chevalier <theo.chevalier11@gmail.com>

Update network-api/networkapi/wagtailpages/templates/buyersguide/product_page.html

Co-authored-by: Théo Chevalier <theo.chevalier11@gmail.com>

7386 - Content Update

fixes

Update network-api/networkapi/wagtailpages/templates/buyersguide/product_page.html

Co-authored-by: Théo Chevalier <theo.chevalier11@gmail.com>

Changes

vote now button enabled on-click (#7627)

* vote now button enabled on-click

* removed box shadow while disabled

* transition

Co-authored-by: Daniel Miranda <daniel@mozillafoundation.org>

updated styling of search bar (#7604)

* removed unneeded JS, updated CSS, and relocated search bar to hero section of the page

* updated use of variable for color white

* updated styling of search bar to be round with 1px black border, 6 col width

* css changes requested by design

Co-authored-by: Daniel Miranda <daniel@mozillafoundation.org>

7441 category share data (#7606)

* added new image field, then checking if we can use them for share data

* comment

* migrations and Theo's localization feedback

* formatting

* updated migrations

* localization of category share data

* Removing slug from editable panels

Co-authored-by: Daniel Miranda <daniel@mozillafoundation.org>

Add subcategory functionality to PNI categories. (#7641)

* add "subcategories" by letting categories specify a parent.

migration fix
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.

None yet

5 participants