[buddypress-trac] [BuddyPress Trac] #5130: Synchronizing activity comments to main component
buddypress-trac
noreply at wordpress.org
Wed Mar 26 13:46:24 UTC 2014
#5130: Synchronizing activity comments to main component
-------------------------+-----------------------
Reporter: r-a-y | Owner: r-a-y
Type: enhancement | Status: assigned
Priority: normal | Milestone: 2.0
Component: Core | Version: 1.2
Severity: normal | Resolution:
Keywords: |
-------------------------+-----------------------
Comment (by boonebgorges):
Many thanks for the update, r-a-y. Looking great. A couple
thoughts/concerns:
- No need to define an 'action' in `bp_blogs_record_comment()`. This is
handled by the callback
`bp_blogs_format_activity_action_new_blog_comment()`. (It looks like I
actually forgot to clean this up totally and you just kept what was there
- so this is more a self-critique :) )
- I agree that, given `comments_open()`, something like
`bp_blogs_setup_activity_loop_globals()` is probably necessary. I still
feel very uneasy about it. It could result in up to 20 `switch_to_blog()`s
on a single page. (Unlikely, but possible.) I think we should still
consider using blogmeta for `allow_comments` and `thread_depth` at some
point in the future. `thread_depth` will be pretty easy, since, as you
note, it won't be changed often. For comments_open, I wonder if we can set
up our own parallel `comments_open()` (which would reproduce the logic of
`_close_comments_for_old_post()`, but wouldn't require the post object -
it'd use the activity date instead). I'm probably just being a worrywart,
so this is probably something we could think about in 2.1 if there are
reports of performance issues in the wild.
- Likewise, you have a note about how we should probably use the object
cache for these values. This is a very good idea, especially since the
kind of site using an object cache is the most likely to be busy enough to
experience performance problems related to this feature. Doing super-
precise cache invalidation will be difficult, since
`_close_comments_for_old_post()` is passive (there's no user action that
we can hook to at the moment when comments are auto-closed). This might be
a spot to use a transient, so that our cached value would be at most (say)
an hour or two off. We could even do a realtime check when someone
actually attempts to post a comment, and return an error if they happen to
be posting in the window between comments being closed and our cache being
invalidated. Not the most elegant experience in the world, but it might
work. As above, it's likely that I'm overthinking what's necessary for a
first iteration.
- I'm concerned about the force-deletion of blog comments when activity
items are deleted. I think there may be cases when a user might
legitimately want to remove an activity item but not the content itself.
We don't necessarily have to account for that in the UI, but at the very
least maybe we should refrain from deleting the original content
irrevocably. Are there technical reasons why it has to be deleted rather
than just trashed?
--
Ticket URL: <https://buddypress.trac.wordpress.org/ticket/5130#comment:23>
BuddyPress Trac <http://buddypress.org/>
BuddyPress Trac
More information about the buddypress-trac
mailing list