BlogPost extension

[wesley]

Hi,

I'm a new mezzanine user (great framework !), I would like to create a
new class of BlogPost with some custom fields.
Can we just inherit from BlogPost class and create an admin class
(like Page in the documentation/tutorial) ?

If not what is the best way to accomplish that ?

Thanks for your help.

[Stephen McDonald]

Subclassing the Page model is part of Mezzanine's API for creating content types, while the blog app is fairly stand-alone and not really designed to be customized outside of its templates. If all you wanted to do is add some fields and have them exposed in templates, subclassing would work.

Having said all that, I've been playing around with Django's class_created signal and it looks like if you time everything correctly, you can successfully inject fields onto an existing model without having to subclass it. The beauty of this is that you would avoid having a separate database table for your extra fields, and the performance penalty that would occur on the SQL JOINS when accessing those fields.

I haven't explored it fully but I have a working example (not integrated with Mezzanine, just a regular Django project) that looks like this:

* Create an empty app and ensure that it's the first app in the list of INSTALLED_APPS. Mezzanine does some messing around with the order of INSTALLED_APPS so I'm not sure if that's possible with Mezzanine in its current state. The order is important as the class_created signal handler needs to be registered before the models you wish to custom are set up by Django.

* In the empty app's __init__.py module, set up the signal handler as follows:

    from django.db.models.signals import class_prepared

    from django.db import models

    def add_field(sender, **kwargs):

        if sender.__name__ == "BlogPost":

            custom_field = models.CharField(max_length=200)

            custom_field.contribute_to_class(sender, "custom_field_name")

    class_prepared.connect(add_field)

So with that set up as the first app, the signal handler will be run when Django loads each app in INSTALLED_APPS. We need to check the model name rather than importing the model outside of the handler and using it as the sender arg to the connect function, since importing it would then cause the model to be set up by Django prior to the signal handler being registered, and as such it would never be run for the model we're interested in.

Like I said I haven't fully explored this, but at first glance it looks like a much better approach than subclassing for customising models in libs outside of your project. One area where I think this might fall down is with South migrations, but it really needs more exploration. 

If this does turn out to be a feasible approach to customising models in Mezzanine, what I might try and do is provide some hooks for specifying a mapping of custom fields to models you'd like to apply, and then have Mezzanine deal with the setup I described above. Anyone else have any thoughts on this?

--
Stephen McDonald
http://jupo.org

[Stephen McDonald]

Sorry I called the signal class_created when I meant class_prepared as in the code snippet.

Also with regard to admin classes, yes the usual approach of unregistering an existing admin class, subclassing it and registering your own still applies.

[Josh Cartmell]

I like the way you have done it and definitely like that it is more efficient.  I have subclassed BlogPost in the past to add a field and it works just fine but isn't the most efficient.

I have been trying to figure out if it would be possible to do this with mixins.  For example in an empty django project I have created apps app1 and app2 (they are in that order in settings).  They look like the following:

app1.models.py:

from django.db import models
from app2.models import BlogPost

class BlogPostExtend:
    custom_field_name = models.CharField(max_length=200)
   
    class Meta:
        abstract = True

BlogPost.__bases__ += (BlogPostExtend,)


app2.models.py:

from django.db import models

class BlogPost(models.Model):
    field_name = models.CharField(max_length=100)


Unfortunately this does not result in custom_field_name being created in the database when I syncdb, although at the command line if I type BlogPost.custom_field_name it does recognize it as a CharField.

Do you have any ideas on how to make this work Steve, or anyone?

I thought that mixins could have been an easier solution than signals but I guess I may have been wrong.

Regards,
Josh

[Stephen McDonald]

I don't think mixins/signals are mutually exclusive in the solution. 

One of the reasons why my snippet above works is that I don't actually import the BlogPost class. I'm only speculating as I'm not familiar enough with Django's internals, but I believe the all the setup for a model happens when the definition for its class is executed. 

So in your example, you've imported the BlogPost class, as Python executes its class definition (the class statement itself) all of Django's setup for the model occurs, and *after* that has happened, you add to its bases, so the extra fields never make it into the class early enough for them to be part of the model's definition.

In my snippet I originally import the BlogPost class to use as the sender argument when calling connect for the signal, but then the signal never was called for the BlogPost class, because class_prepared occurs as part of Django's model setup I keep referring to, and when I import BlogPost all that executes prior to the signal handler being connected.

So your approach would work if you don't import BlogPost anywhere and in order to do that you need to make use of the class_prepared signal where the BlogPost class will actually be available as the sender, you just need to check the sender's name and the modify bases as you're doing. At that point the difference in our approaches boils down to a choice between class hacking and using Django's API on fields - contribute_to_class. Once you get down to that stage I think the latter is cleaner.

Here's my initial attempt at having this all done for you in Mezzanine via a setting: https://bitbucket.org/stephenmcd/mezzanine/changeset/267179d932da

So you simply define the setting EXTRA_MODEL_FIELDS with the required info for what fields to add to what models with what args and those fields will get injected onto the specified models:

EXTRA_MODEL_FIELDS = (

    ("mezzanine.blog.models.BlogPost.field_name", "CharField", {

        "verbose_name": _("Name"), "max_length": 200, "default": "value"

    }),

    ("mezzanine.pages.models.Page.another_field", "IntegerField", {

        "verbose_name": _("Another name"), "blank": True, "default": 1

    }),

)

All feedback welcome!

[Josh Cartmell]

Thanks for the explanation, that makes sense.  I agree that the signals vs. mixin don't have to be mutually exclusive, but I was looking for a solution which didn't use signals because I guessed that from a performance standpoint not using signals would be better.  I'm not sure about that and am trying to find something definite one way or the other now.

I also like using the mixin vs the django contribute_to_class (via signals or otherwise) because a mixin allows you to add arbitrary class methods which could definitely prove useful, especially if you are adding a new field.

I imagine that we could have an optional user creatable file called model_mixins.py (maybe needs a different name, but similar to how users can create page_processors.py) within which you could define model mixins which would really just be full django models.  There could be a decorator at the top of each model called mixin_for (or whatever name seems appropriate) which would specify the model it is to be mixed into, connect to the signals and mix the model in.  Then we would need to have an autodiscover() similar to the one page_processors have which would be called before apps are loaded.

Any thoughts on that?

Regards,
Josh

[Stephen McDonald]

This is a really clever idea definitely worth some more discussion, so let me play devil's advocate for a moment.

I guess there's a bit of a worry around MRO with classes - lots of Mezzanine classes are built up using multiple inheritance, and if the inheritance order is incorrect then things fall apart entirely. So I can imagine some scenarios where people would use this mixin feature and things would break. It might work in most cases, but not in some. Do we want to expose an API that requires careful planning in order to avoid problems like these? 

You're absolutely right in that this approach would cover a much broader level of customization, particularly with being able to inject methods onto models as well, but I feel like the most common case for customizing models definitely falls under the category of adding additional fields to the model and exposing them through the admin.

So to me the two approaches could be summed up as:

EXTRA_MODEL_FIELDS setting: A very granular and explicit API that covers off the main use case for customizing models, with not much room for error.

Mixins decorator: A very broad API that allows you to customize models in any way you can imagine, with some potential caveats that could bite you in the ass.

I really like the mixins idea, but I'm leaning towards the former approach from the perspective of simplicity, explictness and control. Perhaps we could implement both? I'm not sure. I imagine that the mixins approach could still be set up manually by a developer in their own project, and that approach would be more geared towards the advanced developer who's comfortable working our the correct MRO and associated hand-holding around multiple inheritance. This is in contrast to the less experienced developer who just wants to extend the models with some extra fields, which defining a single setting is much more geared towards.

Incredibly keen to hear more thoughts on this! This is really exciting and either way this is gonna be a great step forward in the ability to customize Mezzanine. 

[Eli Spizzichino]

+1 for contribute_to_class

I think is so cool and simple with your boot app!

This way we may even create an app that alter the dictionary for the model to extend...  

in other words, like forms app lets you create (extend) new forms, this may let you create (extend) new models very easily

wow

2011/10/14 Stephen McDonald <st...@jupo.org>

--
~ Eli ~

[j_syk]

This thread seems well timed as I was just about to try to figure out
a way to add an image field to each blog post. I think that I will try
your boot app method to inject an image field.

I am referring to a 'title image" so to speak, one that would be
relatively unique and displayed on the blog post list and top of
posts. Or is there another method of doing this?

This seems like a pretty common feature on many blogs and is desired
my some of the users of my sites. Have you considered adding it to the
project as a default on the blog model? I could see how it would be
clutter for the majority of people who wouldn't use it. Having
EXTRA_MODEL_FIELDS in the settings would be an awesome solution.

> ...
>
> read more »

[Josh Cartmell]

This is kind of a longer email so I will include a short summary at the bottom.

I agree that MRO could be problematic, I was experiencing that myself as I tried to get a mixin solution working.

You can work around it (at least sometimes) by doing things like this:
try:
    BlogPost.__bases__ = (mixin,) + BlogPost.__bases__
except: # would want to catch the right exception, I'm just not sure what it is off the top of my head
    BlogPost.__bases__ = BlogPost.__bases__ + (mixin,)

But I'm not super familiar/good at MRO myself so that could fail sometimes.

This may be a bit of a moot point anyways as I have not been able to find a way to get a mixin to add a field to a a model anyways.  Even when I put the mixin inside the class_prepared signal and use the sender rather than importing the model I am mixing into, the field does not end up in the database.

After reading the docs this does make some sense as it says class_prepared is sent "once model has been defined and registered with Django's model system"
-https://docs.djangoproject.com/en/dev/ref/signals/#class-prepared

With that in mind I actually think the use of signals with contribute_to_class may be unnecessary.  I have made simple test project (not mezzanine, just django) which has two apps, app1 and app2.

app1.models.py:

from django.db import models
from app2.models import BlogPost

# note I do not connect to the class_prepared signal
custom_field_name = models.CharField(max_length=200)
custom_field_name.contribute_to_class(BlogPost, "custom_field_name")

app2.models.py:

from django.db import models

class BlogPost(models.Model):

    new_field = models.CharField(max_length=100)


Both have empty __init__.py and I have made no other modifications to the code.

Regardless of the order that I put them in INSTALLLED_APPS custom_field_name always ends up in the database. 

I then tried putting app1 inside a fresh mezzanine project (mezzanine from pypi within the past few days).  I import BlogPost from mezzanine.blog rather than app2.  Again custom_field_name ended up in the database regardless of the order I put app1 in INSTALLED_APPS (I tried putting it first and last) without using the class_prepared signal.  I think the signal may be unnecessary and a bit misleading as it isn't even called until after the model is registered (if the docs are right).

I am still curious if there is a way to get mixins to add a field but I think it may not be easily possible.  I have seen this ticket which may be related https://code.djangoproject.com/ticket/5685 but it is still open.

I still like the idea of writing normal models with a decorator to specify which model they should be added to and I think it could still work similar to the idea I posted before.  The difference would be that instead of having each model mixed in, each models fields could be looped over and then have contribute_to_class applied to each field.  At that point I think it becomes a matter of preference and I prefer to write models as I always have instead of having to learn more syntax to put in settings (not that the way you have done it is incredibly difficult, I just would prefer uniformity if possible).  It may even make sense to not even look for the model_mixins.py file in every app but just check if it exists in the root of the project and document that that is where it belongs.

A final thought would be that as you loop over each field to be contributed you could remove it from the model (I'm not exactly sure how to do that).  Once all fields have been contributed to the class you could change the models inheitance set it to inherit from nothing, or object (something that wouldn't be an MRO problem or maybe just inherit from object in the first place).  Then with no fields left you could mix the model/class into the appropriate model thereby inheriting any class methods.  The decorator could/should have a boolean switch that would tell whether to do the final mixin.

Summary:

-I think the class prepared signal is unnecessary.

-Mixins do not appear to be capable of adding fields to a model.

-My proposal/idea:
Create a model_mixins.py file in the root of the project directory.  In that file write normal models preceded by a decorator to specify the model they should be added to and whether there are any new class methods.  The decorator would loop over the fields in the model and contribute them to the model specified.  As the decorator loops it would also remove the fields from the model.  After the loop is finished and if the decorator specified new class methods mix the model into the specified model thereby gaining any class methods.

Regards,
Josh

[Stephen McDonald]

Thanks for your thoroughness Josh. That's really interesting around getting it to work without the signal.

I might try some things out around your idea. Rather than using a decorator I'm thinking you could just create your model mixins by inheriting from a Mixin class defined in Mezzanine, and then having that base Mixin class do all the setup, whatever that turns out to be. Really like the idea bout reading the fields from that class and then copying them over to the model you're mixing into. So something like:

from django.db import models

from mezzanine.utils.models import Mixin

from mezzanine.blog.models import BlogPost

class my Mixin(Mixin):

    some_field = models.CharField()

    mixin_for = BlogPost

... or with an inner class following the style of Django models...

    class Meta:

        mixin_for = BlogPost

... then the actual definition for Mixin would probably do some metaclass stuff to do all the work.

Definitely needs more playing around with.

[Stephen McDonald]

Of course that typo should have been: class MyMixin(Mixin):

[Stephen McDonald]

Ok here's an initial attempt: 

https://bitbucket.org/stephenmcd/mezzanine/changeset/cf1f0d1f95d5

This seems to work for the following:

from django.db import models

from mezzanine.blog.models import BlogPost

from mezzanine.utils.models import ModelMixin

class BlogPostMixin(ModelMixin):

    image = models.ImageField("Image", upload_to="blog")

    class Meta:

        mixin_for = BlogPost

    def foo(self):

        return "bar"

>>> p = BlogPost.objects.all()[0]

>>> p.image

<ImageFieldFile: None>

>>> p.foo()

'bar'

My gut feeling is that this is a major hack that's bound to break in some way down the line.

[Josh Cartmell]

That sounds pretty good.  So I'm thinking that setup could look something like this:

for field in AdditionalFieldsModel._meta.fields[:]:
    field.contribute_to_class(ModelToAddTo, field.name)
    AdditionalFieldsModel._meta.fields.remove(field)

# specifies if there are new class methods to mixin
# probably should be specified in AdditionalFieldsModel's meta or somesuch
if additional_class_methods:
    ModelToAddTo.__bases__ += (AdditonalFieldsModel,)

More work may be need to done as far as MRO goes, but I'm not sure and think it would depend on the Mixin class.  I'm guessing it would inherit from models.Model but possibly having Mixin inherit directly from object could mitigate those issues?

How would you have the setup be performed by the parent Mixin class?

Regards,
Josh

[Josh Cartmell]

I see you beat me to it. I don't totally understand what you did as
I'm not super familiar with Python classes. I'm trying to read up on
them more and starting to see what you did. Whether or not it is
hackish it looks like a good place to start and I think could be
useful to other Django projects, beyond just Mezzanine.

Regards,
Josh

> >     app1.models.py <http://app1.models.py>:

>
> >     from django.db import models
> >     from app2.models import BlogPost
>
> >     # note I do not connect to the class_prepared signal
> >     custom_field_name = models.CharField(max_length=200)
> >     custom_field_name.contribute_to_class(BlogPost, "custom_field_name")
>

> >     app2.models.py <http://app2.models.py>:

>
> >     from django.db import models
>
> >     class BlogPost(models.Model):
> >         new_field = models.CharField(max_length=100)
>
> >     Both have empty __init__.py and I have made no other modifications
> >     to the code.
>
> >     Regardless of the order that I put them in INSTALLLED_APPS
> >     custom_field_name always ends up in the database.
>
> >     I then tried putting app1 inside a fresh mezzanine project
> >     (mezzanine from pypi within the past few days).  I import BlogPost
> >     from mezzanine.blog rather than app2.  Again custom_field_name
> >     ended up in the database regardless of the order I put app1 in
> >     INSTALLED_APPS (I tried putting it first and last) without using
> >     the class_prepared signal.  I think the signal may be unnecessary
> >     and a bit misleading as it isn't even called until after the model
> >     is registered (if the docs are right).
>
> >     I am still curious if there is a way to get mixins to add a field
> >     but I think it may not be easily possible.  I have seen this
> >     ticket which may be related

> >    https://code.djangoproject.com/ticket/5685but it is still open.

> >>     On Oct 14, 6:12 am, Eli Spizzichino<scr...@gmail.com>  <mailto:scr...@gmail.com>  wrote:

> >>>     +1 for contribute_to_class
> >>>     I think is so cool and simple with your boot app!
> >>>     This way we may even create an app that alter the dictionary for the model
> >>>     to extend...
> >>>     in other words, like forms app lets you create (extend) new forms, this may
> >>>     let you create (extend) new models very easily
> >>>     wow
>

> >>>     2011/10/14 Stephen McDonald<st...@jupo.org>  <mailto:st...@jupo.org>

>
> >>>>     This is a really clever idea definitely worth some more discussion, so let
> >>>>     me play devil's advocate for a moment.
> >>>>     I guess there's a bit of a worry around MRO with classes - lots of
> >>>>     Mezzanine classes are built up using multiple inheritance, and if the
> >>>>     inheritance order is incorrect then things fall apart entirely. So I can
> >>>>     imagine some scenarios where people would use this mixin feature and things
> >>>>     would break. It might work in most cases, but not in some. Do we want to
> >>>>     expose an API that requires careful planning in order to avoid problems like
> >>>>     these?
> >>>>     You're absolutely right in that this approach would cover a much broader
> >>>>     level of customization, particularly with being able to inject methods onto
> >>>>     models as well, but I feel like the most common case for customizing models
> >>>>     definitely falls under the category of adding additional fields to the model
> >>>>     and exposing them through the admin.
> >>>>     So to me the two approaches could be summed up as:
> >>>>     EXTRA_MODEL_FIELDS setting: A very granular and explicit API that covers
> >>>>     off the main use case for customizing models, with not much room for error.
> >>>>     Mixins decorator: A very broad API that allows you to customize models in
> >>>>     any way
>

> ...
>
> read more »

[Stephen McDonald]

I've finally gotten around to finishing this off - the result is here:

https://github.com/stephenmcd/mezzanine/commit/fb0f669ea54197b7cf96c4313cd747dc0795a851

I've implemented the EXTRA_MODEL_FIELDS setting as previously described in this thread, and documented it as a first-class citizen in Mezzanine with some warnings around the caveats involved with using migrations.

The code I also posted earlier in this thread using a mixin approach is committed in mezzanine.utils.models.ModelMixin, but I haven't documented it in any way as I wasn't able to get it to work at all with migrations.

New 0.12 release coming shortly!

[Pushkar Paranjpe]

Correct me if I am missing something. Relatively new to Mezzanine and to Django :)

I managed to add a custom field header_image to the BlogPost model using the EXTRA_MODEL_FIELDS approach.

Just have to add one note in case someone else finds this useful:

If the DB already exists and then the above extra field is injected,

The following command does not pick it up:
python manage.py schemamigration theme --auto

I needed to delete the dev.db, clean-up the migration history and run the following command for the custom fields to be incorporated into the BlogPost model:
python manage.py createdb --noinput

(then ...
python manage.py schemamigration theme --initial
python manage.py migrate theme --fake ... )

[Pushkar Paranjpe]

Update:

A cleaned-up work-flow:

1. Add custom fields using the EXTRA_MODEL_FIELDS in settings.py
2. Update respective model's fieldsets in admin.py
3. delete dev.db
4. delete all migrations .py under your custom theme app
5. python manage.py createdb --noinput
6. python manage.py syncdb
7. python manage.py migrate
8. Login to admin and Go to blog posts section and add new post with your custom fields :)

NOTE: Of course, you will lose all previous blog posts, pages data that was entered through the cms earlier. You will have to re-add that stuff.
If anyone knows how to automate this part, given the data to be entered that would be very valuable :)

Cheers!
Pushkar

[Josh Cartmell]

Hi Pushkar, for you and anyone else who comes across this, you do not have to delete your db and migrations to add fields from EXTRA_MODEL_FIELDS

The correct way to add the image field to BlogPost would be:

python manage.py schemamigration mezzanine.blog --auto --stdout > path/to_store_the/migration_file.py

python manage.py migrate app_migrations_were_stored_in

Good luck!

--
You received this message because you are subscribed to the Google Groups "Mezzanine Users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to mezzanine-use...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Pushkar Paranjpe]

Hi Josh,

Deleting the db and migrations each time I add fields to a core mezzanine model did feel a bit drastic and was killing the idea of South.
Thanks for your corrections. I will use them right away!

By using your approach, it looks like the old entries in the DB will still be intact, right ? (awesome!)

Cheers!
Pushkar

[Pushkar Paranjpe]

Hi Josh,

Deleting the db and migrations each time I add fields to a core mezzanine model did feel a bit drastic and was killing the idea of South.
Thanks for your corrections. I will use them right away!

By using your approach, it looks like the old entries in the DB will still be intact, right ? (awesome!)

Cheers!
Pushkar
On Monday, 30 March 2015 21:09:16 UTC+5:30, Josh Cartmell wrote:

[Josh Cartmell]

Yep!  The approach I mentioned should leave existing entries intact

[Pushkar Paranjpe]

Hi Josh,

I am getting the following error when I ask it to migrate:

Traceback (most recent call last):
  File "manage.py", line 28, in <module>
    execute_from_command_line(sys.argv)
  File "/usr/lib/python2.7/dist-packages/django/core/management/__init__.py", line 399, in execute_from_command_line
    utility.execute()
  File "/usr/lib/python2.7/dist-packages/django/core/management/__init__.py", line 392, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
  File "/usr/lib/python2.7/dist-packages/django/core/management/base.py", line 242, in run_from_argv
    self.execute(*args, **options.__dict__)
  File "/usr/lib/python2.7/dist-packages/django/core/management/base.py", line 285, in execute
    output = self.handle(*args, **options)
  File "/usr/local/lib/python2.7/dist-packages/South-1.0.2-py2.7.egg/south/management/commands/migrate.py", line 81, in handle
    apps = [Migrations(app)]
  File "/usr/local/lib/python2.7/dist-packages/South-1.0.2-py2.7.egg/south/migration/base.py", line 64, in __call__
    self.instances[app_label] = super(MigrationsMetaclass, self).__call__(app_label_to_app_module(app_label), **kwds)
  File "/usr/local/lib/python2.7/dist-packages/South-1.0.2-py2.7.egg/south/migration/utils.py", line 41, in app_label_to_app_module
    app = models.get_app(app_label)
  File "/usr/lib/python2.7/dist-packages/django/db/models/loading.py", line 190, in get_app
    raise ImproperlyConfigured("App with label %s could not be found" % app_label)
django.core.exceptions.ImproperlyConfigured: App with label py could not be found

Please let me know if you have any clues to the cause of the error.

Cheers!
Pushkar

[Pushkar Paranjpe]

OK! Please ignore that last error message. I was doing something really silly.

Having defined the "defaults" for all the non-null fields I ran:

python manage.py schemamigration mezzanine.blog --auto --stdout > theme/migrations/0001_blog_customization.py

then

python manage.py migrate theme

(Also updated the blog_fieldsets in the admin.py)

And it all worked marvellously well! Columns were added to the DB table (checked with dbshell (PRAGMA table_info(blog_blogpost);) ), admin page for blog post is also rendering all the extra fields.

Thanks again.

Pushkar

[Josh Cartmell]

Great, glad to hear you figured it out Pushkar!