ls.joyous icon indicating copy to clipboard operation
ls.joyous copied to clipboard

Joyous EventBase.status field clashes with Django-Model-Utils StatusModel.status field

Open linuxsoftware opened this issue 4 years ago • 5 comments

@sam-mi reports:

Event.status fieldname clashes with Django Model Utils Model.status field making subclasses of ls.joyous EventBase incompatible with any project that already implements Model Utils StatusField in their models.

I assume https://github.com/jazzband/django-model-utils is the correct homepage for Django Model Utils?

Pull request https://github.com/linuxsoftware/ls.joyous/pull/34

This would break backwards compatibility for Joyous so would have to go in a 2.0 release.

linuxsoftware avatar Aug 11 '20 22:08 linuxsoftware

I have never used django-model-utils, but would it not be easier just to use a StatusField with a different name instead of using the StatusModel mixin? e.g.

class DeliveryPage(joyous.models.EventBase):
    delivery_status = StatusField(_('status'))
    delivery_status_changed = MonitorField(_('status changed'), monitor='delivery_status')

etc?

linuxsoftware avatar Aug 11 '20 22:08 linuxsoftware

Closing this issue as I have had no response

linuxsoftware avatar Aug 29 '20 00:08 linuxsoftware

Sorry, I got pulled away.

No, it would not be easier because that breaks interoperability with other libraries that already rely on ModelUtils via model mixins etc. Django Model Utils is an established library that has set a precedent in the Django community that status on a model is generally a CharField that stores values the the DB rather than a property method that calculates a value.

Besides it's much easier to modify the name of a calculated property than to change the name of a model field that is inherited from an installed library, especially once migrations have been run.

I think this simple change is valuable as it improves Joyous compatibility with an existing and well established library and as such improves interoperability with other apps as well, which can only help the uptake of LS.Joyous - which is an excellent library btw!

sam-mi avatar Aug 31 '20 03:08 sam-mi

I have another branch that I could submit a PR from if you think it would be useful?

It's a much larger change - refactoring models as abstract models that can be extended in the final project - no field changes, no migrations, updated tests, custom templates etc, actually quite possibly all non-breaking changes. I have done this to allow me to integrate registration and ticket sales on event pages within my wagtail implementation - which was not possible using the current version of LS.Joyous, at least not while extending my other page types as well.

sam-mi avatar Aug 31 '20 03:08 sam-mi

Where I would like to get to is where there was just one Event type and one Exception type. There is a discussion document on this in the wiki. Code changes towards that goal would be welcome. This will be a major change though, so will need to wait until 2.0.

linuxsoftware avatar Sep 01 '20 00:09 linuxsoftware