as part of #34496: [meta] Add Flag module to allow users to subscribe/unsubscribe without posting a comment, we need to add functionality to automatically flag an issue for a user when they either initially create it, or when they comment on an existing issue. users can then manually un-follow issues as necessary.

psuedo-code suggested by quicksketch at #397464-11: Action for executing a flag:

function my_site_comment($comment, $op) {
  if ($op == 'insert') {
    flag('flag', 'subscribe', $comment->nid);
  }
}
CommentFileSizeAuthor
#9 1284244-auto-flag-issues-2.patch3.09 KBhunmonk
#7 1284244-auto-flag-issues-1.patch3.15 KBhunmonk

Comments

dww’s picture

dww
we/he/they
commented
Issue tags: +flag integration, +prairie

We want to make sure we're adding the flag as the right user. I guess both hook_comment() and hook_nodeapi() run as the user creating the node and the comment, so we can just rely on global $user for this. But maybe it's safer to explicitly specify?

Anyway, I agree this code should just live in project_issue, conditionally fired based on the flag-specific setting we're adding over at #397458: Revamp mailing logic to leverage flag module (as opposed to trying to get this all working via actions/triggers/rules -- let's just deploy before the dependency chain spirals out of control). ;)

dww’s picture

dww
we/he/they
commented

Oh right. There's code to do something similar in the patch at #404084-20: Add support for tracking flagged nodes. However, not everyone using project_issue uses tracker2, so it'd still be nice to have this code live in project_issue itself.

Wonder if that code makes sense in #404084 or not. Hrm.

hunmonk’s picture

hunmonk commented

I guess both hook_comment() and hook_nodeapi() run as the user creating the node and the comment, so we can just rely on global $user for this. But maybe it's safer to explicitly specify?

it doesn't look to me like it's possible to specify another user when creating a new comment, at least not via the regular UI. outside of programmatically created comments, i can't think of another way this would bite us -- should we bother worrying about this?

dww’s picture

dww
we/he/they
commented

isn't "bother worrying about this" adding all of $comment->uid to the parameters to flag()? ;)

hunmonk’s picture

hunmonk commented

actually, it would be $arg['uid'] (don't we love comment module), and also we need to load the user account, flag is expecting the whole user object. i guess i can build that into the helper function i'm writing...

dww’s picture

dww
we/he/they
commented

if it's a PITA, skip it.

hunmonk’s picture

hunmonk commented
Status: Active » Needs review
StatusFileSize
new 1284244-auto-flag-issues-1.patch3.15 KB

first crack at this, not tested yet, just posting for code quality feedback.

dww’s picture

dww
we/he/they
commented
Status: Needs review » Needs work

Basically looks fine. Minor nit:

+  if (module_exists('flag') && !module_exists('tracker2')) {
+    if ($flag = project_issue_get_follow_flag()) {

project_issue_get_follow_flag() already checks for if flag is enabled, so I'd just combine these two lines into a single check:

  if ($flag = project_issue_get_follow_flag() && !module_exists('tracker2')) {
    ...
  }

Otherwise, if it works, I'm happy. ;)

hunmonk’s picture

hunmonk commented
Status: Needs work » Needs review
StatusFileSize
new 1284244-auto-flag-issues-2.patch3.09 KB

cleaned up code per #8

dww’s picture

dww
we/he/they
commented
Priority: Major » Normal
Status: Needs review » Fixed

I noticed another simplification. Instead of this:

 +      if (!isset($uid)) {
+        $account = $GLOBALS['user'];
+      }
+      else {
+        $account = user_load($uid);
+      }

We just need:

    $account = isset($uid) ? user_load($uid) : $GLOBALS['user'];

Made that change, then committed and pushed to 34496-flag-integration:
http://drupal.org/commitlog/commit/1894/e7b97d957a2784098b17ba82274ab215...

However, in testing I realized #8 was broken and neither of us tested it. ;) Fixed that and also pushed:

http://drupal.org/commitlog/commit/1894/43887a0b30093f54d7c933588e30b9e7...

Given that this is in the main feature branch now, I'm going to call it fixed. We can wait to mark #34496: [meta] Add Flag module to allow users to subscribe/unsubscribe without posting a comment fixed itself until we merge that into master.

hunmonk’s picture

hunmonk commented
Status: Fixed » Needs review

reopening to address an implementation concern:

bypassing the auto flag simply based on if tracker2 is installed is not a complete test. it's possible that tracker2 could be installed, but not have its follow flag set or set to a different follow flag. do we care to shore this up or not?

hunmonk’s picture

hunmonk commented
Status: Needs review » Fixed

http://drupal.org/commitlog/commit/1894/0e022780065f0f9e722b538ca69e4d15...

Automatically closed -- issue fixed for 2 weeks with no activity.