RouteViewHandler check raised on Route

[meshy]

The check ensures that Route models handled by RouteViewHandler have a view attribute.

Unfortunately, it's failing on Route itself, instead of on just the subclasses which will actually have instances.

As we should never really have a Route instance in the DB, it shouldn't need a view attribute.

[meshy]

I'm not 100% about how best to solve this. Three options occur to me:

1. Skip the check when the class is Route.
2. Add a flag to Route that allows the check to be skipped. (How would we automatically change this for subclasses? A @property?)
3. Skip the check when the class is a direct subclass of PolymorphicModel (ie: a parent model).

None of these seem quite right, but I think, for simplicity, the first option may be best. Perhaps there's something deeper wrong somewhere.

[LilyFoote]

A few thoughts:

• Option 1 seems like an easy fix.
• If part of the problem is a Route can end up in the database, perhaps Route should be an abstract model?
• Another option is to remove RouteViewHandler from Route and require users to add it to subclasses.

[meshy]

Hey :)... Thanks for chipping in.

If part of the problem is a Route can end up in the database, perhaps Route should be an abstract model?

Yes, that is surely a state we should be avoiding. Unfortunately, I don't think we can do abstract base classes, and still benefit from using django-polymorphic. Perhaps it already has a mechanism to stop this?

Another option is to remove RouteViewHandler from Route and require users to add it to subclasses.

This is worth considering.

EDIT: Oops, submitted comment too early.

The downside of course, is that of introducing additional boilerplate... and in the case of making it a required attribute, we would probably benefit from a check to ensure that the field had been applied, and be left in a similar situation to where we are right now.

[meshy]

Ok, just for the sake of progress, I'm going to go with option 1.

What's more, you're right -- it does seem like an easy fix :)

[meshy]

Perhaps this is just procrastination, but I'm not sure it's a good idea.

I know it's easy... but it doesn't make it feel right:

if type(instance) is Route:
    ...  # This feels wrong...

[LilyFoote]

I think we can pick one and see how it works. I think we can still change our mind.

[meshy]

I elected to make the decision to skip the checks on the Route class instead of the handler. It doesn't feel even nearly as dirty this way. I might even say I'm happy with it.

@Ian-Foote what do you think?

[meshy]

Thanks :)