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

Rename ReportingExamplePage to Example Page, add features #3303

Merged
merged 6 commits into from
Nov 4, 2019

Conversation

johnnyporkchops
Copy link
Contributor

@johnnyporkchops johnnyporkchops commented Oct 28, 2019

Summary

Rename ReportingExamplePage to Example Page, add features
- Resolves #3149

  • Rename ReportingExample page to ExamplePage

  • makemigrations/migrate

  • Add new features to ExamplePage

  • makemigrations

  • Restore local database to latest dump

  • Combine the operations in the last two migrations above by manually editing the first to include operatons from the second * and then delete the second file .(rename page first, then add new features.) This way I only push one new migration file up.

  • migrate again
    *Note: If I had renamed the model and added new features in the same makemigrations action, Django would have considered the ExamplePage as a whole new model and deleted the ReportingExamplePage model , making existing pages based on that would not work, as I found in testing. If the model is exactly the same but has a new name, Django asks if you intended to just rename it. Then you can add your new features to the model and migrate again without breaking pages created under the old name.

  • Followup issue: Its probably a good time to squash migrations. See Squash CMS database migrations #3094 and Django Docs:

Impacted areas of the application

new file: home/migrations/0106_auto_20191101_1759.py
modified: home/models.py
modified: home/templates/home/reporting_example_page.html

How to test

Content team: This can be tested on feature space https://fec-feature-cms.app.cloud.gov/admin
Don't forget to test existing pages that were created originally with ReportingExampePage before it was renamed:
Go to Add Child Page > on the far right column of the page types listing you can click on Pages using Example page and spot check that these pages match what is on production

Developers:

  1. Checkout feature/3149-ExamplePage-template-wagtail
  2. ./manage.py migrate
  3. Go to Wagtail admin: http://localhost:8000/admin
  4. Create and preview/publish a page of type ExamplePage
  5. Test the image, external-link, internal-link blocks
  6. Embed a video using the Paragraph blocks RTE embed media option
  7. It is also important to spot-check existing pages that were created originally with ReportingExamplePage template, before it was renamed.
    Go to Add Child Page > on the far right column of the page types listing you can click on Pages using Example page and spot-check that these pages match what is on production

@johnnyporkchops johnnyporkchops changed the title New ExamplePage template [Do Not Merge]New ExamplePage template Oct 28, 2019
@codecov-io
Copy link

codecov-io commented Oct 29, 2019

Codecov Report

Merging #3303 into develop will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           develop   #3303   +/-   ##
=======================================
  Coverage     74.6%   74.6%           
=======================================
  Files          120     120           
  Lines         7229    7229           
  Branches       633     633           
=======================================
  Hits          5393    5393           
  Misses        1836    1836
Impacted Files Coverage Δ
fec/home/models.py 87.53% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c713a19...9da5cb9. Read the comment docs.

@johnnyporkchops johnnyporkchops changed the title [Do Not Merge]New ExamplePage template New options to Reporting Example Page template for disclosure examples Oct 29, 2019
Copy link
Contributor

@dorothyyeager dorothyyeager left a comment

Choose a reason for hiding this comment

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

Hi @johnnyporkchops - now that I've looked at it, is there anyway we can rename it from "reporting example page" to "example page" - The pre-titles work well, so I'm just talking about the name of the template itself. It's confusing when it can be used for other things.

Also, we need the ability to embed a video in the body part, so please add that function as well.

Thank you!

@johnnyporkchops
Copy link
Contributor Author

@dyeager you can add vide embed code via the rich text box paragraph or html called html

@johnnyporkchops
Copy link
Contributor Author

@dorothyyeager , With the name change, we would no longer have ReportingExamplePage, just ExampePage template ? Is that what you mean?

@patphongs patphongs requested review from rfultz and removed request for patphongs October 31, 2019 13:23
Copy link
Contributor

@rfultz rfultz left a comment

Choose a reason for hiding this comment

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

I'm not sure I'm testing it fully but, following the How to Test directions, it's working well for me!

@johnnyporkchops johnnyporkchops dismissed dorothyyeager’s stale review November 1, 2019 06:03

This is not on feature space so cannot be tested there

@johnnyporkchops
Copy link
Contributor Author

johnnyporkchops commented Nov 2, 2019

@dorothyyeager , The page has been renamed and new features added, so I re-requested your review. This IS on feature-space for testing

@johnnyporkchops johnnyporkchops changed the title New options to Reporting Example Page template for disclosure examples Rename ReportingExamplePage to Example Page, add features Nov 2, 2019
Copy link
Contributor

@dorothyyeager dorothyyeager left a comment

Choose a reason for hiding this comment

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

This is fantastic, @johnnyporkchops. Works perfectly. Thank you so much! Approved from content team perspective.

@johnnyporkchops johnnyporkchops removed the request for review from patphongs November 4, 2019 16:23
@dorothyyeager
Copy link
Contributor

Merging since @rfultz advises this is good to go.

@dorothyyeager dorothyyeager merged commit e8caf98 into develop Nov 4, 2019
@dorothyyeager dorothyyeager deleted the feature/3149-ExamplePage-template-wagtail branch November 4, 2019 18:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create Example Page template in Wagtail
4 participants