linuxsoftware / ls.joyous

A calendar application for Wagtail
BSD 3-Clause "New" or "Revised" License
74 stars 35 forks source link

Issues trying to use Modeltranslation #33

Closed linuxsoftware closed 4 years ago

linuxsoftware commented 4 years ago

Report from @vbroskas :

I'm running into some issues trying to use Modeltranslation with the joyous calendar. When I add the translation options as you have above I get this error:

Exception in thread django-main-thread:
Traceback (most recent call last):
  File "/usr/lib/python3.6/threading.py", line 916, in _bootstrap_inner
    self.run()
  File "/usr/lib/python3.6/threading.py", line 864, in run
    self._target(*self._args, **self._kwargs)
  File "/home/lo/Code/clients/nmafc/venv/lib/python3.6/site-packages/django/utils/autoreload.py", line 54, in wrapper
    fn(*args, **kwargs)
  File "/home/lo/Code/clients/nmafc/venv/lib/python3.6/site-packages/django/core/management/commands/runserver.py", line 117, in inner_run
    self.check(display_num_errors=True)
  File "/home/lo/Code/clients/nmafc/venv/lib/python3.6/site-packages/django/core/management/base.py", line 390, in check
    include_deployment_checks=include_deployment_checks,
  File "/home/lo/Code/clients/nmafc/venv/lib/python3.6/site-packages/django/core/management/base.py", line 377, in _run_checks
    return checks.run_checks(**kwargs)
  File "/home/lo/Code/clients/nmafc/venv/lib/python3.6/site-packages/django/core/checks/registry.py", line 72, in run_checks
    new_errors = check(app_configs=app_configs)
  File "/home/lo/Code/clients/nmafc/venv/lib/python3.6/site-packages/wagtail/admin/checks.py", line 63, in get_form_class_check
    if not issubclass(edit_handler.get_form_class(), WagtailAdminPageForm):
  File "/home/lo/Code/clients/nmafc/venv/lib/python3.6/site-packages/wagtail/admin/edit_handlers.py", line 369, in get_form_class
    widgets=self.widget_overrides())
  File "/home/lo/Code/clients/nmafc/venv/lib/python3.6/site-packages/wagtail/admin/edit_handlers.py", line 64, in get_form_for_model
    return metaclass(class_name, (form_class,), form_class_attrs)
  File "/home/lo/Code/clients/nmafc/venv/lib/python3.6/site-packages/wagtail/admin/forms/models.py", line 66, in __new__
    new_class = super(WagtailAdminModelFormMetaclass, cls).__new__(cls, name, bases, attrs)
  File "/home/lo/Code/clients/nmafc/venv/lib/python3.6/site-packages/modelcluster/forms.py", line 245, in __new__
    new_class = super(ClusterFormMetaclass, cls).__new__(cls, name, bases, attrs)
  File "/home/lo/Code/clients/nmafc/venv/lib/python3.6/site-packages/django/forms/models.py", line 266, in __new__
    raise FieldError(message)
django.core.exceptions.FieldError: Unknown field(s) (utc2local, upload) specified for CalendarPage

If I take the calendar page out of my translation.py file, i then run into an error where I can't publish a calendar page. I get a validation error saying that the title field can't be empty, even though I have it filled in.

Any ideas? Thanks for any help you can provide!

Originally posted by @vbroskas in https://github.com/linuxsoftware/ls.joyous/issues/2#issuecomment-670985461

linuxsoftware commented 4 years ago

As a temporary work-around I suggest adding the following code to your translation.py before you register CalendarPage for translation.

from ls.joyous.models.calendar import CalendarPageForm
CalendarPageForm.registerImportHandler = lambda *args: None

This will disable the iCal import functionality. I will continue to investigate a better solution, but this will probably require a new release of Joyous.

My most minimal translation.py file looks like this (below). I do not know why those 5 Joyous pages require registering, but others like CancellationPage, ExtraInfoPage, PostponementPage etc do not?!

from wagtail_modeltranslation.translation import TranslationOptions
from modeltranslation.decorators import register
from .models import HomePage
from ls.joyous.models import GroupPage, CalendarPage, SimpleEventPage, MultidayEventPage, RecurringEventPage

@register(HomePage)
class HomeTR(TranslationOptions):
    fields = (
    )

from ls.joyous.models.calendar import CalendarPageForm
CalendarPageForm.registerImportHandler = lambda *args: None

@register(CalendarPage)
class CalendarTR(TranslationOptions):
    fields = (
    )

@register(GroupPage)
class GroupTR(TranslationOptions):
    fields = (
    )

@register(SimpleEventPage)
class SimpleEventTR(TranslationOptions):
    fields = (
    )

@register(MultidayEventPage)
class MultidayTR(TranslationOptions):
    fields = (
    )

@register(RecurringEventPage)
class RecurringEventTR(TranslationOptions):
    fields = (
    )
linuxsoftware commented 4 years ago

The full source code for my test case is at https://github.com/linuxsoftware/joyous-issue33-mysite

vbroskas commented 4 years ago

Works great, thanks for all the help @linuxsoftware !

linuxsoftware commented 4 years ago

Another problem with this work-around is that the validation that the start-time of the event is before its end-time is now broken.

This is because wagtail-modeltranslation replaces the page's base_form_class attribute with no regard to its previous value. I intend to raise an issue for this once I have a nice simple test case for it.

linuxsoftware commented 4 years ago

For a proper fix install ls.joyous 1.3.0 and add the setting JOYOUS_DEFEND_FORMS=True

When this setting is set to True then the Joyous page models will not allow their base_form_class to be replaced. Instead they will assimilate the newly assigned form class. The purpose of this setting is to make Joyous compatible with the wagtail-modeltranslations app. (Users who do not use wagtail-modeltranslations probably do not need to enable this option.)

I have also raised the issue with the maintainers of wagtail-modeltranslations, for better documentation if nothing else.

linuxsoftware commented 4 years ago

Ooops. Sorry. Looks like there is still a problem. :-( I need to test further.

linuxsoftware commented 4 years ago

There is a new version of wagtail-modeltranslations v0.10.14, which fixes the form overwriting bug. And a new version of ls.joyous v1.3.1 which works around a problem with modeltranslation patch_constructor breaking initialization of pages which have multiple inheritance. I recommend upgrading both apps to these versions.

NB: The JOYOUS_DEFEND_FORMS setting mentioned above is still there in ls.joyous v1.3.1, but is not needed with wagtail-modeltranslations v0.10.14.

linuxsoftware commented 4 years ago

Also note that all pages (CancellationPage, ExtraInfoPage, PostponementPage etc) do need to be registered with modeltranslations. It may not give an error on startup, but it will error during runtime if these page types are accessed. See https://github.com/linuxsoftware/joyous-issue33-mysite/blob/master/home/translation.py for my translations file.

joaquinstirling commented 4 years ago

Hello, Thanks for making an example site. Although I cannot get your solution working. I keep getting this, I've the same as you on a fresh install.

Do you have a hint on what It might be? image

linuxsoftware commented 4 years ago

Can you confirm you have wagtail-modeltranslations v0.10.14 and ls.joyous v1.3.1 installed? And, which version of Django and Wagtail are you using?

joaquinstirling commented 4 years ago

Can you confirm you have wagtail-modeltranslations v0.10.14 and ls.joyous v1.3.1 installed? And, which version of Django and Wagtail are you using?

I can confirm I'm using those versions. Django: 2.2.X STABLE (LTS) Wagtail: 2.10.1

PD: I'm using Divio and using their packages for both apps. https://github.com/divio/aldryn-wagtail https://github.com/divio/aldryn-django

linuxsoftware commented 4 years ago

I suspect the problem is with trying to add an extra migration to an installed package like Joyous in this environment. Try copying the migrations from Joyous to you own app and then add a MIGRATION_MODULES entry for it to your settings. e.g.

MIGRATION_MODULES = {
    'joyous': 'home.migratejoyous'
}

Then, when you run makemigrations, the new migration should be created in this directory under your app.

I have updated https://github.com/linuxsoftware/joyous-issue33-mysite with this change.

joaquinstirling commented 4 years ago

I suspect the problem is with trying to add an extra migration to an installed package like Joyous in this environment. Try copying the migrations from Joyous to you own app and then add a MIGRATION_MODULES entry for it to your settings. e.g.

MIGRATION_MODULES = {
    'joyous': 'home.migratejoyous'
}

Then, when you run makemigrations, the new migration should be created in this directory under your app.

I have updated https://github.com/linuxsoftware/joyous-issue33-mysite with this change.

Hi, I've replicated what you did on issue-33 site but I cannot even run the migrations because the same exception appears... It's quite strange. I think it has to do with Page TranslationOption, The fields that raise the error come because the page hierarchy

linuxsoftware commented 4 years ago

I have no further suggestions. This is working for me on Divio with Aldryn-Wagtail and Aldryn-Django.