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

[TC_LEARNER_41] > date and course outline pages are blank #222

Closed
2 tasks done
Tracked by #67
fayyazahmed66 opened this issue Oct 27, 2022 · 11 comments
Closed
2 tasks done
Tracked by #67

[TC_LEARNER_41] > date and course outline pages are blank #222

fayyazahmed66 opened this issue Oct 27, 2022 · 11 comments
Assignees
Labels
bug Report of or fix for something that isn't working as intended release blocker Blocks the upcoming release (fix needed) release testing Affects the upcoming release (attention needed)
Milestone

Comments

@fayyazahmed66
Copy link

fayyazahmed66 commented Oct 27, 2022

Steps to reproduce:

  1. Go to the progress page on any course in LMS
  2. check the date and course outline pages

Expected result:
Pages should have some data in them.
Actual result:
Pages are blank
progress page

PRs

@fayyazahmed66
Copy link
Author

label: olive testing

@github-actions github-actions bot added the release testing Affects the upcoming release (attention needed) label Oct 27, 2022
@mariajgrimaldi
Copy link
Member

mariajgrimaldi commented Nov 15, 2022

I believe this issue is related to the learning MFE, since the path should be:
/learning/course/course-v1:edX+DemoX+Demo_Course/home -this path works-
Instead of just:
/course/course-v1:edX+DemoX+Demo_Course/home -this path doesn't-
image
I don't know why this is behaving like that or if that is even the problem. Since I'm unfamiliar with MFEs, I'll tag some folks who are.
cc @arbrandes @kdmccormick

@kdmccormick
Copy link
Member

@mariajgrimaldi hmmm, interesting. The path /course/course-v1:edX+DemoX+Demo_Course/home actually looks correct; it's just that the path is meant to be relative to the root of the learning mfe, which in our case is https://apps.olive.overhang.io/learning. The learning mfe makes this same assumption in many other places.

So, I would expect some other step in Tutor's build pipeline to account for this by to prepending /learning/ to the URL. I don't know exactly why that's not working here. If I had to guess, I'd bet it's a bug in the <Hyperlink> component from Paragon (or a bug in how we handle it), since this is the only place in the learning mfe where we use <Hyperlink> for an absolute path to the learning mfe.

@mariajgrimaldi
Copy link
Member

Thanks, @kdmccormick, for the detailed explanation!

@arbrandes arbrandes added the release blocker Blocks the upcoming release (fix needed) label Nov 21, 2022
@ghassanmas ghassanmas self-assigned this Dec 6, 2022
ghassanmas added a commit to ghassanmas/frontend-app-learning that referenced this issue Dec 6, 2022
 This fix does fixes the url links by getting them from the state
 simliar to how tabs navigation gets them.

 This would allow it work for PUBLIC_PATH is not '/', i.e. in
 tutor.

  This was reported in openedx/wg-build-test-release/issues/222
@ghassanmas
Copy link
Member

@arbrandes @mariajgrimaldi following our yesterday meeting, this issue caught my attention, so while looking into I came out with a suggested fix here openedx/frontend-app-learning/pull/1009. The way my fix work is by getting urls of the tabs from the state, simlair to how TabNavigation gets them, which orginally gets them from coursemetadata API.

ghassanmas added a commit to ghassanmas/frontend-app-learning that referenced this issue Dec 6, 2022
 This fix does fixes the url links by getting them from the state
 simliar to how tabs navigation gets them.

 This would allow it work for PUBLIC_PATH is not '/', i.e. in
 tutor.

  This was reported in openedx/wg-build-test-release/issues/222
ghassanmas added a commit to ghassanmas/frontend-app-learning that referenced this issue Dec 6, 2022
 This fix does fixes the url links by getting them from the state
 simliar to how tabs navigation gets them.

 This would allow it work for PUBLIC_PATH is not '/', i.e. in
 tutor.

  This was reported in openedx/wg-build-test-release/issues/222
ghassanmas added a commit to ghassanmas/frontend-app-learning that referenced this issue Dec 6, 2022
 This fix does fixes the url links by getting them from the state
 simliar to how tabs navigation gets them.

 This would allow it work for PUBLIC_PATH is not '/', i.e. in
 tutor.

  This was reported in openedx/wg-build-test-release/issues/222
ghassanmas added a commit to ghassanmas/frontend-app-learning that referenced this issue Dec 6, 2022
 This fix does fixes the url links by getting them from the state
 simliar to how tabs navigation gets them.

 This would allow it work for PUBLIC_PATH is not '/', i.e. in
 tutor.

  This was reported in openedx/wg-build-test-release/issues/222
@mariajgrimaldi
Copy link
Member

Thank you @ghassanmas! I left you a comment in the PR :)

@ghassanmas
Copy link
Member

ghassanmas commented Dec 6, 2022

[Edit] Sorry I meant to reply on the PR, but got overwhelmed by github notifcation 🙈

I tested this on production with tutor on my demo instance at olive.zaat.dev and it worked as expected, but in anycase, to debug this could you when loading the learning MFE check the network logs in for the result of this call:

https://LMS_BASE_URL/api/course_home/course_metadata/course-id

For example in my demo instance I for

https://olive.zaat.dev/api/course_home/course_metadata/course-v1:zaatdev+101+1

I get this

{
    "can_show_upgrade_sock": false,
    "verified_mode": null,
    "celebrations": {
        "first_section": false,
        "streak_length_to_celebrate": null,
        "streak_discount_enabled": false,
        "weekly_goal": false
    },
    "course_access": {
        "has_access": true,
        "error_code": null,
        "developer_message": null,
        "user_message": null,
        "additional_context_user_message": null,
        "user_fragment": null
    },
    "course_id": "course-v1:zaatdev+101+1",
    "is_enrolled": true,
    "is_self_paced": true,
    "is_staff": true,
    "number": "101",
    "org": "zaatdev",
    "original_user_is_staff": true,
    "start": "2022-12-05T00:00:00Z",
    "tabs": [
        {
            "tab_id": "courseware",
            "title": "Course",
            "url": "https://apps.olive.zaat.dev/learning/course/course-v1:zaatdev+101+1/home"
        },
        {
            "tab_id": "progress",
            "title": "Progress",
            "url": "https://apps.olive.zaat.dev/learning/course/course-v1:zaatdev+101+1/progress"
        },
        {
            "tab_id": "dates",
            "title": "Dates",
            "url": "https://apps.olive.zaat.dev/learning/course/course-v1:zaatdev+101+1/dates"
        },
        {
            "tab_id": "wiki",
            "title": "Wiki",
            "url": "https://olive.zaat.dev/courses/course-v1:zaatdev+101+1/course_wiki"
        },
        {
            "tab_id": "instructor",
            "title": "Instructor",
            "url": "https://olive.zaat.dev/courses/course-v1:zaatdev+101+1/instructor"
        }
    ],
    "title": "Olive Demo",
    "username": "admin",
    "user_timezone": null,
    "can_view_certificate": true
}

@mariajgrimaldi
Copy link
Member

I'm not sure what's wrong with my setup then. Either way, can you add an image similar to this one? So we have a record that it's working. Thanks again!

@ghassanmas
Copy link
Member

I am not very good with photo editing, but here is an attempt screenshot when I hoverovered the dates in progress
image

You can try it with https://olive.zaat.dev email: [email protected] password: admin

@arbrandes arbrandes moved this from 💡 Incoming to 🏗 In Progress in Build-Test-Release Working Group Dec 6, 2022
@arbrandes arbrandes moved this to In progress in Frontend Working Group Dec 6, 2022
@mariajgrimaldi
Copy link
Member

@ghassanmas thank you! ❤️

@arbrandes arbrandes added this to the Olive.1 milestone Dec 6, 2022
@arbrandes arbrandes added the bug Report of or fix for something that isn't working as intended label Dec 6, 2022
@arbrandes arbrandes removed their assignment Dec 6, 2022
@arbrandes arbrandes moved this from In progress to In review in Frontend Working Group Dec 6, 2022
@arbrandes arbrandes moved this from In progress to In review in Build-Test-Release Working Group Dec 6, 2022
ghassanmas added a commit to ghassanmas/frontend-app-learning that referenced this issue Dec 7, 2022
 This fix does fixes the url links by getting them from the state
 simliar to how tabs navigation gets them.

 This would allow it work for PUBLIC_PATH is not '/', i.e. in
 tutor.

  This was reported in openedx/wg-build-test-release/issues/222
ghassanmas added a commit to ghassanmas/frontend-app-learning that referenced this issue Dec 7, 2022
 This fix does fixes the url links by getting them from the state
 simliar to how tabs navigation gets them.

 This would allow it work for PUBLIC_PATH is not '/', i.e. in
 tutor.

  This was reported in openedx/wg-build-test-release/issues/222
ghassanmas added a commit to ghassanmas/frontend-app-learning that referenced this issue Dec 7, 2022
 This fix does fixes the url links by getting them from the state
 simliar to how tabs navigation gets them.

 This would allow it work for PUBLIC_PATH is not '/', i.e. in
 tutor.

  This was reported in openedx/wg-build-test-release/issues/222
ghassanmas added a commit to ghassanmas/frontend-app-learning that referenced this issue Dec 7, 2022
 This fix does fixes the url links by getting them from the state
 simliar to how tabs navigation gets them.

 This would allow it work for PUBLIC_PATH is not '/', i.e. in
 tutor.

  This was reported in openedx/wg-build-test-release/issues/222
ghassanmas added a commit to ghassanmas/frontend-app-learning that referenced this issue Dec 7, 2022
 This fix does fixes the url links by getting them from the state
 simliar to how tabs navigation gets them.

 This would allow it work for PUBLIC_PATH is not '/', i.e. in
 tutor.

  This was reported in openedx/wg-build-test-release/issues/222
ghassanmas added a commit to ghassanmas/frontend-app-learning that referenced this issue Dec 7, 2022
 This fix does fixes the url links by getting them from the state
 simliar to how tabs navigation gets them.

 This would allow it work for PUBLIC_PATH is not '/', i.e. in
 tutor.

  This was reported in openedx/wg-build-test-release/issues/222
ghassanmas added a commit to ghassanmas/frontend-app-learning that referenced this issue Dec 7, 2022
 This fix does fixes the url links by getting them from the state
 simliar to how tabs navigation gets them.

 This would allow it work for PUBLIC_PATH is not '/', i.e. in
 tutor.

  This was reported in openedx/wg-build-test-release/issues/222
arbrandes pushed a commit to openedx/frontend-app-learning that referenced this issue Dec 7, 2022
 This fix does fixes the url links by getting them from the state
 simliar to how tabs navigation gets them.

 This would allow it work for PUBLIC_PATH is not '/', i.e. in
 tutor.

  This was reported in openedx/wg-build-test-release/issues/222
ghassanmas added a commit to ghassanmas/frontend-app-learning that referenced this issue Dec 7, 2022
 This fix does fixes the url links by getting them from the state
 simliar to how tabs navigation gets them.

 This would allow it work for PUBLIC_PATH is not '/', i.e. in
 tutor.

  This was reported in openedx/wg-build-test-release/issues/222
arbrandes pushed a commit to openedx/frontend-app-learning that referenced this issue Dec 7, 2022
 This fix does fixes the url links by getting them from the state
 simliar to how tabs navigation gets them.

 This would allow it work for PUBLIC_PATH is not '/', i.e. in
 tutor.

  This was reported in openedx/wg-build-test-release/issues/222
@arbrandes
Copy link
Contributor

Fixed! Thanks, @ghassanmas!

Repository owner moved this from In review to In progress in Frontend Working Group Dec 7, 2022
Repository owner moved this from In review to Done in Build-Test-Release Working Group Dec 7, 2022
@arbrandes arbrandes moved this from In progress to Closed in Frontend Working Group Dec 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Report of or fix for something that isn't working as intended release blocker Blocks the upcoming release (fix needed) release testing Affects the upcoming release (attention needed)
Projects
Status: Closed
Development

Successfully merging a pull request may close this issue.

5 participants