db_ignore_slave() not called when saving a comment [#965628]

Comment	File	Size	Author
#5	ignore_slave_db_after_saving_comment-965628-5.patch	1.1 KB	AnalogFile

#1	ignore_slave_db_after_saving_comment-965628-1.patch	758 bytes	AnalogFile

Comment #1

AnalogFile CreditAttribution: AnalogFile commented 8 November 2010 at 16:14

Assigned:	AnalogFile	» Unassigned
Status:	Needs work	» Needs review

File	Size
ignore_slave_db_after_saving_comment-965628-1.patch	758 bytes

And the patch.

There's no testing and no current way to test this at the moment. See http://drupal.org/node/808560#comment-3681926

Log in or register to post comments

Comment #2

moshe weitzman CreditAttribution: moshe weitzman commented 9 November 2010 at 20:22

Status:

Needs review

» Needs work

Looks good. Should say comment instead of node at '// saved node to be propagated to the slave.'.

Log in or register to post comments

Comment #3

AnalogFile CreditAttribution: AnalogFile commented 10 November 2010 at 13:28

I took the call and the comment from the other leg of the 'if' which is also part of the comment (it's creating a new comment instead of saving an existing one).

I'm not that familiar with Drupal internals. Since that comment says 'node' I assumed that when creating/saving a comment we were really within a "wider" operation related to the node.

So there are three possibilities:
1) we are always within a wider operation, and both comments are correct (as well as the patch in #1)
2) we are within a wider node-level save operation when we create the comment, but not when saving it. In this case #2 is correct and the patch in #1 should be changed to say "comment" instead of node.
3) both creation and deletion of a comment is, generally, done independently from the node. In this case not only the comment introduced in patch #1 is to be changed, but also the comment in the other leg of the 'if'.

I do not know which is of those three. I may come back to this when I understand Drupal internals better. But if someone else knows already, go ahead and either mark this RTBC or post the correct patch.