Allow revision log messages to be required

Patch failed to apply. More information can be found at http://testing.drupal.org/node/14354. If you need help with creating patches please look at http://drupal.org/patch/create

Bah, made the patch against the node folder instead of the drupal root.

Looks good to me. I haven't had a chance to test it yet. My only nit-picky thing (since this is core) is that comments should end in a period.

Your second patch adds a line with nothing more than two spaces.

I'm also attempting to review it more in depth, but I'm currently trying to get past http://drupal.org/node/299661.

Everything looks fine, but I want to verify one validity check.

Okay, as I suspected, there are not enough validation checks being done. Here are the things I found wrong with the proposed patch:

• If 'Require log messages' is enabled for a content type, a log message is required for both the creation of a content as well as its deletion, which should not be.
• If 'Require log messages' is enabled for a content type but 'Create new revision' is not enabled (for either the content type or the content being edited), the log messsage is saved, but one can never view it, because only the current revision is saved in the database.

Also, not to sound like a broken record or anything, but we'll need tests for this. There's a lot of tweaky logic, as evidenced by oadaeh's #5. In fact, this might be a great case where "test-driven development" (writing tests that document the logic first, then filling in code so they pass) could shine.

http://drupal.org/node/120207 marked as a duplicate... though it was the predecessor since the patches/work is here keeping this one.

subscribing as author of predecessor request

If 'Require log messages' is enabled for a content type, a log message is required for both the creation of a content as well as its deletion, which should not be.

So I traced this down. The problem is that node_validate is being called by the edit form when you hit the delete button, validating all of the filled in form elements. Since any changes will be lost when you click the delete button, can't validation just be removed for that case? Or am I missing something more complex?

If 'Require log messages' is enabled for a content type but 'Create new revision' is not enabled (for either the content type or the content being edited), the log messsage is saved, but one can never view it, because only the current revision is saved in the database.

This brings up a more interesting case. I suppose it's possible to want to force log messages without revisions. If that's a valid case, then I guess the 'Revisions' tab could be changed to a 'Log messages' tab when revisions are disabled. Otherwise, the workflow form can be changed so that the option is only available if revisions are enabled.

As for doing tests, I totally agree, but unfortunately I'm rather new at testing in general. I'll take this as an opportunity to learn and see what I can come up with :).

--Andrew

@deviantintegral: Awesome! Don't hesitate to stop by #drupal and ask testing questions if you get stuck. There are lots and lots of people who would love to help you out. :)

#265259: Node form is validated before deletion should probably be resolved before this then.

This brings up a more interesting case. I suppose it's possible to want to force log messages without revisions. If that's a valid case, then I guess the 'Revisions' tab could be changed to a 'Log messages' tab when revisions are disabled. Otherwise, the workflow form can be changed so that the option is only available if revisions are enabled.

AFAIK, there is no way to view the saved log message if revisions are disabled. Saving them is fine, but kind of pointless w/o a UI.