Hey there, it is me again.
Short problem description:
We have incorporated the comment count for a node into some teasers. So if a new comment arrives those teasers should be updated.
What I have done:
I have incorporated an option to clear the display cache for certain display modes on comment creation.
So you can now set up for each display mode for each node type if its cache should be cleared if a comment is created for it.
I have decided to make it configurable for EACH display mode, because a big site like ours would not handle a cache clearing of ALL display caches at once that well.
I will try to incorporate both options so people who need it can enable if for a whole node type and others only for certain display modes.
Let me know what you think.
The patch is in the making. Almost done, just need to work around some of the Field Group modules bugs.
func0der
| Comment | File | Size | Author |
|---|---|---|---|
| #5 | display_cache-delete_comments_host_entity_cache-2511314-5.patch | 9.21 KB | func0der |
Comments
Comment #1
func0der commentedThere we go. A perfectly appliable patch ^^
Oh, I have forgotten to mention that the weird behavior you might see on the view mode configuration site is caused by the Field Group module.
I will create a patch and an issue for it today.
Comment #2
func0der commentedComment #3
func0der commentedRemoved a line of code, that should have not been in there in the first place.
Comment #4
func0der commentedComment #5
func0der commentedOkay, after internal testing we have found that clearing the cache on CREATION of a comment is non-sense, because it could not be published at that point.
So I have rewritten the code to clear the cache on publication instead of creation.
This will also work for moderated comments, since the clearing now triggers on publication.
Also it should be cleared on "unpublication" and deletion of a comment. The source has been changed in that way.
A new patch is attached.
func0der
Comment #6
kars-t commentedMaybe shorter function names would be a little easier to understand ;)
display_cache_get_comment_publication_clears_host_entity_cache_settings
display_cache_get_comment_publication_clears_host_entity_cache_setting_name
Maybe just
display_cache_get_comment_publication_settings
display_cache_get_comment_publication_setting_name
And things like this:
variable_get('display_cache_comment_publication_clears_host_entity_cache_' . $node->type, FALSE);
The "name" col in the "variable" table is just a varchar(128). You use 59 Cahrs with this and if there is a very long nodetyp name it might crash..
Well still I would commit this but the problematic part is how to make sure this works? The code seems fine and it should not destroy the modules functionality. Would be great to get more opinions in a different way than just do a release and wait... :)
Comment #7
func0der commentedI explicit naming over short names, because you really know that way to which part of the module this piece of code belongs.
I saw the problem with the variable name, too. But I have decided, that no one would name his node type "Supercalifragilisticexpialidocious" and even that is only 35 chars. So I definitely doubt that that will be a problem at any time.
What do you mean by: make sure it works?
What you would want to do to make sure this works is the following: Write tests :P
No, I am just kidding.
What you should do is the following
Comment #8
kars-t commentedAh I just commit it. Who needs testing...
Comment #10
kars-t commented