Inject a comment body text area and input format selector into the node edit form [#1733658]

I just had a useful discussion with mikey_p in IRC about a somewhat nasty problem. Currently, folks use real comment body text areas when updating an issue, complete with a real input format. This is absolutely critical functionality to maintain, so that [#] works, embedding images, etc, etc. However, if we just use the node revision log message field, there's no input format. Furthermore, we were already planning to alter the node edit UI to make the revision log field more prominent (not buried in a fieldset) and give it a new name, etc, so it's obvious what's going on. Frankly, my idea of using the revision log message as the comment body seemed good at the time, but in practice, it's a weird hack.

To solve both of these, instead of trying to re-use the revision log message field for this, we should probably just form_alter() the node edit form to inject a real comment body text area and input format selector. We just have to make sure node.module doesn't try to save this as part of the node. Instead, nodechanges will use it for the comment it automatically generates when the revision is saved.

Benefits include:

Users can provide a real comment body taking advantage of input formats and filters.
The UI will be more obvious, since we won't be trying to reuse a field many users are already familiar with for something else.
The code should hopefully be more obvious, since we'll be explicitly handling all this instead of playing tricks.
Some of the code to support something like this already exists, thanks to changes for #1700530: Ability to update an existing comment

While we're at it, we can either hide and completely ignore the revision log message, or we can try to automatically set the revision log to something like:

See comment $comment_num for changes (or whatever). We'll already see the user and date, so we don't need to duplicate any of that.

We could punt this part about mucking with revision log to a separate issue if we want. The main thing to focus on here is to *stop* trying to touch the revision_log field entirely and just inject an explicit body + input format into the node form that'll end up as the comment body.

Level Of Effort

It is estimated to take 0.5 days of effort to complete this issue.

Comment	File	Size	Author
#5	1733658-5.nodechanges-comment-field.patch	2.64 KB	dww
#4	1733658-4.nodechanges-comment-field.patch	2.31 KB	dww

Comments

Comment #1

senpai commented 15 August 2012 at 23:59

Agreed. This is a better approach than using the revision_log field, for sure.

Comment #2

senpai commented 11 October 2012 at 18:47

Assigned:

Unassigned

» bdragon

Dear @bdragon, happy early halloween. :)

Comment #3

dww

we/he/they

commented 20 October 2012 at 20:32

Assigned:

bdragon

» dww

This seems pretty central to getting #1545952: [META] UI for updating an issue in D7 moving forward, so I'm going to take a stab at this.

Comment #4

dww

we/he/they

commented 20 October 2012 at 22:20

Status:

Active

» Needs review

Status	File	Size
new	1733658-4.nodechanges-comment-field.patch	2.31 KB

Here's an initial stab at this. A few questions/TODOs:

A) The rest of nodechanges seems to assume that the comment bundle will always define a comment_body field. Is that a safe assumption? Should we only do this stuff if we discover a comment_body field in the corresponding comment bundle?

B) Should the label for this injected field take whatever label is defined for the comment_body field?

C) The logic where nodechanges_node_update() looks directly in $node for magic properties (to aid during migrations) seems a bit wonky. Now that this field is existing, perhaps we could just re-use that for the migrate stuff? This probably needs to move into a separate issue, but I wanted to raise it while I'm thinking of it.

D) ~~Currently if you *just* enter a comment in this field, nodechanges (or maybe even node.module itself) doesn't think the node was updated and so nodechanges doesn't generate a comment for you.~~ (Fixed by patch in comment #5)

E) Not sure what the behavior should be regarding if this comment field is required or not. I suppose we should leave it not required. I guess we just let sites use hook_form_alter() if they want to change this?

Comment #5

dww

we/he/they

commented 20 October 2012 at 22:27

Status	File	Size
new	1733658-5.nodechanges-comment-field.patch	2.64 KB

This fixes D (and removes any logic that's falling back to the node revision log field).

That's the only actual bug in the list from #4. Everything else is just optional and/or bling. So, I think this is actually 'needs review' at this point... any objections before I commit? Thoughts on the other items from #4?

Thanks,
-Derek

Comment #6

chx commented 21 October 2012 at 03:54

While this is a reasonable fix to the issue at hand, it introduces you to the problem that log is required and now you need to specify one. We need to do something about that too.

Comment #7

dww

we/he/they

commented 21 October 2012 at 06:40

We're going out of our way in drupalorg.module to make log required. We can just stop doing that. I don't see any reason to have a revision log message if you've got nodechanges enabled for a given node type.