Integrated Wagtail to support course/program detail pages

[gsidebo]

Pre-Flight checklist

• Screenshots and design review for any changes that affect layout or styling
  • Desktop screenshots
• Migrations
  • Migration is backwards-compatible with current production code
• Testing
  • Code is tested
  • Changes have been manually tested

What are the relevant tickets?

Closes #155

What's this PR do?

Properly integrates Wagtail so that Wagtail serves course/program detail pages

How should this be manually tested?

• Run migrations
• Check that the CMS has the correct page hierarchy and that "View Live" links for detail pages work correctly
• Unapply migrations and check the CMS still looks correct (i.e.: no course/program index pages, detail pages are once again children of the top level page)

Any background context you want to provide?

• cms/templates/detail_page.html and the partial templates in the same directory are basically copies of the same templates that were deleted in the courses app
• I added logic to exclude Courses/Programs from the catalog page that don't have an associated Wagtail page. This is something I discussed in person with Ferdi and it makes sense as a requirement

Screenshots (if appropriate)

[gsidebo]

@asadiqbal08 @ahmed-arbisoft This PR would affect at least 2 of your open PRs (#221 and #237). This PR is not 100% complete (I still want to add some query optimization and clean up some deprecated code) but it would be a good idea for you to look at it now. Right now I'm thinking that the best approach would be to merge those 2 PRs you have open, then reimplement both of them in this PR using this same pattern. Any future PRs involving a new program/course detail page subsection should be based on this branch. Let me know what you think

EDIT: Actually it looks like 3 PRs you have open (add #192 to the list)

[asadiqbal08]

@gsidebo I believe, these 3 PR's will be merged soon after the fix.

[gsidebo]

@asadiqbal08 Sounds good. Please just make sure that if you work on any new product page PRs, you base your work on this branch

[asadiqbal08]

@ahmed-arbisoft FYI over it .

[asadiqbal08]

@gsidebo Just want to know so are we going to serve Home Page from wagtail now ?

[gsidebo]

@asadiqbal08 That home page should be served from Wagtail, yes. Since that isn't implemented yet, I think I'm going to open a small PR to have the root route (/) serve the course catalog

[ahmed-arbisoft]

@gsidebo during development of the home page I figured out that we should let wagtail handle the routing at least for the course/program detail pages. In order for that to work we don't really need to anything apart from using the wagtail pageurl tag to create links and have the course and program page classes use the product_page.html template. In this template we are simply going to move everything from courses/templates/course_detail.html and rather than going through the Course/Program objects for data, we'll access through their corresponding CoursePage/ProgramPage. What's nice about this is that we don't have to worry about fetching the actual page from the database or do any kind of routing on our own. All the "view live" links will also be functional from the CMS this way.

We are already incrementally changing our partials to use the CoursePage/ProgramPage as the source of truth rather than their corresponding Course/Program.

I did a simple PoC of this and was able to render the product_page.html with a proper hero section by simply moving the hero partial include over to the product_page.html template.

[gsidebo]

... @ahmed-arbisoft I hope I'm wrong, but I think you're describing exactly what this PR already does

[ahmed-arbisoft]

@gsidebo Nope, we're both correct 😆 just saying we don't need to remove the product_page.html template, it'll be simple enough to deal with both ProgramPage and CoursePage pretty easily. Some of the things can be further simplified, for example on the catalog page, that card is being rendered via courseware_object.page as the source of data which leads me to believe that we are still querying/fetching through our Course/Program whereas it would be more standard to have everything coming in directly through the pages.

To clarify, I'd rather have {% include "partials/catalog_card.html" with page=courseware_page %} completely eliminating the need for the courseware_object in the source query.

This is just an example of the change. Also, I wanted to let you know that if you could look into the recent PRs we are already doing most of these Course->CoursePage changes in the partials so you won't have to redo a lot of it once you rebase.

[gsidebo]

@ahmed-arbisoft In this PR the product_page.html template is simply replaced by the detail_page.html template. That template serves the same purpose

[ahmed-arbisoft]

Okay, that makes sense

…

On Sat, 25 May 2019, 12:27 am Gavin Sidebottom, ***@***.***> wrote: @ahmed-arbisoft <https://github.com/ahmed-arbisoft> In this PR the product_page.html template is simply replaced by the detail_page.html template. That template serves the same purpose — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#264?email_source=notifications&email_token=AKZ74EWWIBXLHPIS7GW4WBTPXA6SPA5CNFSM4HMRMQM2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODWGL3WY#issuecomment-495762907>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AKZ74ETY4S4LL3JC4Y3ZPR3PXA6SPANCNFSM4HMRMQMQ> .

-- The content of this email and attachment(s) are confidential and intended solely for the recipient(s) specified in the message. If you received this email by error, please alert the sender by replying to this email and follow with its deletion, so that we can ensure such an error does not occur in future. If you are not the intended recipient, you are strictly prohibited to use, copy, disseminate or store this email or its attachment(s).

[ahmed-arbisoft]

You might want to move the tree manipulation (adding/removing pages) outside a migration and into a management command. I tried the same earlier and faced a ton of issues, it's a running problem with Wagtail as of right now. Reference: wagtail/wagtail#742

[gsidebo]

Closing in favor of #395