Skip to content

Drop legacy outline tab in favor of the MFE's version#30064

Merged
mikix merged 1 commit intomasterfrom
mikix/drop-outline-view
Apr 14, 2022
Merged

Drop legacy outline tab in favor of the MFE's version#30064
mikix merged 1 commit intomasterfrom
mikix/drop-outline-view

Conversation

@mikix
Copy link
Copy Markdown
Contributor

@mikix mikix commented Mar 15, 2022

Dropped Features

  • The "course home" view (what we would call "Legacy Outline Tab")
  • The legacy implementation of Course Goals (the one that asked for intent rather than frequency - MFE only offers the frequency version)
  • Course outline in-course search (never enabled for learners on edx.org, MFE does not have course search)

Modified Endpoints

/courses/{key}/course: 👍 Primary purpose of this PR. Redirects to MFE. Fixes openedx/public-engineering#50

Dropped Endpoints

Fragments are a platform way to share snippets around. But these endpoints also provided a way to view just the fragment in isolation. My best guess is that they were for debugging or mobile, but most were never used in mobile.

/courses/{key}/course/home_fragment: Added in 2017, never used.
/courses/{key}/course/welcome_message_fragment: Added in 2017, never used.
/courses/{key}/course/latest_update_fragment: Added in 2017, never used.
/courses/{key}/course/updates_fragment: Added in 2017, never used.
/courses/{key}/course/outline_fragment: Added in 2017, never used.
/courses/{key}/course/mobile_dates_fragment: Unused since 2020 for android and iOS, replaced by native implementations. There are still active hits to this endpoint from old versions of the apps, but the mobile team tells me that we don't support these old versions and can drop the endpoint.

/courses/{key}/search: This is the course search result page, added in 2017, never enabled for learners on edx.org, and not a feature in the MFE.
/courses/{key}/search/home_fragment: Added in 2017, never used.

/courses/{key}/course/dismiss_welcome_message: Added in 2017, but the MFE uses /api/course_home/dismiss_welcome_message instead.

/api/course_goals/v0/course_goals/: This is the old version of course goals that was never properly productized. The MFE now reads goals from /api/course_home/v1/outline/{key} and updates them via /api/course_home/save_course_goal

Left-Alone Endpoints

Obviously, many endpoints. But these are the "nearby endpoints" that one might be surprised to see still surviving.

/courses/{key}/course/updates: Still used even in the MFE, linked from a 'course tool' in the sidebar that says "Updates"

/courses/{key}/info: This is an old precursor to the "course home" page. It should also be removed in favor of the MFE. But since it can be bitten off separately, we should. This current PR is already big enough. Dropping the info page has its own depr ticket. Enabled by the course_experience.disable_unified_course_tab waffle.

/courses/{key}/about: The "course about" page - similar in some ways to the course home landing page, but it's a feature for those sites without a marketing page. A little more marketing focused than the outline/home page. Enabled by ENABLE_MKTG_SITE=False and ENABLE_COURSE_HOME_REDIRECT=True feature toggles.

Dropped Feature Toggles

course_experience.latest_update: (Off for edx.org, with a few 2017 courses opting-in) Switches labeling of the welcome message to a "latest update" message. Not used for the MFE.
course_experience.show_upgrade_msg_on_course_home: (On for edx.org) Enabled an upgrade banner on outline tab. Not used for the MFE.
course_experience.upgrade_deadline_message: (Off for edx.org) Controlled an upgrade banner on the outline page.
course_home.course_home_use_legacy_frontend: (Off for edx.org) No longer needed. The only remaining tab using this toggle is the Progress tab, and it has its own toggle.

@mikix mikix force-pushed the mikix/drop-outline-view branch 2 times, most recently from 17ca942 to 82756da Compare March 21, 2022 20:34
@mikix mikix changed the title WIP: Drop outline tab Drop legacy outline tab in favor of the MFE's version Mar 21, 2022
@mikix mikix force-pushed the mikix/drop-outline-view branch 9 times, most recently from bd752a3 to 97011c8 Compare March 28, 2022 18:32
@mikix mikix marked this pull request as ready for review March 28, 2022 18:49
@cdeery
Copy link
Copy Markdown
Contributor

cdeery commented Mar 29, 2022

You also took out the dates page? I assume it was obsolete, but I expected to see a reference to it in the description

@mikix
Copy link
Copy Markdown
Contributor Author

mikix commented Mar 29, 2022

You also took out the dates page? I assume it was obsolete, but I expected to see a reference to it in the description

Oh right - this PR sits on top of #29846 and github is probably showing both diffs, since I targeted master. I could have and probably should have filed this PR with the dates PR as the target. Then you'd only see the delta between those.

Copy link
Copy Markdown
Contributor

@cdeery cdeery left a comment

Choose a reason for hiding this comment

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

Nice job. That's a lot to churn through. I appreciate the small refactors that you put in as well.
Just a handful of questions, mostly for my own understanding.

def get(self, request, *args, **kwargs): # pylint: disable=too-many-statements
course_key_string = kwargs.get('course_key_string')
course_key = CourseKey.from_string(course_key_string)
course_usage_key = modulestore().make_course_usage_key(course_key) # pylint: disable=unused-variable
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm surprised the linter would even let there be an unused variable in the code.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It normally wouldn't, but this line had pylint: disable=unused-variable at the end.

Copy link
Copy Markdown
Contributor

@cdeery cdeery 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. Thanks for all the cleanup.

@kdmccormick
Copy link
Copy Markdown
Member

@mikix This is related to openedx/public-engineering#50, right? Could you note that in the PR's description?

This was the "outline tab" view of the course. Preceded by the
course info view, succeeded by the MFE outline tab.

In addition to the course home view itself, this drops related
features:
- Legacy version of Course Goals (MFE has a newer implementation)
- Course home in-course search (MFE has no search)

The old course info view and course about views survive for now.

This also drops a few now-unused feature toggles:
- course_experience.latest_update
- course_experience.show_upgrade_msg_on_course_home
- course_experience.upgrade_deadline_message
- course_home.course_home_use_legacy_frontend

With this change, just the progress and courseware tabs are still
supported in legacy form, if you opt-in with waffle flags. The
outline and dates tabs are offered only by the MFE.

AA-798
@mikix mikix force-pushed the mikix/drop-outline-view branch from c0398ef to be5c1a6 Compare April 14, 2022 13:12
@mikix mikix merged commit 6af3cb1 into master Apr 14, 2022
@mikix mikix deleted the mikix/drop-outline-view branch April 14, 2022 15:59
@edx-pipeline-bot
Copy link
Copy Markdown
Contributor

EdX Release Notice: This PR has been deployed to the staging environment in preparation for a release to production.

@edx-pipeline-bot
Copy link
Copy Markdown
Contributor

EdX Release Notice: This PR may have caused e2e tests to fail on Stage. If you're a member of the edX org, please visit #e2e-troubleshooting on Slack to help diagnose the cause of these failures. Otherwise, it is the reviewer's responsibility. E2E tests have failed. https://gocd.tools.edx.org/go/tab/pipeline/history/deploy_to_stage

@mikix mikix restored the mikix/drop-outline-view branch April 14, 2022 19:17
@edx-pipeline-bot
Copy link
Copy Markdown
Contributor

EdX Release Notice: This PR has been deployed to the production environment.

@edx-pipeline-bot
Copy link
Copy Markdown
Contributor

EdX Release Notice: This PR has been rolled back from the production environment.

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.

[DEPR]: Course Home Outline and Dates tabs -> micro-frontend

4 participants