fix: [FC-0044] Course unit - Replaced the LMS endpoints #882

PKulkoRaccoonGang · 2024-03-10T11:23:14Z

Settings

EDX_PLATFORM_REPOSITORY: https://github.com/raccoongang/edx-platform.git
EDX_PLATFORM_VERSION: ts-develop

TUTOR_GROVE_WAFFLE_FLAGS:
  - name: contentstore.new_studio_mfe.use_new_unit_page
    everyone: true

TUTOR_GROVE_MFE_LMS_COMMON_SETTINGS:
  MFE_CONFIG:
    ENABLE_UNIT_PAGE: true

Description

This PR is the final stage in the process of replacing the LMS of endpoints for navigation on the units of the course.

The primary features were implemented:

Code refactoring and replacing LMS endpoints with Studio endpoints.

Issue: openedx/platform-roadmap#321

Developer notes

To view the units of the course, now it is not necessary to publish sections, etc.

Testing instructions

Run master devstack.
Start platform make dev.up.lms+cms+frontend-app-course-authoring and make checkout on this branch.
Enable the new Unit page by adding a waffle flag contentstore.new_studio_mfe.use_new_unit_page in the CMS admin panel.
Make sure that the MFE setting ENABLE_UNIT_PAGE=true is enabled.
Go to the Course Unit page from the Course Outline page.

Screen.Recording.2024-03-10.at.17.21.22.mov

openedx-webhooks · 2024-03-10T11:23:19Z

Thanks for the pull request, @PKulkoRaccoonGang! Please note that it may take us up to several weeks or months to complete a review and merge your PR.

Feel free to add as much of the following information to the ticket as you can:

supporting documentation
Open edX discussion forum threads
timeline information ("this must be merged by XX date", and why that is)
partner information ("this is a course on edx.org")
any other information that can help Product understand the context for the PR

All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here.

Please let us know once your PR is ready for our review and all tests are green.

codecov · 2024-03-10T11:31:58Z

Codecov Report

Attention: Patch coverage is 90.90909% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 91.19%. Comparing base (17b1360) to head (e396b1c).

Files	Patch %	Lines
src/course-unit/CourseUnit.jsx	50.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #882      +/-   ##
==========================================
+ Coverage   90.64%   91.19%   +0.54%     
==========================================
  Files         578      578              
  Lines       10037     9913     -124     
  Branches     2142     2122      -20     
==========================================
- Hits         9098     9040      -58     
+ Misses        899      839      -60     
+ Partials       40       34       -6

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

open-craft-grove · 2024-03-10T13:35:29Z

Sandbox deployment successful 🚀
🎓 LMS
📝 Studio
ℹ️ Grove Config, Tutor Config, Tutor Requirements

fix: [AXIMST-424] Course unit - Fixed network connection behavior (#138) * fix: [AXIMST-424] fixed network connetcion behavior * fix: added placeholder for unsuccessful loading for the page * refactor: code refactoring

open-craft-grove · 2024-03-10T16:34:21Z

Sandbox deployment successful 🚀
🎓 LMS
📝 Studio
ℹ️ Grove Config, Tutor Config, Tutor Requirements

open-craft-grove · 2024-03-10T17:06:47Z

Sandbox deployment successful 🚀
🎓 LMS
📝 Studio
ℹ️ Grove Config, Tutor Config, Tutor Requirements

open-craft-grove · 2024-03-11T09:36:25Z

Sandbox deployment successful 🚀
🎓 LMS
📝 Studio
ℹ️ Grove Config, Tutor Config, Tutor Requirements

bradenmacdonald · 2024-03-11T19:43:11Z

This PR seems to depend on openedx/edx-platform#34055 and openedx/edx-platform#34337 , so I don't think it should be merged until those ones are merged.

Also, the error handling is not great; when I didn't have those PRs checked out, I just see something like this:

Instead of "Error loading navigation" or "Error loading components" or something.

arbrandes

@PKulkoRaccoonGang, in the future, please make an explicit note in the description when the PR depends on another one being merged first. That way we avoid merging broken stuff accidentally.

It might also make sense to always have such PRs be drafts. Let's discuss this further.

Also, what @bradenmacdonald said. We probably need to improve the error handling.

PKulkoRaccoonGang · 2024-03-12T20:11:03Z

@arbrandes The current PR is independent and does not depend on other open PRs.
In the new PR that was recently opened, I included important information about PR dependencies as a separate message.

PKulkoRaccoonGang · 2024-03-13T09:45:07Z

@arbrandes @bradenmacdonald thanks 👍

error handling is not great

Yes, it would be great to handle errors that may occur and show a user-friendly message. We discussed this with the team today and propose to do this later in a separate PR.

arbrandes

I love PRs that remove more code than they add. Nice, approved! I do have one question below, though it's not blocking.

arbrandes · 2024-03-13T13:53:36Z

src/course-unit/data/utils.js

-    courseExitPageIsActive: data.course_exit_page_is_active,
-    certificateData: camelCaseObject(data.certificate_data),
-    entranceExamData: camelCaseObject(data.entrance_exam_data),
-    timeOffsetMillis: getTimeOffsetMillis(headers && headers.date, requestTime, responseTime),


Just curious: what did we need this for, before?

Previously, we transferred functionality from MFE Learning, so the data received from the LMS endpoint is formatted there. We decided not to refactor the code that was awaiting replacement and after switching to Studio endpoints, delete it.

openedx-webhooks · 2024-03-13T13:56:14Z

@PKulkoRaccoonGang 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future.

PKulkoRaccoonGang requested a review from a team as a code owner March 10, 2024 11:23

openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Mar 10, 2024

PKulkoRaccoonGang changed the title ~~fix: [AXIMST-416] replaced the LMS endpoint for navigating the course…~~ fix: [AXIMST-416] Course unit - Replaced the LMS endpoints Mar 10, 2024

PKulkoRaccoonGang marked this pull request as draft March 10, 2024 11:23

PKulkoRaccoonGang added the create-sandbox open-craft-grove should create a sandbox environment from this PR label Mar 10, 2024

PKulkoRaccoonGang changed the title ~~fix: [AXIMST-416] Course unit - Replaced the LMS endpoints~~ fix: [FC-0044] Course unit - Replaced the LMS endpoints Mar 10, 2024

PKulkoRaccoonGang self-assigned this Mar 10, 2024

PKulkoRaccoonGang mentioned this pull request Mar 10, 2024

feat: [FC-0044] Course unit - Copy/paste functionality #884

Merged

PKulkoRaccoonGang marked this pull request as ready for review March 10, 2024 15:23

fix: replaced the LMS endpoint for navigating the course unit page

e396b1c

fix: [AXIMST-424] Course unit - Fixed network connection behavior (#138) * fix: [AXIMST-424] fixed network connetcion behavior * fix: added placeholder for unsuccessful loading for the page * refactor: code refactoring

PKulkoRaccoonGang force-pushed the Peter_Kulko/replacing-endpoints-from-LMS-to-CMS branch from 713ca4b to e396b1c Compare March 10, 2024 15:37

PKulkoRaccoonGang added create-sandbox open-craft-grove should create a sandbox environment from this PR and removed create-sandbox open-craft-grove should create a sandbox environment from this PR labels Mar 10, 2024

PKulkoRaccoonGang added create-sandbox open-craft-grove should create a sandbox environment from this PR and removed create-sandbox open-craft-grove should create a sandbox environment from this PR labels Mar 11, 2024

PKulkoRaccoonGang requested review from arbrandes and KristinAoki March 11, 2024 08:55

KristinAoki approved these changes Mar 11, 2024

View reviewed changes

arbrandes requested changes Mar 12, 2024

View reviewed changes

arbrandes approved these changes Mar 13, 2024

View reviewed changes

arbrandes merged commit 8acd27d into openedx:master Mar 13, 2024
6 checks passed

arbrandes mentioned this pull request Mar 19, 2024

New Unit Page (Studio MFE conversion) openedx/platform-roadmap#321

Open

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: [FC-0044] Course unit - Replaced the LMS endpoints #882

fix: [FC-0044] Course unit - Replaced the LMS endpoints #882

PKulkoRaccoonGang commented Mar 10, 2024 •

edited

Loading

openedx-webhooks commented Mar 10, 2024 •

edited

Loading

codecov bot commented Mar 10, 2024 •

edited

Loading

open-craft-grove commented Mar 10, 2024

open-craft-grove commented Mar 10, 2024

open-craft-grove commented Mar 10, 2024

open-craft-grove commented Mar 11, 2024

bradenmacdonald commented Mar 11, 2024

arbrandes left a comment

PKulkoRaccoonGang commented Mar 12, 2024 •

edited

Loading

PKulkoRaccoonGang commented Mar 13, 2024

arbrandes left a comment •

edited

Loading

arbrandes Mar 13, 2024

PKulkoRaccoonGang Mar 13, 2024

openedx-webhooks commented Mar 13, 2024

fix: [FC-0044] Course unit - Replaced the LMS endpoints #882

fix: [FC-0044] Course unit - Replaced the LMS endpoints #882

Conversation

PKulkoRaccoonGang commented Mar 10, 2024 • edited Loading

Description

Developer notes

Testing instructions

openedx-webhooks commented Mar 10, 2024 • edited Loading

codecov bot commented Mar 10, 2024 • edited Loading

Codecov Report

open-craft-grove commented Mar 10, 2024

open-craft-grove commented Mar 10, 2024

open-craft-grove commented Mar 10, 2024

open-craft-grove commented Mar 11, 2024

bradenmacdonald commented Mar 11, 2024

arbrandes left a comment

Choose a reason for hiding this comment

PKulkoRaccoonGang commented Mar 12, 2024 • edited Loading

PKulkoRaccoonGang commented Mar 13, 2024

arbrandes left a comment • edited Loading

Choose a reason for hiding this comment

arbrandes Mar 13, 2024

Choose a reason for hiding this comment

PKulkoRaccoonGang Mar 13, 2024

Choose a reason for hiding this comment

openedx-webhooks commented Mar 13, 2024

PKulkoRaccoonGang commented Mar 10, 2024 •

edited

Loading

openedx-webhooks commented Mar 10, 2024 •

edited

Loading

codecov bot commented Mar 10, 2024 •

edited

Loading

PKulkoRaccoonGang commented Mar 12, 2024 •

edited

Loading

arbrandes left a comment •

edited

Loading