Skip to content

Resolve "Fix broken undo/redo for actions in the Table scope"

Nigel Gott requested to merge 1189-fix-broken-undo-redo-for-actions-in-the-table-scope into develop

The problem

  1. Undo redo for field actions like update field etc isn't working on develop, it was broken this last month and was working in 1.11
  2. An MR was introduced which very innocently imported a serializer into a signals.py file, there's nothing wrong with doing this and this MR was perfectly correct
  3. That serializer came from a file which on import time created the UndoRedoRequestSerializer . This serializer only accepts requests that look like:
{
scope: {
database_id: 10,
group_id:4
}
}
  1. The way that this serializer was constructed at import table was to loop over all of the ActionScopeTypes in the action_scope_registry, which is populated in the core/app.py:ready and database/app.py:ready methods
  2. Prior to the new import introduced in the MR in step 2 the UndoRedoRequestSerializer was constructed when Django imported all the various url.py files after it has already called all the .ready methods (and hence setup all of the registries)
  3. Now because of the new import and because the ws app which contained the signals.py this UndoRedoRequestSerializer was being instantiated before the database.ready app method was called and hence was missing the database `ActionScopeTypes
  4. Because of DRF's default behaviour of ignoring unknown JSON attributes on request bodies it parses, we were now just silently ignoring the scope.table_id property on the undo / redo endpoints, hence causing these endpoints not to find actions in this scope to undo.

The fix

  1. I ensure we only construct the UndoRedoRequestSerializer in the views.py file itself. This file should only ever be imported AFTER the registries have been setup.

Contributing factors to this bug

  1. It was discovered the providing no scopes, matches all actions in the session, and DRF silently ignoring the table scope resulting in tests running matching against no scopes and hence all actions
  2. We didn't have an tests of the undo/redo API endpoints themselves in the database module using database scopes
  3. The unknown fields mixin which was meant to error when the user provided an unknown scope was not working but is now fixed.

Merge Request Checklist

  • changelog.md Not updated the changelog as this bug was never released
  • New/updated Premium features are separated correctly in the premium folder
  • The latest Chrome and Firefox have been used to test any new frontend features
  • Documentation has been updated
  • Quality Standards are met
  • Performance: tables are still fast with 100k+ rows, 100+ field tables
  • The redoc API pages have been updated for any REST API changes
  • Our custom API docs are updated for changes to endpoints accessed via api tokens
  • The UI/UX has been updated following UI Style Guide

Closes #1189 (closed)

Edited by Nigel Gott

Merge request reports