Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Making Entry Model swappable using django-swappable-models. #507

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

divick
Copy link

@divick divick commented Apr 15, 2017

* With this change, there is no need for the app extending
  Entry model to provide workarounds like MIGRATION_MODULES
  or ZINNIA_ENTRY_BASE_MODEL.
* The app just needs to make a dependency in the migration
  to zinnia by adding

  ('zinnia', '0001_initial'),

  to depencies section in the generated migration. After that
  first zinnia:0001_initial migration is run, followed by
  the migration generated in the app extending the Entry model.
  And then rest of the migrations defined in zinnia/migrations/
  are run.

  This way the migrations are generated in the correct place.

What is the purpose of your pull request?

  • [491 ] Bug fix
  • As explained in the commit log as well as above, using django-swappable-models, there is no need to provide workarounds to generate the migrations in your own app to prevent generating it in zinnia package. The migrations are generated in the app extending the Entry model and the dependencies are
    correctly run. There is even no need to define the ENTRY_BASE_MODEL as this change obviates the need for doing that.
  • Fixed the test cases so that they work with the new changes.

Proposed changes

  • The documentation needs to be modified about how to extend the EntryModel or the AbstractEntryModel.

Warning

Before submitting a pull request make sure you have:

divick added 2 commits April 15, 2017 14:59
    * With this change, there is no need for the app extending
      Entry model to provide workarounds like MIGRATION_MODULES
      or ZINNIA_ENTRY_BASE_MODEL.
    * The app just needs to make a dependency in the migration
      to zinnia by adding

      ('zinnia', '0001_initial'),

      to depencies section in the generated migration. After that
      first zinnia:0001_initial migration is run, followed by
      the migration generated in the app extending the Entry model.
      And then rest of the migrations defined in zinnia/migrations/
      are run.

      This way the migrations are generated in the correct place.
@coveralls
Copy link

coveralls commented Apr 15, 2017

Coverage Status

Coverage decreased (-0.3%) to 99.627% when pulling 5129b7c on divick:swappable-models into 274910c on Fantomas42:develop.

    * The load_model_class is now not being used.
    * The user can define the ZINNIA_ENTRY_MODEL = 'myblog.Entry'
      to swap his/her own Entry model with default zinnia's Entry
      model with the swapped model extending from AbstractEntry
      model or can define his/her own AbstractEntry model and
      extend from that model. Only redefining AbstractEntry model
      is not sufficient. If one is redefining AbstractEntry model,
      then he/she also needs to define his/her own Entry model
      which needs to be swapped. This is because changing the
      AbstractEntry only changes the schema for the default
      Entry model and thus when generating migration, they are
      generated by default in zinnia's own migrations folder.

      Summary:

       -- If you simply want to add extra fields to Entr model,
          then simply define ZINNIA_ENTRY_MODEL = 'myblog.Entry'
          in settings, and define Entry model with additional
          fields. Run makemigrations and set dependency on
          zinnia_0001 in generated migration. The migration
          is generated in your own app and not in zinnia
          and the dependencies are properly set.
       -- If you want to override some of the fields defined
          in AbstractEntry, then redefine it in your models,
          and also define your Entry model extending from this
          AbstractEntry. And follow rest of the instructions
          in step1.
@coveralls
Copy link

coveralls commented Apr 15, 2017

Coverage Status

Coverage increased (+0.0004%) to 99.898% when pulling fa5446b on divick:swappable-models into 274910c on Fantomas42:develop.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants