BaseMediaForm cannot be subclassed

[jsma]

Issue Summary

I attempted a customization along the lines of #64. My first approach was to subclass the existing BaseMediaForm so I could inherit the existing __init__ logic and permission_policy and focus solely on modifying the widgets. However, this lead to errors since get_base_form() is called at the module level:

wagtailmedia/src/wagtailmedia/forms.py

Line 48 in 4d0efb9

media_base_form = get_media_base_form()

The resulting error:

AttributeError: partially initialized module 'my_app.forms' has no attribute 'CustomMediaForm' (most likely due to a circular import)

Any reason the get_base_form() call cannot happen inside get_media_form()?

Happy to submit a PR if this change is acceptable. Thanks!

[zerolab]

Hey @jsma,

I've customised the media form without issues in the past.

Can you update the issue with

1. a full traceback
2. a minimal code sample to reproduce the issue

The key issue here seems to be the circular import in the error.

[jsma]

Hi @zerolab,

Thanks for the reply! The simplest way to reproduce in my project is to have this in forms.py:

from wagtailmedia.forms import BaseMediaForm

def no_op():
    pass


class CustomMediaForm(BaseMediaForm):
    pass

Then in admin.py:

from .forms import no_op

This results in:

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

Traceback (most recent call last):
  File "manage.py", line 27, in <module>
    execute_from_command_line(sys.argv)
  File "/usr/local/lib/python3.8/site-packages/django/core/management/__init__.py", line 419, in execute_from_command_line
    utility.execute()
  File "/usr/local/lib/python3.8/site-packages/django/core/management/__init__.py", line 395, in execute
    django.setup()
  File "/usr/local/lib/python3.8/site-packages/django/__init__.py", line 24, in setup
    apps.populate(settings.INSTALLED_APPS)
  File "/usr/local/lib/python3.8/site-packages/django/apps/registry.py", line 122, in populate
    app_config.ready()
  File "/usr/local/lib/python3.8/site-packages/django/contrib/admin/apps.py", line 27, in ready
    self.module.autodiscover()
  File "/usr/local/lib/python3.8/site-packages/django/contrib/admin/__init__.py", line 24, in autodiscover
    autodiscover_modules('admin', register_to=site)
  File "/usr/local/lib/python3.8/site-packages/django/utils/module_loading.py", line 47, in autodiscover_modules
    import_module('%s.%s' % (app_config.name, module_to_search))
  File "/usr/local/lib/python3.8/importlib/__init__.py", line 127, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
  File "<frozen importlib._bootstrap>", line 1014, in _gcd_import
  File "<frozen importlib._bootstrap>", line 991, in _find_and_load
  File "<frozen importlib._bootstrap>", line 975, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 671, in _load_unlocked
  File "<frozen importlib._bootstrap_external>", line 843, in exec_module
  File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
  File "/app/myproj/myapp/admin.py", line 6, in <module>
    from .forms import no_op
  File "/app/myproj/myapp/forms.py", line 8, in <module>
    from wagtailmedia.forms import BaseMediaForm
  File "/usr/local/lib/python3.8/site-packages/wagtailmedia/forms.py", line 48, in <module>
    media_base_form = get_media_base_form()
  File "/usr/local/lib/python3.8/site-packages/wagtailmedia/forms.py", line 42, in get_media_base_form
    base_form = import_string(base_form_override)
  File "/usr/local/lib/python3.8/site-packages/django/utils/module_loading.py", line 22, in import_string
    raise ImportError('Module "%s" does not define a "%s" attribute/class' % (
ImportError: Module "myapp.myproj.forms" does not define a "MediaForm" attribute/class

I noticed that wagtail does not call get_document_base_form() and get_image_base_form() at the module level but only within their corresponding get_document_form() and get_image_form() functions. Here is the current implementation of document form handling:

def get_document_base_form():
    base_form_override = getattr(settings, "WAGTAILDOCS_DOCUMENT_FORM_BASE", "")
    if base_form_override:
        from django.utils.module_loading import import_string
        base_form = import_string(base_form_override)
    else:
        base_form = BaseDocumentForm
    return base_form


def get_document_form(model):
    fields = model.admin_form_fields
    if 'collection' not in fields:
        # force addition of the 'collection' field, because leaving it out can
        # cause dubious results when multiple collections exist (e.g adding the
        # document to the root collection where the user may not have permission) -
        # and when only one collection exists, it will get hidden anyway.
        fields = list(fields) + ['collection']


    return modelform_factory(
        model,
        form=get_document_base_form(),
        fields=fields,
        formfield_callback=formfield_for_dbfield,
    )

I'll submit a PR shortly.

[zerolab]

Thank you for taking the time to provide the additional info and the PR.
Funny enough I submitted the Wagtail core documents/images form customisation functionality based on wagtailmedia's one (e.g. wagtail/wagtail#6462)

Will try to review your PR asap