google-code-export / django-page-cms

Automatically exported from code.google.com/p/django-page-cms
BSD 3-Clause "New" or "Revised" License
0 stars 0 forks source link

Content objects #212

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1. Settings PAGE_CONTENT_REVISION to True.
2. Changing slugs for a page.
3. Using than the old slug (which is stored as an old revision) to get to the 
new page.

What is the expected output? What do you see instead?
Well, it does not look well:
Page "support" has slug "support", but can be available via "porn" slug, 
because, time ago she had that slug. Content.objects.filter(type="slug", 
page=support) can prove that. If i turn of revisions, they are not stored, 
hopefully.

Are you using the master version or a released version of this CMS on the
github repository?
Master

If you can write a test that reproduce the problem, there is better chance
it will be resolved quickly.
See above, described already.

Original issue reported on code.google.com by klich...@gmail.com on 12 Oct 2010 at 11:34

GoogleCodeExporter commented 9 years ago
The issue that you describe comes from this function:

http://github.com/batiste/django-page-cms/blob/master/pages/managers.py#L254

It's probably possible to change this function so the older slugs are not used 
to find the pages. But I think a additionnal SQL statement is needed.

In the other hand, the fact that the page is still reachable with an older slug 
can actually be a very desirable property. Especially if somebody linked on 
your page and you decide to change the slug. The link will continue to work.

What would be the ideal solution for you? What about special command the 
flushes the old slugs?

Original comment by batiste....@gmail.com on 13 Oct 2010 at 11:17

GoogleCodeExporter commented 9 years ago
Yes, this can be treated as a feature, not as a bug, reaching site within the 
old urls, for indexing purposes by search-engine. One thing, that scares me is 
that count of Content objects can grow in uncontrolled way. The heap of old 
revisions is unmanaged, as i can see, this gives me very bad feeling. This 
should be limited to some depths of revisions or revisions should have some 
kind of management interface (should be removable?).
I see that i am possible to view revisions of content, but not slug & title. It 
would be preferable. 

I suggest you to provide PAGE_CONTENT_REVISION_DEPTH to limit the number of 
revisions (default to 3-4).

One more situation, that i haven't investigated: if i change the slug of the 
page from 'old' to 'new', and it's still available within the 'old' one, if i 
set another page to  slug 'old', which of them will be available? If the page 
with old, cached slug will override the new one, that is very bad. I don't 
revisions for slugs at all. It could be just a unique SlugField and that's all.

Original comment by klich...@gmail.com on 13 Oct 2010 at 1:05

GoogleCodeExporter commented 9 years ago
You are indeed right, everything is kept right now. A 
PAGE_CONTENT_REVISION_DEPTH will be indeed welcome. The implementation shoudn't 
be more than 10 lines of code.

For the slug collision you describe. I am pretty sure the CMS does the right 
thing because I extensively tested this corner case:

http://github.com/batiste/django-page-cms/blob/master/pages/managers.py#L114

Did you spotted any bug?

Original comment by batiste....@gmail.com on 13 Oct 2010 at 1:25

GoogleCodeExporter commented 9 years ago
Oh, it's good that you reminded me. Here comes one more issue :) About exactly 
that code, you've pointed me, but about another side of it.

Original comment by klich...@gmail.com on 13 Oct 2010 at 1:34

GoogleCodeExporter commented 9 years ago
I implemented the setting. It was indeed quite easy:

http://github.com/batiste/django-page-cms/commit/24cdafbf4975b64f1dfab72f9ba7577
1f3b2b5b5

Original comment by batiste....@gmail.com on 13 Oct 2010 at 1:58

GoogleCodeExporter commented 9 years ago
Nice, an alternative place to implement this can be save() for Content model to 
prevent adding extra-old content not through the site, but manually.

Original comment by klich...@gmail.com on 13 Oct 2010 at 2:09