rename url slug(not allowed) of static tabs on course import - #3152
rename url slug(not allowed) of static tabs on course import#3152zubair-arbi wants to merge 1 commit into
Conversation
|
@nasthagiri You might want to review this PR. |
|
@zubair-arbi Hi Zubair, what is the root reason for why there could be static tabs with url_slug values of "static"? |
|
@nasthagiri users can add/modify static tabs through editing exported xml course and re-import it. In this way they can change 'url_slug' to some verbose string e.g. 'static', instead of some hash string (through studio). |
|
@zubair-arbi So why just replace url_slugs that have the value "static"? Why don't we generate a new random url_slug for all static tabs when they are imported? |
|
The thing is nginx configuration checks for '/static/' in url and checks for staticfiles against it. So I guess there is no need for us to change all url_slugs on import. |
|
@zubair-arbi @adampalay I have done some digging into this issue. The problem is that the original course, https://edge.edx.org/courses/edX/X801/2013_Fall/, has a tab, "Peer Groups", with an invalid URL. If you click on "Peer Group", it will take you to a 404 Not Found error. Then, when Piotr exported that course and imported it, it continued to have the invalid static tab location (url_slug = "static"). So this issue is unrelated to the nginx configuration. You would get the same behavior if an invalid configured course had any other invalid url_slug such as "something_completely_different". So here are possible paths to proceed:
|
|
@nasthagiri Please check my comment on relative ticket STUD-1407. |
|
@zubair-arbi Thanks for clarifying the issue. Ideally, we should tackle it from the following two fronts:
|
|
@nasthagiri @zubair-arbi the nginx rules are all for urls where the root is '/static' not for all urls that have static in the name. |
|
@feanil I agree that the intent of the nginx rules must be to match the '/static' string in the beginning of the URL. However, checkout the following proof-of-concept test course I created with "static" as the "course number": https://studio.edge.edx.org/tabs/edx.static.2014/branch/draft/block/2014 |
|
@nasthagiri I see, I've created a ticket on the devops board to investigate this further. I think it makes sense to only have the static rule applied at the root of the url and not for just anywhere in the url. |
|
Here's the new devops ticket for updating the nginx rule: |
|
Closing this PR in favor of STUD-1515 |
STUD-1407
@waheedahmed @symbolist @adampalay