Page MenuHomePhabricator

Create new serializer for list diff generation
Closed, ResolvedPublicBUG REPORT

Description

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.

Event Timeline

Slst2020 changed the task status from Open to In Progress.Jan 27 2022, 12:11 PM
Slst2020 moved this task from To Do to Doing on the User-Slst2020 board.

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

https://gerrit.wikimedia.org/r/757836

Slst2020 changed the task status from In Progress to Open.Jan 28 2022, 8:01 AM
Slst2020 moved this task from Doing to Done on the User-Slst2020 board.

Change 757836 merged by jenkins-bot:

[wikimedia/toolhub@main] lists: ToolListDiffSerializer for generating diffs

https://gerrit.wikimedia.org/r/757836

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

https://gerrit.wikimedia.org/r/770638

Change 770638 merged by jenkins-bot:

[operations/deployment-charts@master] toolhub: Bump container version to 2022-03-15-002555-production

https://gerrit.wikimedia.org/r/770638