richardbarran / django-photologue

A customizable plug-in photo gallery management application for the Django web framework.
BSD 3-Clause "New" or "Revised" License
675 stars 239 forks source link

Setting a canonical site for multi-site photos/galleries #135

Closed b-jam closed 9 years ago

b-jam commented 9 years ago

As per Google, if content is on multiple sites, it should be only in one sitemap. https://support.google.com/webmasters/answer/66359?hl=en

However, when using PHOTOLOGUE_MULTISITE and the default sitemaps, the content will go into each selected sites sitemap.

If I submit a pull request for this, will it be accepted?

It would most likely be to add another 1-to-1 field primary_site, which is invisible unless using multisite. This field would be queried in get_absolute_url, and in the sitemap.

richardbarran commented 9 years ago

Optimising content for Google is a Good Thing, so I am in favour of your idea.

As you suggest, adding a new field to Gallery and to Photo is probably to neatest approach; I would call the field 'canonical_site', simply so that the terminology is similar to what Google uses.

You say that it would be a 1-to-1, but surely it would be a foreign key to the Site table?

I'm not sure about using it in get_absolute_url - this method should return a url for the current site, even if it's not the canonical site. Maybe an optional argument would be a good approach:

get_absolute_url(canonical=False)

Anyway, just some thoughts on my part! I look forward to your pull request.

b-jam commented 9 years ago

Very good points, I agree. I'll just make another method get_canonical_url which returns the canonical url.

b-jam commented 9 years ago

So I've done most of the work, see my fork. I just have a couple questions. 1) I've set canonical_site( null=True... ) but im not sure whether we should have a data migration, it only makes sense to migrate for users not using MULTISITE as otherwise which site do you choose as the canonical? 2) all the existing ones tests are passing, should I add more tests?

richardbarran commented 9 years ago

Closing this - replaced by pull request #136.