-
Notifications
You must be signed in to change notification settings - Fork 642
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
DiffViewer breaks if article has been moved #6056
Comments
To replicate this, you can start by populating a course with data that includes edited articles (eg, via the populate_dashboard script), and then editing the title of the Article and/or ArticlesCourses records via a rails console. |
Working on this. |
@Abishekcs be sure to start from the latest code as of now; I just merged some significant changes to DiffViewer. |
I think that's okay behavior for the Article Viewer, as it still renders the component and shows the error. |
Okay, I got it. Thank you😄
I attempted this approach by using the MediaWiki page ID of the article ( as the MediaWiki page ID remain the same even if the article is moved). to fetch the current title through the API endpoint: Example: I then compared the fetched title with the current article title. If they were different, I updated the article name in the Redux store and fetched the revision details (i.e., the first and last revisions) with the current updated title.
Or would it be better to stick with this approach? |
If you can make it successfully load a moved page, that's definitely better than a 'could not find' message. Using the page ID is a fine strategy. There are cases where that might fail even while following a redirect would succeed, but that will also work in some cases where there is no redirect, so that's probably actually a little better than the redirect-based approach. |
The DiffViewer tool is available on the Articles Edited view of a course page (as well as a few other places).
If an article gets subsequently moved to a new title, and the Dashboard hasn't updated that article to reflect the move, then attempting to use the DiffViewer results in JS console error (and the DiffViewer doesn't render).
The DiffViewer should be updated to handle this situation. Ideally, it should be able to follow redirects and still show diffs for moved articles, but just showing a message like 'could not find article' would be okay (and probably necessary for the case where an article was deleted or moved without a redirect).
The text was updated successfully, but these errors were encountered: