-
Notifications
You must be signed in to change notification settings - Fork 15
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
[ECOM > Create a course seat] > After creating course I am not able to open its detail page #301
Comments
I don't have access to an e-commerce installation, so I can't test this to confirm the issue. @sambapete can you lend us a hand here? Thanks! Also, if you think you know how to solve this @fayyazahmed66, open a PR! We would appreciate it. Not a lot of us have access to e-commerce installations. |
Thanks for the reminder @mariajgrimaldi. I had planned to look into it when @fayyazahmed66 raised the issue and try it on one of our systems but completely forgot about it after a few days. Right now, I can test it on a nutmeg.3 instance (a copy of our production instance) and a palm.2 instance (where we are testing our own fork). I will first try to reproduce the problem. |
@fayyazahmed66 I would need more details about what you were trying because I can definitely access the product details for the course I created. Question: are you using the Course Administration Tool to create your courses in ecommerce? For example, in my case it would be Here are some screenshots from a course I just created under palm.2 |
I also tried a course with a session ending with .9 instead of .09 and I had the same behavior. course-v1:UMontreal+Session101.2+2023.9 and course-v1:UMontreal+Session101+2023.09 |
I don't know how to solve it yet, but it is clear the problem would be in the ecommerce/ecommerce/core/constants.py file where COURSE_ID_REGEX is defined. it is currently I tried using it with https://pythex.org/ and it does match |
Based on the substrings I suspect that it should be COURSE_ID_REGEX = r'[^/+]+(/|+)[^/+]+(/|+)[^/+]' but I will try it first in my fork of ecommerce. |
After trying a few things, I discovered that the issue seems limited to courses where the sessions ends with a . (dot) followed with a number. I tried ending the session with a . (dot) by itself or followed by a letter and it worked. One thing I noticed is that when a seat is opened in order to edit it, it first calls When we encounter the problem, it never adds the / before the ?include_products=true argument If you add the / it will display the JSON data for this course, but not pretty printed like we expect to see it for a course where we can edit the data. I still haven't figured out why this is the case. |
Reproducing the issue is easy with Tutor:
|
The problem does not come from the COURSE_ID_REGEX. It comes from the Django REST framework. To troubleshoot I opened a shell in the ecommerce container:
Import Django url resolver:
Then, compare:
with:
Notice the The problem can be diagnosed more precisely with:
Notice the course-detail url pattern:
The "format" part from the regex matches ".09":
This "non-formatted" detail url does not match because the url does not include a trailing slash: This seems to be a related issue: https://stackoverflow.com/questions/27963899/django-rest-framework-using-dot-in-url We can resolve this issue by appending a trailing slash to the url. This url is created by backbone in "course_model.js". I managed to fix the issue with the following piece of code:
When we define the above "url" method, the course page loads fine. If this is a satisfactory fix then I can open a PR. |
These changes were implemented to research the following issue: openedx/wg-build-test-release#301
Thanks for the detailed explanation @regisb I agree that adding a trailing slash to the url does seem to solve the current issue and it's what I had noticed during my testing when I added a trailing slash to the url in order to see the JSON data. I would be fine with this PR. Thanks again for your help. |
fix course detail page url as after creating a course, its detail page was not accessible and showing nothing. The course page was unavailable when the course run includes a dot, as a result it's url was mismatching and it is fixed by adding the trailing slash "/" to the url. Here's the link to the initial GitHub issue: openedx/wg-build-test-release#301
These changes were implemented to research the following issue: openedx/wg-build-test-release#301
fix course detail page url as after creating a course, its detail page was not accessible and showing nothing. The course page was unavailable when the course run includes a dot, as a result it's url was mismatching and it is fixed by adding the trailing slash "/" to the url. Here's the link to the initial GitHub issue: openedx/wg-build-test-release#301
fix course detail page url as after creating a course, its detail page was not accessible and showing nothing. The course page was unavailable when the course run includes a dot, as a result it's url was mismatching and it is fixed by adding the trailing slash "/" to the url. Here's the link to the initial GitHub issue: openedx/wg-build-test-release#301
I left a comment on the PR, but if we don't get this merged I'll close it, considering that e-commerce will be archived after Sumac. |
I was testing the course seat creation on Ecom and I noticed that when I create a course on Studio having
course run
like 2000.01, and then create a course seat on Ecom. It will create course seats successfully. But when I try to open that course seat to see the details, it doesn't show anything.The problem is with the
course run
has zero after.
operator. i-e 2000.01. We need to fix the regex I think.Steps to reproduce:
Expected result:
It should show course seat detail.
Actual Result:
Not found
is showing in the network tab.Note: I can reproduce this issue if we installed the
ECOM
service on ourDemo
instance.The text was updated successfully, but these errors were encountered: