fix: fixing the documentation article links #37493
Conversation
The articles were updated because they pointed to the old documentation.
The documentation links were pointing to the old documentation and in many cases, these links no longer worked, so we have updated these links to the new documentation.
|
Thanks for the pull request, @DeimerM! This repository is currently maintained by Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review. 🔘 Get product approvalIf you haven't already, check this list to see if your contribution needs to go through the product review process.
🔘 Provide contextTo help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:
🔘 Get a green buildIf one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green. DetailsWhere can I find more information?If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources: When can I expect my changes to be merged?Our goal is to get community contributions seen and reviewed as efficiently as possible. However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:
💡 As a result it may take up to several weeks or months to complete a review and merge your PR. |
openedx-webhooks
added
the
open-source-contribution
|
Hi folks, I hope you're doing very well. I wanted to ask for your support in reviewing this Pull Request. @feanil @sarina @felipemontoya @Apgomeznext |
|
The reason these links were not changed is because (I thought) they were in the legacy frontend, and I (am not sure) if sites can override these, and we didn't want to break all the links for edx.org. The links in the MFEs (should) default to docs.openedx.org, and can be overridden: https://github.com/openedx/frontend-platform?tab=readme-ov-file#overriding-default-external-links |
|
The behavior we are seeing is very much in the course authoring mfe. The "Getting started with Open edX Demo Site - Studio" link points to https://edx.readthedocs.io/projects/open-edx-building-and-running-a-course/getting_started/CA_get_started_Studio.html which is a 404. That is how we initially found it and how this PR came about. When researching this, @DeimerM found a way to override it on our end, but it seemed to us that a default community install should have it's help links already pointing to the community docs. I believe edx.org is already using the override, as the same link in https://course-authoring.edx.org/home points towards https://edx.readthedocs.io/projects/edx-partner-course-staff/en/latest/getting_started/CA_get_started_Studio.html which does work. |
|
I thought this was addressed in the Authoring MFE with the completion of openedx/platform-roadmap#440 |
|
on my master sandbox I see that the link to the demo course is wrong though: |
|
Oh, that's because it should be training.openedx.io - I'll make a PR to fix it. openedx/frontend-component-footer#561 |
|
I don't have a master sandbox at the moment to test is myself. The screenshot you posted fixes it for the buttons below (green line). Does it also fix it for the sidebar (orange-line)? 
|
|
That sidebar doesn't appear on my master branch: 
|
|
I suppose that you also don't have anything in the sidebar in the /authoring/home page? In the course outline I get a complete sidebar full of helpful content and links that also point towards the edx-specific docs. 
@DeimerM do you have an environment from the main branches to see if the whole sidebar is gone now? |
|
@felipemontoya I'm using Tutor main with the main branches, and I can see the sidebar. The only difference is that I've disabled the Indigo Tutor plugin. 
|
|
@DeimerM I spoke to Adolfo, his is what he says (note our sandbox is not running Indigo): Ah. I've seen this before. The question would be, which is correct? Dev or prod? |
|
It could be an issue with the plugin slot in the prod configuration, as that's where Aspects shows. |
|
It could be, but aspects also has a button in the top and at least in teak it is not causing issues.
|
|
Gotcha. We don't have time on our side to investigate this before the cut, unfortunately, but looking at it, That sidebar is the only default content in the course_outline_sidebar slot. It's very likely that a plugin is clobering it, and I suspect Aspects. It should look like this: 
|
|
@robrap could you take a look at this? Felipe reports this:
|
|
By pure chance I was investigating the way aspects loads UI Slots just this morning. I think this is the culprit: This happens when you do have aspects installed, but have the flag for ASPECTS_ENABLE_STUDIO_IN_CONTEXT_METRICS as off (default). Anyhow, the fixing of the sidebar slot config should not block the conversation of having better defaults for the documentation links. |
|
Agreed, I'd like Robert to weigh in before proceeding. |
|
@felipemontoya @DeimerM could we confirm that these defaults are overridable and that there is documentation on how to do that? |
|
Hi sarina, @DeimerM is out on vacation this week. On monday he will be able to answer in detail. My understanding is that yes it is overridable, but I'll let him answer properly. |
Hello @sarina, please excuse the delay in my response. I would like to confirm that I conducted some research and can verify that it is indeed possible to override these defaults. To do so, you need to follow these steps:
By following these steps, you should be able to replace these defaults. Regarding the documentation, the tool responsible for forming these URLs details its functionality and consequences thoroughly in its README: https://github.com/openedx/help-tokens?tab=readme-ov-file#documentation I await any questions or comments you may have. Thank you very much for your help. |
|
@sarina: As long as the required override is documented in the Verawood release notes, feel free to proceed as you wish. Note that a fast-track DEPR is probably the right process for inform around this. Thanks. |
|
@DeimerM could you follow the fast-track depr process for this change, and we can get it merged? |
Hi @sarina Thank you very much for your answer. I would like to confirm that I have started the fast-track process. Please let me know if there is anything I need to do on my end. Thank you again for your help! |
|
@DeimerM: Thanks. The OEP doesn't have a documented process for Fast-Track DEPRs yet, but openedx/.github#175 (comment) will give you some guidance for announcing. |
Hi @robrap Thank you very much for your help. I would like to confirm that I have followed the announcement process. Please let me know if there is anything else I need to do. Thanks. |
Hi @robrap thank you very much for all your help and your comments. I will remain attentive to all the comments added in the DEPR ticket. Also, I would like to request your help if you could share with me the documentation on how to follow the process to add these notes to the operator notes in the Verawood release note. Thank you very much for your help. |
|
@DeimerM the Releases Homepage (https://openedx.atlassian.net/wiki/spaces/OEPM/pages/4191191044/Open+edX+Releases+Homepage) will always have information on upcoming releases and places to put breaking changes. There's a link to the Verawood page there currently. |
|
@robrap can we merge this if the DEPR is accepted? |
|
@sarina: Your DEPR gives warning until 11-24 for merging. This would just affect developers on master, and MIT as an early deployer at this point. I no longer need to be a blocker. Yay! |
sarina
left a comment
sarina
left a comment
There was a problem hiding this comment.
Just a couple questions about the defaults, when resolve this'll be good to merge.
| checklist = course_author:concepts/releasing-course/course_launch_checklist.html | ||
| import_library = course_author:how-tos/course_development/export_import_library.html#import-a-legacy-library | ||
| import_course = course_author:how-tos/releasing-course/import_course.html | ||
| export_library = course_author:how-tos/course_development/export_import_library.html#export-a-legacy-library |
There was a problem hiding this comment.
Should this link be for legacy libraries? Do you know where this link is used?
There was a problem hiding this comment.
I investigated where this article is used, but could not locate any references.
While checking the Authoring MFE, I found the following article in the libraries section:
However, upon reviewing the MFE code, I determined that the link was hardcoded there, as you can see in the following reference link: https://github.com/openedx/frontend-app-authoring/blob/735159311d72e21f2adff1f12cf5dee335f612f8/src/studio-home/tabs-section/libraries-v2-tab/WelcomeLibrariesV2Alert.tsx#L13
Additionally, my search for documentation on how to export/import libraries only returned articles related to the legacy libraries.
There was a problem hiding this comment.
OK, I'm asking around on this.
Nevermind, I think we should just remove the unused tokens in followup PR.
| welcome = course_author:index.html | ||
| login = course_author:index.html | ||
| register = course_author:index.html |
There was a problem hiding this comment.
Where are these used? I'm not sure the course_author:index is a very useful path to point to
There was a problem hiding this comment.
I conducted an investigation into these tokens, but they do not appear to be used anywhere. Given that no specific article addressing these functionalities could be located, I believe this is the most pertinent approach.
DeimerM
changed the title
Hello @sarina, thank you very much for your feedback! I have conducted some investigation and provided a response. I will be very attentive in case you have any doubts or suggestions. I would also like to confirm that I have added these changes to the Verawood Operator Release Notes article. Thank you very much for your help! |
|
@DeimerM could you create a follow-up PR once this is merged to remove the unused tokens? |
|
@felipemontoya will you be able to merge this? |
|
Sure, I'm happy to merge. I'll give notice in the slack channel and hit the button in half hour |
5 participants
Description
Upon reviewing the documentation article links displayed in edx-platform as a resource for users, it was evident that the links were pointing to the old documentation: https://edx.readthedocs.io
Many of these links no longer work today, and attempting to access them results in errors. An example of this can be found on the Studio homepage, in the link highlighted in the image below:
As part of the process, I created the following document: Procedure for Resolving Broken Links, which contains the current documentation links (old documentation) and the proposed current documentation links in this PR.
I believe it would be important to use this PR for a discussion regarding the proposed links so that we can select the best options.