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

Abstract models containing ImageSpecFields are not handled properly #185

Merged
merged 1 commit into from Feb 10, 2013

Conversation

seanbell
Copy link
Contributor

@seanbell seanbell commented Feb 9, 2013

One of my models is abstract and includes several ImageSpecField fields. Calling ./manage.py generateimages leads to an exception in imagekit/specs/sourcegroups.py at the line:

    for instance in self.model_class.objects.all():

The attached commit fixes this particular exception, but I don't think it addresses all the potential issues involved with abstract base classes. For example, the generator registry only includes one entry corresponding to the abstract base; it may make sense to instead list it for all the descendants that correspond to non-abstract models.

matthewwithanm added a commit that referenced this pull request Feb 10, 2013
Includes a fix for undispatched signals, as well as signals being
handled twice.

A regression of #126
Related: #185
@matthewwithanm matthewwithanm merged commit af6ebcb into matthewwithanm:develop Feb 10, 2013
@matthewwithanm
Copy link
Owner

Thanks @seanbell! I think you found an issue not only with abstract models, but also model subclasses in general. However, I think your get_nonabstract_descendants() didn't quite solve it since it only returned the first non-abstract descendants, and not their descendants. (I added a test to illustrate.)

As for your concern about the generator registry, all the handling of sources is done by the source groups, so I don't think we have any problems there. The source group just needs to be aware of the subclasses. I think we should be good with the commits I just pushed.

I've bumped the version to 3.0a3. Thanks so much for the report and the PR! Let me know if it works for you now.

@seanbell
Copy link
Contributor Author

Great! It seems to work for me.

@matthewwithanm
Copy link
Owner

Fantastic! Thanks again for your help!

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.

None yet

2 participants