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

Migrate instructor data to individual selectable pages #1786

Merged
merged 13 commits into from
Jul 31, 2023

Conversation

jkachel
Copy link
Contributor

@jkachel jkachel commented Jul 27, 2023

What are the relevant tickets?

Closes mitodl/hq#1543
#1773

Description (What does it do?)

Converts the current method of storing resource page instructor data (a stream block within the page marked Faculty Members) to a new method that uses individual pages for the instructors that can be linked to the resource page instead. Also contains some helper stuff to convert the data, so the existing data is not lost.

Instructor pages

These are new and are designed to be located underneath an Instructor index page in Wagtail. The configure_wagtail command has been updated to create this index page if it doesn't exist.

Otherwise, instructor pages are normal Wagtail CMS pages. Some notes on fields:

  • Title and Name are separate - in the case of duplicates, titles can be different to allow instructor pages to be more easily found in the CMS while the instructor's name remains the same. (This is especially important for the convert_faculty_to_pages command - see below.)
  • Name is what's used in the card on the page.
  • There is a new Title field, this is displayed as a <h4> if present in the card.
  • The short bio is analogous to the old description field.
  • The long bio is not used currently.

convert_faculty_to_pages

This is a helper command that will load the data that's in the Faculty Members field in existing course pages, creates new Instructor Page instances for them, and then links them to the course page. It does not operate on Program pages as we're not using them now.

An Instructor Page is created for the faculty member listings regardless of whether or not the instructor already exists in the system so we preserve whatever distinct imagery and copy is there from that particular page. The output of the command should tell you the title of the page that was created for the particular course page so you can find it later. These can be coalesced into a single instructor page at your leisure later.

As part of the conversion process, the Faculty Members section is cleared for the page it's processing so duplicate cards aren't shown.

If there isn't an index page, the command will make one.

Template changes

The product_page.html template is now set up to display the Meet your instructors section if there's linked Instructor Pages or old-style Faculty Members. Instructor pages will be displayed first.

Instructor Pages have an additional Title field that wasn't in the old Faculty Members section - this is displayed as an h4 under the name if there is one.

No other changes for the cards - they should look the same as they did before.

How can this be tested?

  1. Create a course page with some faculty members in it.
  2. Run convert_faculty_to_pages. This should convert the faculty member listings to pages, and apply them back to the course page.
  3. View the course page. It should not be visibly different.
  4. View the course page in the CMS. You should see the linked Instructor Pages and Faculty Members should be empty.
  5. Explore the CMS - you should see an Instructors section with the new pages underneath. They should be Drafts.

- Finalizes instructor page models and link to product page
- Adds support into the template for this (alongside the in-page faculty members section)
- Started in on a migration to backfill instructor pages from existing faculty member sections; not done yet
…ing root page if it doesn't exist in convert management command
Otherwise converting with duplicates (which there are guaranteed to be) will result in people's cards having (3) (4), etc. appended to their names. (The titles don't get used int he card, just the name.)
@pdpinch
Copy link
Member

pdpinch commented Jul 28, 2023

Does this work with the existing design, the upcoming redesign, or both?

How will we transition from the current system to the new one? What should I be telling the MITx folks who manage the about pages?

@jkachel
Copy link
Contributor Author

jkachel commented Jul 28, 2023

Does this work with the existing design, the upcoming redesign, or both?

Both!

How will we transition from the current system to the new one? What should I be telling the MITx folks who manage the about pages?

There's a command to facilitate the migration process that will need to be run manually. The command scrapes the instructor data off the existing course pages, puts them into new instructor pages, links them to the course page it was working on, and then clears the Faculty Members section on the page; the end result is that the faculty members all get moved to instructor pages and there's no duplication of cards on the course pages.

The timeline for migration looks like this:

  • Until the command is run: everything works as it does normally, except there's a new section in the CMS. The template still displays the people in the Faculty Members section so there's no change to the learner, and the MITx folks can ignore the new section in the CMS.
  • When we perform the migration (immediately after running the command): All the data in each Faculty Members section will be translated into an Instructor Page. The command doesn't make any effort to dedupe this so there will likely be a bunch of similar pages (we can use the output from the command to determine which one goes to which page); it'd be best to clean that up some. But, immediately after running the migration command, the course about pages should look exactly as they did.
  • After the migration: I'll need to put up a new PR to eliminate the old Faculty Members section (and move the title for the section; it's out of order now), and they'll just create and link in instructor pages as necessary.

@collinpreston collinpreston self-requested a review July 28, 2023 17:10
Copy link
Contributor

@collinpreston collinpreston left a comment

Choose a reason for hiding this comment

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

Looks good and works for me.

I noticed that a faculty page that is in "draft" status will still display if added to a Course about page.

I am also able to create an Instructor Page in the Courses "folder" in the CMS.

cms/api.py Outdated
@@ -212,6 +210,30 @@ def ensure_certificate_index() -> cms_models.CertificateIndexPage:
return certificate_index


def ensure_instructors_index() -> cms_models.InstructorIndexPage:
"""
Ensures that an index page has been created for instructorns.
Copy link
Contributor

Choose a reason for hiding this comment

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

"instructorns" -> "instructors"

)

parser.add_argument(
"--live", action="store_true", help="Make the page live. (Defaults to not.)"
Copy link
Contributor

Choose a reason for hiding this comment

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

Defaults to draft or not live?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Made the flag and text more understandable (Wagtail calls it Draft/Published in the UI but the DB table calls it "live"). df1ea62 also adds some code to disallow serving of InstructorPages specifically - we don't want them exposed right now (but we might later and I repurposed other code for this so it's in there from that).

@@ -472,6 +473,112 @@ def create_country_field(self, field, options):
return ChoiceField(**options)


class InstructorPage(Page):
"""
Detail page for instructors.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you can add in something like:

subpage_types = []

Which will not allow child pages to be created for an InstructorPage.

Copy link
Contributor

@collinpreston collinpreston left a comment

Choose a reason for hiding this comment

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

Looks good to me!

InstructorIndexPage,
InstructorPage,
InstructorPageLink,
ProgramPage,
Copy link
Contributor

Choose a reason for hiding this comment

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

This import can be removed.

@jkachel jkachel merged commit 58659cb into main Jul 31, 2023
3 checks passed
@jkachel jkachel deleted the jkachel/1773-instructor-pages branch July 31, 2023 14:22
@odlbot odlbot mentioned this pull request Jul 31, 2023
4 tasks
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.

3 participants