fix!: fix course detail page url bug #4033
Conversation
Faraz32123
force-pushed
the
fix/fix_course_detail_page_url_issue
branch
from
4a6a4a4
to
078abaa
Compare
Faraz32123
force-pushed
the
fix/fix_course_detail_page_url_issue
branch
2 times, most recently
from
05838de
to
b3ec527
Compare
|
@mphilbrick211 @regisb Following up on the discussion from https://discuss.openedx.org/t/contributors-meetup-async-update-september-2nd-2023-september-15th-2023/11185/19 as suggested
@regisb You mentioned that this is should actually be an OSPR - is it because it originates from you, or that @Faraz32123 you meant to open this as an OSPR? |
|
I'm not sure what is meant by "truly open source". If the question is whether this PR was opened on behalf of 2U, then no, that's not the case. Faraz is making this contribution on behalf of Edly, to resolve a problem for the community. For all means and purposes, this PR can be considered as a free, open source contribution. Is there a bigger context that I am missing here? Would there be the same question if I had opened the PR myself? |
As far as I understand that's what created the confusion yes - but @mphilbrick211 might be able to confirm. |
mphilbrick211
added
open-source-contribution
|
@antoviaque @Faraz32123 - I checked in on this with Ned B. to see if there was an issue with the bot. We don't see any issues, so suspect maybe there was a brief outage and this pull request never got labeled. I've added it to the contributions board now. |
mphilbrick211
removed
the
needs test run
|
Hi @Faraz32123! Looks like there's a failing check. Would you mind taking a look? |
|
I'm pretty sure that the build issue has nothing to do with the changes in this PR: https://readthedocs.org/projects/edx-ecommerce/builds/22486927/ @Faraz32123 you should just have to rebase your changes on top of master to fix the build. |
Faraz32123
force-pushed
the
fix/fix_course_detail_page_url_issue
branch
from
46bd190
to
e4461c3
Compare
mphilbrick211
added
the
needs test run
e0d
removed
the
needs test run
mphilbrick211
added
waiting for eng review
|
@Faraz32123 what's the status of this PR? Can you please rebase on top of master? |
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
Faraz32123
force-pushed
the
fix/fix_course_detail_page_url_issue
branch
from
e4461c3
to
5b1fde3
Compare
| @@ -31,6 +31,11 @@ define([ | |||
|
|
|||
| return Backbone.RelationalModel.extend({ | |||
| urlRoot: '/api/v2/courses/', | |||
| // course detail page url was not matching because of missing trailing slash, | |||
| // with thi function, trailing slash will be added to match the url | |||
There was a problem hiding this comment.
| // with thi function, trailing slash will be added to match the url | |
| // with this function, trailing slash will be added to match the url |
|
@Faraz32123 a small grammar change that I can't make to your PR but if you accept that, I can merge this. |
Opened this PR with a fix that @regisb suggested for this issue.