| Project: | Checkout (Content locking) |
| Version: | 6.x-2.x-dev |
| Component: | Code |
| Category: | bug report |
| Priority: | critical |
| Assigned: | EugenMayer |
| Status: | needs work |
Issue Summary
Well i installed checkout and it did not work at all.
While i looked for how it works, i was wondering why hook_init is used at all.
hock_form_alter is used, but not for locking..instead check_request_handler is used as an evil hack to lock nodes. With any ajax request this thing gets broken, locks get removed after they go added.
i removed the handler completely, as it is doomed to fail anyway.
Instead Iam using checkout_form_alter to set the lock, if a node gets editted.
On save, the lock gets removed and the node is free again.. ( this is the old behavior anyway)
If the user does anything else, use the back button or whatever...a new feature comes into play.
The user gets warned on every site he visits, that he has pending locks. An link is offered to remove this lock for this node ( or nodes) immediately.
A user can only remove his own locks, while an admin can still remove any locks. This way nodes arent locked accidently for too long. In addition, locks get cleared after the time you set int the checkout configuration anyway.
As the changes arent to minor, i did not attach a patch, but the whole file. I used the current dev version, so feel free to generate a patch, if needed.
I also applied the latest patch of these http://drupal.org/node/422870
| Attachment | Size |
|---|---|
| checkout.module.txt | 13.76 KB |
Comments
#1
#2
patched attched
#3
diff -up patch
#4
This patch is against the dev version. Compairing to the old it fixing some exception and special cases. Module works stable right now.
I hope the authors will react in 1-2 weeks, otherwise i will fork it :(
#5
Interesting, but should preferably avoid "unnecessary" forking, especially if it is just due to too short deadlines...
IMO, 1-2 weeks notice is way too short deadine for such feedback, say 2 months instead...
For example, August is public holiday month in Europe, people could be gone for weeks and more. Also common to have some flexibility regarding which weeks, not necessarily ending the holiday just when the month ends.
#6
First of all, thanks for taking this forward. After having taken a look at the patch, I have a few concerns:
1. Locking via Form API is definitely a good approach. Have you seen the issues #538522: What triggers checking out? (regarding Editable fields) and #528138: Add support for edit_section module (Edit section)? Does your patch also work with these modules?
2. However, the approach chosen to unlock contents when moving away from an edit page is IMO by far inferior to what we have right now. Enforcing one extra click to manually unlock contents (which was done automatically before) is a bit weird. Personally, I would like to see a JavaScript-based solution which unlocks on the browser "onunload" event, with a fallback on manual unlocking for those users without JavaScript.
3. There are certain rules regarding the coding style of submitted patches.
If you could clean up the code this could be the starting point of a new 3.x branch.
#7
Hello!
Well i will convert the patch according to the coding styles. I have fixed as small bug ($form['#form'] should be $form['#node']) it was just a typo.
Well to the "section" edits and all the other stuff, the checkout (when its done) is yet limited to all _node_forms of every node type in the system. You can easily add new stuff here, if section edits do not work with node_form ( but that would suprise me actually).
For the unlocking issue, actually any JS approach will fail:
* User has no JS -> no unlocking _at all_
* User simply closes the browser
Iam sure there are additional scenrios where this can break. e.g. you edit a node and then start a new tab, looking for some other information on the side (maybe something you want to include)..when you do automatic unlocking on "view of other nodes" your locks break here again.
Yet, unlocking is done
* When the user presses "save"
* When the user presses "cancel" (well this will be mostly missing for your systems, as the global cancel button is added by me)
So on every _real_ transaction. When you normaly save/cancel your content, you wont be confrontated with any unlocking. Just when you end your editing session abnormaly, you will need to unlock it manually. This is the most robust way. In addition the user will learn to end his edit sessions properly then.
I think you did not get this right or? You thought after every edit, i need to manually unlock, or?
I yet have some problems with the translation of the messages, as my strings do not show up in the Translation system in drupal, not sure why. So i will first need to include the english messages again in the patch, yet they are german hardcoded.
Thanks for the reply, glad to see the project is still alive.
#8
is there any progress on this? Would love to see your latest patch
#9
there was a lil problem with the patch. Probably the form api changed.
This version though now works.
#10
The checkout module works in most of the cases in my situation. But, it failed sometimes. Here are some situations that it failed.
1. In "Content management->Content->Locked documents", I sometimes saw a document being locked for over a day and this was not a "Persistent lock". Why this happens? Is it because the user closed the browser directly without leaving "editing" mode?
2. UserA started editing a page, while he was editing, UserB started editing. In most cases, UserB got the document is locked message and could not edit. But, sometimes, UserB could get into editing without any warning message. Then, when UserA saved the page, he got a message saying that the document is locked by UserB. If UserA clicked "save" again. He got the document is locked message again. If he then clicked "View", the editing UserA made was saved! I do not know what would happen to UserB. But, this did not happen every time. Sometimes, UserA could not save his changes. I found this might happen in the situation when UserA got into "editing" mode but did not save the document for a long time. A user complained that his 2 hours editing was not saved and got a message saying the document was locked by someone else. :-(
Thanks for any comments and help on this.
BTW, I am using the 6.x-2.2 version, not the 6.x-2.x-dev version.
#11
Well i pretty much changed the code of Checkout, but did not release it yet due to time reasons. Is the author actually interested / active in this project at all?
#12
Hi EugenMayer,
I will try your code later. I will let you know the result of my test. Our project is in critical time. I do not want to screw it up. Drupal is still not very steady. On my testing server, the options to choose input format are disabled, even for the admin account.
Thanks.
#13
Its not the code in the upper nathanz. The code i have here is working in production for 4 Months now...so pretty stable
#14
Hi any way to have access to the code you've fixed EugenMayer? I'm testing checkout but it gives me strange results with revisions (though wysiwyg is working fine). I'd like to see if any latest versions would be a better start-up to fix this revision issue.
#15
Well just wait some few days we have fixed this issue and then you will get the code, this or the other way :)
The only problem i know yet with the current checkout is, forms that are redirecting to the "edit" form after saving, like user edit. So actually, you never end the transaction properly..but well this will need some documentation then. Otherwise it works in production for quiet some time now.
#16
Ok. As explained above, last version of checkout didn't look like working for me but this could be related to the quite complex workflow I had to implement. Eager to see the code and bug report.