Merge first and second level menu items

[frcroth]

Backup before applying these migrations would be wise.

Noticed an error during the migration on production data

[lukasrad02]

When testing your migration on a database with production data, the following error is thrown:

Traceback (most recent call last):
  File "/usr/local/lib/python3.11/site-packages/django/db/backends/utils.py", line 87, in _execute
    return self.cursor.execute(sql)
           ^^^^^^^^^^^^^^^^^^^^^^^^
psycopg2.errors.ObjectInUse: cannot DROP TABLE "core_firstlevelmenuitem" because it has pending trigger events


The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/app/manage.py", line 13, in <module>
    execute_from_command_line(sys.argv)
  File "/usr/local/lib/python3.11/site-packages/django/core/management/__init__.py", line 442, in execute_from_command_line
    utility.execute()
  File "/usr/local/lib/python3.11/site-packages/django/core/management/__init__.py", line 436, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
  File "/usr/local/lib/python3.11/site-packages/django/core/management/base.py", line 412, in run_from_argv
    self.execute(*args, **cmd_options)
  File "/usr/local/lib/python3.11/site-packages/django/core/management/base.py", line 458, in execute
    output = self.handle(*args, **options)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/django/core/management/base.py", line 106, in wrapper
    res = handle_func(*args, **kwargs)
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/django/core/management/commands/migrate.py", line 356, in handle
    post_migrate_state = executor.migrate(
                         ^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/django/db/migrations/executor.py", line 135, in migrate
    state = self._migrate_all_forwards(
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/django/db/migrations/executor.py", line 167, in _migrate_all_forwards
    state = self.apply_migration(
            ^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/django/db/migrations/executor.py", line 252, in apply_migration
    state = migration.apply(state, schema_editor)
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/django/db/migrations/migration.py", line 132, in apply
    operation.database_forwards(
  File "/usr/local/lib/python3.11/site-packages/django/db/migrations/operations/models.py", line 389, in database_forwards
    schema_editor.delete_model(model)
  File "/usr/local/lib/python3.11/site-packages/django/db/backends/base/schema.py", line 487, in delete_model
    self.execute(
  File "/usr/local/lib/python3.11/site-packages/django/db/backends/postgresql/schema.py", line 48, in execute
    return super().execute(sql, None)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/django/db/backends/base/schema.py", line 201, in execute
    cursor.execute(sql, params)
  File "/usr/local/lib/python3.11/site-packages/django/db/backends/utils.py", line 67, in execute
    return self._execute_with_wrappers(
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/django/db/backends/utils.py", line 80, in _execute_with_wrappers
    return executor(sql, params, many, context)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/django/db/backends/utils.py", line 84, in _execute
    with self.db.wrap_database_errors:
  File "/usr/local/lib/python3.11/site-packages/django/db/utils.py", line 91, in __exit__
    raise dj_exc_value.with_traceback(traceback) from exc_value
  File "/usr/local/lib/python3.11/site-packages/django/db/backends/utils.py", line 87, in _execute
    return self.cursor.execute(sql)
           ^^^^^^^^^^^^^^^^^^^^^^^^
django.db.utils.OperationalError: cannot DROP TABLE "core_firstlevelmenuitem" because it has pending trigger events

Could be related to https://stackoverflow.com/a/12838113, as this error does not occur when splitting the RunPython and DeleteModel calls from migration 0013 into two separate migrations.

[lukasrad02]

MenuItem is missing here

[lukasrad02]

I've just noticed that the visible_for permissions are lost on all menu items after applying the migration.

[frcroth]

I've just noticed that the visible_for permissions are lost on all menu items after applying the migration.

Hmm, for me with sqlite this is not the case. Need to investigate

[lukasrad02]

I was able to reproduce it in the local setup with SQLite using the following steps:

1. Check out the latest commit on the main branch
2. Ensure all packages are up-to-date
3. Re-create the database (rm db.sqlite3; python manage.py migrate)
4. Populate the database with test data and create a superuser
5. Run the server.
6. Check that the "Study Council" menu item from the test data is indeed visible for "Student"
7. Stop the server
8. Check out the latest commit of this PR
9. Migrate the database
10. Run the server
11. The "Study Council" menu item has now lost its visible for permissions

[frcroth]

I'm stumped on how to fix this. Setting the visible_for value in the shell later works in the way it is done in the migration (I also tried using set() suggested by ChatGPT), also stepping through it seems visible_for is set for the new item. Maybe the delete of the old menuitem triggers the removal of the relationship at a later point and thus the newly set relationship is then deleted again?
Any ideas @jeriox ?

[lukasrad02]

Maybe the delete of the old menuitem triggers the removal of the relationship at a later point and thus the newly set relationship is then deleted again?

Even without migration 0014 (deletion of old menu item models), the permissions are still lost for me.

[frcroth]

Maybe the delete of the old menuitem triggers the removal of the relationship at a later point and thus the newly set relationship is then deleted again?

Even without migration 0014 (deletion of old menu item models), the permissions are still lost for me.

Yes I mean the individual object delete() calls

[lukasrad02]

The migration now preserves permission. However, I've received an error when accessing some menu items after the migration. Will investigate…

ERROR 2024-06-11 11:24:48,320 django.request log Internal Server Error: /admin/pages/66/
Traceback (most recent call last):
  File "/usr/local/lib/python3.11/site-packages/django/core/handlers/exception.py", line 55, in inner
    response = get_response(request)
               ^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/django/core/handlers/base.py", line 197, in _get_response
    response = wrapped_callback(request, *callback_args, **callback_kwargs)
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/django/views/decorators/cache.py", line 62, in _wrapper_view_func
    response = view_func(request, *args, **kwargs)
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/wagtail/admin/urls/__init__.py", line 173, in wrapper
    return view_func(request, *args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/wagtail/admin/auth.py", line 151, in decorated_view
    response = view_func(request, *args, **kwargs)
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/django/views/generic/base.py", line 104, in view
    return self.dispatch(request, *args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/wagtail/admin/views/generic/permissions.py", line 31, in dispatch
    return super().dispatch(request, *args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/django/views/generic/base.py", line 143, in dispatch
    return handler(request, *args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/wagtail/admin/views/pages/listing.py", line 221, in get
    return super().get(request)
           ^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/django/views/generic/list.py", line 174, in get
    context = self.get_context_data()
              ^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/wagtail/admin/views/pages/listing.py", line 409, in get_context_data
    side_panels = self.get_side_panels()
                  ^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/wagtail/admin/views/pages/listing.py", line 433, in get_side_panels
    self.parent_page.get_latest_revision_as_object(),
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/wagtail/models/__init__.py", line 1759, in get_latest_revision_as_object
    return latest_revision.as_object()
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/wagtail/models/__init__.py", line 2820, in as_object
    return self.content_object.with_content_json(self.content)
           ^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/django/contrib/contenttypes/fields.py", line 253, in __get__
    rel_obj = ct.get_object_for_this_type(pk=pk_val)
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/django/contrib/contenttypes/models.py", line 181, in get_object_for_this_type
    return self.model_class()._base_manager.using(self._state.db).get(**kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AttributeError: 'NoneType' object has no attribute '_base_manager'

[frcroth]

@lukasrad02 Did you have any further insights on this?

[lukasrad02]

Sorry, I've been tracing down the wrong path for quite a while, but I think I have identified the underlying issue now.

The revision model from wagatil references the object it is a revision of. This includes the type of model: https://github.com/wagtail/wagtail/blob/72b965cd94726e9fa0ceb64f10ef7ef88476fb2c/wagtail/models/__init__.py#L2866C5-L2868C6. After our migration, the revisions of the menu items are still pointing to content types firstlevelmenuitem or secondlevelmenuitem, which causes errors when interacting with the revision.

I have set the correct page.latest.revision.content_type on the page causing the error manually in the python shell and was able to visit /admin/pages/66/ afterwards. For some reason, there are also pages with an incorrect content_type on their latest revision that do not cause an error when viewing them in the admin interface, but calling revision.as_object() on them in the python shell still fails.

I would propose to update the content_type of all revisions of all menu items in the migration.

[frcroth]

@lukasrad02 I updated the content type, you could test it again

Also I guess I messed up the history, but we can squash this

[lukasrad02]

Now we're again losing permissions on some menu items. I'll try to investigate further, but on the first glance, I have no idea why only some pages are affected…

[lukasrad02]

If I did not miss anything, only second level menu items are affected by the permission loss. Changing the migration 13 so that we first migrate second level menu items fixes the issue and both first- and second-level menu items keep the assigned permissions.