Let me describe a problem:
I have opened a 2 sites pages:
1st: admin/content/book/234 (this is a structure of new book)
2nd: node/137/outline (This is a top level book page)

Then in 2nd window I set new parent of this book page to page with id=234 (the same as I see in 1st window).
I have saved a changes in 2nd window and switched to window 1.
There is no changes visible in window 1 because I didn't refresh a page (Just forgot) so I didn't saw a new subpages...
I have changed a title of one of the pages and saved changes.

Page was reloaded and I have seen a book pages without titles!
(To fix this: switch to previous version of page)

Who I think It should work:
I pages was not listed at the book's structure page (admin/content/book/234) book.module shouldn't change a title.
OR
Just write a warning like "structure was modified by other user... You should reload page"

I'm using this module a lot and want to say "Thanks for a great module".

CommentFileSizeAuthor
#10 lock-form-272900-6x-10.patch2.25 KBpwolanin
#6 lock-form-272900-6.patch2.25 KBpwolanin
#2 lock-form-272900-2.patch2.11 KBpwolanin

Comments

pwolanin’s picture

pwolanin commented
Version: 6.2 » 7.x-dev

Right - it sounds like this is a bug in the form submit handler on the admin screen. It should be fixed in 7.x first. I'm not sure how we should handle the lockout. Maybe the hash of the menu tree?

pwolanin’s picture

pwolanin commented
Status: Active » Needs review
StatusFileSize
new lock-form-272900-2.patch2.11 KB

Here's a starting patch - saves the hash of the tree structure as a hidden value and compares it to the current value. Please test, and comment on the error message.

note- I found a serious bug in book at the same time: http://drupal.org/node/276846

pwolanin’s picture

pwolanin commented

for comparison the error message node module throws is:

form_set_error('changed', t('This content has been modified by another user, changes cannot be saved.'));
dmitrig01’s picture

dmitrig01 commented

I think we should keep this consistent with node.module, so once the message is changed I'll RTBC - I testred and it works as advertised.

dmitrig01’s picture

dmitrig01 commented
Status: Needs review » Needs work
pwolanin’s picture

pwolanin commented
Status: Needs work » Needs review
StatusFileSize
new lock-form-272900-6.patch2.25 KB

Ok, revised the error text and added another code comment.

dmitrig01’s picture

dmitrig01 commented
Status: Needs review » Reviewed & tested by the community

Tested, works well, and patch makes sense.

dries’s picture

dries commented
Status: Reviewed & tested by the community » Fixed

Committed to CVS HEAD. Thanks.

pwolanin’s picture

pwolanin commented
Version: 7.x-dev » 6.x-dev
Status: Fixed » Reviewed & tested by the community

same bug is in 6.x

pwolanin’s picture

pwolanin commented
StatusFileSize
new lock-form-272900-6x-10.patch2.25 KB

oops - patch above fails on 6.x (last hunk doesn't apply) - not sure why. Here's a re-roll that applies cleanly to 6.x. Re-tested too.

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Status: Reviewed & tested by the community » Fixed

Thanks, committed to 6.x.

Anonymous’s picture

Anonymous (not verified) commented
Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for two weeks with no activity.