In T298698#7649465, @bd808 wrote:In T298698#7648812, @Slst2020 wrote:So... is this expected behavior, and I just have to beat the output into an appropriate form for diff-viewing, or is it punishment for having wanted the list revisions API changed? xD
This sounds very much like unintended changes to what is being diffed. Specifically this looks to me like the full tool records being diffed rather than just their names. I did not check for that when reviewing @Raymond_Ndibe's patch for T299144: Change GET /api/lists/{list_pk}/revisions/{id}/ to emit full info on tools contained in the list, but in retrospect I should have.
The content for the diff is prepared in the toolhub.apps.lists.views.ToolListRevisionViewSet._get_patch method. In that method we are using the ToolListHistoricVersionSerializer class to prepare the JSON output for each revision to feed into the diff generation. That is the serializer that was changed in T299144 to output each tool in the list using a SummaryToolSerializer. This would lead to the diff output you are now seeing. We do want this new ToolListHistoricVersionSerializer behavior for viewing historic revisions, but we want the older behavior for generating diffs.
The most likely fix would be to introduce a new serializer which works like ToolListHistoricVersionSerializer did before rWTHU355e78c4dce7: lists: emit full info on tools contained in a historic revision specifically for use inside of toolhub.apps.lists.views.ToolListRevisionViewSet._get_patch. This would return the diff generation to only comparing the contents and order of the list of toolinfo record names rather than the summary details of each of those toolinfo records.
Description
Description
Details
Details
Status | Subtype | Assigned | Task | ||
---|---|---|---|---|---|
Resolved | Goal | bd808 | T271386 Curated lists of tools | ||
Resolved | Slst2020 | T293164 Implement UI for viewing list revision history, content diffs, and patrolling actions | |||
Resolved | Slst2020 | T298698 Implement UI for viewing content diffs | |||
Resolved | BUG REPORT | Slst2020 | T300157 Create new serializer for list diff generation |
Event Timeline
Comment Actions
Change 757836 had a related patch set uploaded (by Slavina Stefanova; author: Slavina Stefanova):
[wikimedia/toolhub@main] lists: Bring back previous ToolListHistoricVersionSerializer for use in diffs
Comment Actions
Change 757836 merged by jenkins-bot:
[wikimedia/toolhub@main] lists: ToolListDiffSerializer for generating diffs
Comment Actions
Change 770638 had a related patch set uploaded (by BryanDavis; author: Bryan Davis):
[operations/deployment-charts@master] toolhub: Bump container version to 2022-03-15-002555-production
Comment Actions
Change 770638 merged by jenkins-bot:
[operations/deployment-charts@master] toolhub: Bump container version to 2022-03-15-002555-production