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

[Test failure] <48: <LMS: COURSE HOME: DISCUSSION> #326

Closed
ghassanmas opened this issue Nov 13, 2023 · 9 comments
Closed

[Test failure] <48: <LMS: COURSE HOME: DISCUSSION> #326

ghassanmas opened this issue Nov 13, 2023 · 9 comments
Labels
release blocker Blocks the upcoming release (fix needed)
Milestone

Comments

@ghassanmas
Copy link
Member

Release

quince

Expected behavior

To be see disucssion forum and to post:

Actual behavior

Blank page with error in consle log as shown in the picture

image

Steps to reproduce

Go to any course and then click on the discussions tab

Additional information

This is probably related to the discussions MFE.

@ormsbee
Copy link

ormsbee commented Nov 27, 2023

Just as a heads up that forum behavior has changed since the last release, and that new courses are supposed to default to a mode where you can mark units as discussable via a checkbox in its access controls:

https://openedx.atlassian.net/wiki/spaces/COMM/pages/3470655498/Discussions+upgrade+Sidebar+and+new+topic+structure#How-do-I-enable-discussions-sidebar?

I say "supposed to" because it's not currently working for me on tutor nightly. FYI @ayub02

@ayub02
Copy link

ayub02 commented Nov 29, 2023

I was under the assumption that the new discussion provider (sidebar) is behind a waffle flag discussions.enable_new_structure_discussions. @ghassanmas can you please confirm if this flag has been enabled for the courses or the environment that you are working with?
cc: @asadazam93 @AhtishamShahid

@awais-ansari
Copy link

awais-ansari commented Nov 29, 2023

There is only one reason for this issue to be an invalid URL.

  • A Valid URL: ${BASE_URL}${getConfig().PUBLIC_PATH}:courseId/posts OR https://discussions.stage.edx.org/course-v1:arbisoft_acca+cs1023+2021_T4/posts
  • A Invalid URL: ${BASE_URL}null/:courseId/posts OR course-v1:arbisoft_acca+cs1023+2021_T4/home OR https://discussions.stage.edx.orgundefined/course-v1:arbisoft_acca+cs1023+2021_T4/posts

Valid Example
Four parts are required to build a valid URL.

  1. BASE_URL: https://discussions.stage.edx.org
  2. PUBLIC_PATH: / OR /any-slug/
  3. courseId: course-v1:arbisoft_acca+cs1023+2021_T4
  4. tab: posts

https://discussions.stage.edx.org/course-v1:arbisoft_acca+cs1023+2021_T4/posts

Note:
The above issue can be raised because of an invalid URL or undefined PUBLIC_PATH. It is common practice in openEdx community to define custom PUBLIC_PATH for MFEs. We are following the same pattern to make it easy for openEdx community. PUBLIC_PATH configure in frontend-platform and its default value is /.

@awais-ansari
Copy link

I was debugging this issue in the tutor Quince Branch Demo. The issue is with PUBLIC_PATH. The below images are from Quince Demo. It is creating invalid URLs because of an invalid PUBLIC_PATH.

  • Tutor PUBLIC_PATH: /discussions
  • Tutor BASE_PATH: /discussions:courseId
Screenshot 2023-11-30 at 12 49 45 PM Screenshot 2023-11-30 at 1 06 12 PM

It Should be

  • PUBLIC_PATH: /discussions/
  • BASE_PATH: /discussions/:courseId

@xitij2000
Copy link

I think openedx/frontend-app-discussions#613 should fix the issue.

@mariajgrimaldi mariajgrimaldi added release blocker Blocks the upcoming release (fix needed) and removed needs triage labels Dec 5, 2023
@mariajgrimaldi mariajgrimaldi moved this from Backlog to In progress in Build-Test-Release Working Group Dec 5, 2023
@mariajgrimaldi
Copy link
Member

@xitij2000: thanks! Please let us know when the PR gets merged so we can open a backport for Quince.

@mariajgrimaldi
Copy link
Member

I added the release blocker label, although the discussions MFE can be disabled, so the platform could still work without it. I'll leave it as is to focus our attention on it.

@awais-ansari
Copy link

@mariajgrimaldi This issue has been resolved and merged in master and Quince too.

@mariajgrimaldi
Copy link
Member

I'll be closing this then. Thanks @awais-ansari!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release blocker Blocks the upcoming release (fix needed)
Projects
Development

No branches or pull requests

6 participants