Skip to content
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

Open
ragesoss opened this issue Dec 10, 2024 · 7 comments · May be fixed by #6072
Open

DiffViewer breaks if article has been moved #6056

ragesoss opened this issue Dec 10, 2024 · 7 comments · May be fixed by #6072
Labels

Comments

@ragesoss
Copy link
Member

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).

@ragesoss ragesoss added the bug label Dec 10, 2024
@ragesoss
Copy link
Member Author

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.

@Abishekcs
Copy link
Contributor

Working on this.

@ragesoss
Copy link
Member Author

@Abishekcs be sure to start from the latest code as of now; I just merged some significant changes to DiffViewer.

@Abishekcs
Copy link
Contributor

just an update even the ArticlerViewer along with the reported DiffViewer breaks if article has been moved !

Screenshot from 2024-12-13 23-50-13

@ragesoss
Copy link
Member Author

I think that's okay behavior for the Article Viewer, as it still renders the component and shows the error.

@Abishekcs
Copy link
Contributor

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😄

Ideally, it should be able to follow redirects and still display diffs for moved articles.

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:
https://en.wikipedia.org/w/api.php?action=query&pageids=${article_mw_page_id}&format=json&origin=*.

Example:
https://en.wikipedia.org/w/api.php?action=query&pageids=1639997&format=json.

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.

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).

Or would it be better to stick with this approach?

@ragesoss
Copy link
Member Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants