willhardy / django-seo

Provides a set of tools for managing Search Engine Optimisation (SEO) for Django sites.
BSD 3-Clause "New" or "Revised" License
252 stars 117 forks source link

Inline Add not working #2

Closed philippWassibauer closed 13 years ago

philippWassibauer commented 13 years ago

When I try to add Metadata inline to an Object I get following error:

duplicate key value violates unique constraint "seo_xyzmetadatamodelinstance__content_type_id_key"

could this be triggered due to the changes made to fix the bug yesterday?


Furthermore I have another bug (I think...or I am doing something wrong):

class XYZMetadata(seo.Metadata): title = seo.Tag(head=True, max_length=72, populate_from=seo.Literal("xxxxxx")) description = seo.MetaTag(max_length=155, populate_from=seo.Literal(u"xxxxxx")) keywords = seo.KeywordTag(populate_from=seo.Literal(u"xxxxxxxx")) class Meta: seo_models = ('event.Event', 'profiles.Profile', 'blog.Post')

The populate_from works good when doing {% get_metadata %}, however it returns nothing when I do {% get_metadata for profile %} although it should. It tells me in the Admin that the default values will be displayed, which is never done actually though.

Thanks for the fast fix on the last bug!

willhardy commented 13 years ago

Thanks for the report!

This isn't in the test suite for some reason, and it doesn't throw an exception when running sqlite. I've cleaned up the tests to get them to run on postgres and will add a new one for this.

I'm not sure how to fix it though: there's a signal to create a metadata object when a new instance on the related model is created, but the admin tries to add the one you just edited inline afterwards.

willhardy commented 13 years ago

The only fix I can think of is to disable the instance adding via post_save signal when an inline is used. The difficulty is then trying to work out which models use the inline.

Another option would be to require an explicit command to switch on automatic metadata creation, with a warning in the docs that it cannot coexist with an inline form in the admin.

philippWassibauer commented 13 years ago

hmmm. can you point me to the code parts that are involved? i am not familiar enough with the application. would it be possible to check if the object was already created? in that case just using the object and editing the fields? kind of like get_or_create?

willhardy commented 13 years ago

Nope, not possible and would be problematic in its own way.

The solution is actually simple: if you use an inline form in the admin, don't put the model in the seo_models setting. That setting will add a new metadata instance automatically, and the inline with do something similar, so there's no need for both.

I've updated the docs to mention it, and fixed some other problems I saw when using inlines in the admin. I hope that this won't be a point of confusion, otherwise I might have to consider removing the seo_models feature.

Thanks for the report! I'm feeling a bit better about the library and maybe even confident enough to push a release to pypi.

Cheers,

Will

willhardy commented 13 years ago

(PS make sure you pull an update, I've fixed something else you might come up against)

philippWassibauer commented 13 years ago

cool. it works to add them as inlines now. I have one more small problem. when I do:

{% get_metadata for profile as metadata %}{{ metadata }} in my templates I never get any data back. This happens when I attached metadata to the object and also when I did not (in that case it should display the default values)

am I doing something wrong?

thanks for the quick responses...it is sunday afterall ;)

willhardy commented 13 years ago

Sorry, I worked on the first part and forgot to come back to the second. I'll look into this now.

willhardy commented 13 years ago

Do you definitely have a get_absolute_url method on your profile object? (will speed up my attempt to reproduce the problem)

willhardy commented 13 years ago

(By the way, you don't have to include the "for profile" bit if you are currently on the profile page, the metadata for the current path is displayed when you don't use the "for obj" syntax)

philippWassibauer commented 13 years ago

i have:

@models.permalink def get_absolute_url(self): return ('location_detail', (), {'username': self.user.username })

in my Profile model. The get_absolute_url matches the url on the page.


url(r'^(?P[\w.-]+)/$', 'xxx.views.location', name='location_detail'),

thats the url pattern.

when I remove "for profile" I get the default values back, but I don't get the values I attached to the model.

a question on the side: why would you get the metadata over the url if you already have the actual businessobject that the metadata is associated to?

willhardy commented 13 years ago

Not sure about this, can you confirm in the admin that the relevant "XYZ Metadata (model instance)" objects have an (accurate) path? Up until earlier today the admin inline wouldn't have added an accurate path.

(side question: for flexibility. you only need to specify the metdata in the base template, and it will work for all pages, even the one's without associated objects (like when you link metadata to views or raw paths). The value can also be cached against the path, so there doesn't have to be a performance hit)

Thanks for the feedback!

willhardy commented 13 years ago

when I remove "for profile" I get the default values back, but I don't get the values I attached to the model.

I'm guessing that this one has to do with not using RequestContext. I've added a strict check with a friendly message to help people out who fall into this trap.