Skip to content

Commit

Permalink
Report elided mentions only if a comment caused notification.
Browse files Browse the repository at this point in the history
And include all PC members in mention parsing, not just PC members
who can see the current track.
  • Loading branch information
kohler committed Nov 2, 2023
1 parent 592356b commit 6bea57e
Show file tree
Hide file tree
Showing 5 changed files with 29 additions and 26 deletions.
10 changes: 5 additions & 5 deletions batch/saveusers.php
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<?php
// saveusers.php -- HotCRP command-line user modification script
// Copyright (c) 2006-2022 Eddie Kohler; see LICENSE.
// Copyright (c) 2006-2023 Eddie Kohler; see LICENSE.

if (realpath($_SERVER["PHP_SELF"]) === __FILE__) {
require_once(dirname(__DIR__) . "/src/init.php");
Expand Down Expand Up @@ -137,10 +137,10 @@ static function make_args($argv) {
"config: !",
"help,h !",
"user:,u: =EMAIL Create or modify user EMAIL.",
"roles:,r: Set roles (`-u` only).",
"user-name:,uname: Set user name (`-u` only).",
"disable Disable user (`-u` only).",
"enable Enable user (`-u` only).",
"roles:,r: Set roles for `-u` user.",
"user-name:,uname: Set name for `-u` user.",
"disable Disable `-u` user.",
"enable Enable `-u` user.",
"expression[],expr[],e[] =JSON Create or modify users specified in JSON.",
"no-notify,no-email Do not send email notifications.",
"notify !",
Expand Down
4 changes: 2 additions & 2 deletions src/api/api_comment.php
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ private function run_post(Qrequest $qreq, $rrd, $crow) {
if (($n->types & NotificationInfo::MENTION) !== 0) {
if ($n->sent) {
$mentions[] = $n->user_html ?? $suser->reviewer_html_for($n->user);
} else {
} else if ($xcrow->timeNotified === $xcrow->timeModified) {
$mentions_missing = true;
}
}
Expand All @@ -177,7 +177,7 @@ private function run_post(Qrequest $qreq, $rrd, $crow) {
$this->ms->success($this->conf->_("<5>Notified mentioned users {:nblist}", $mentions));
}
if ($mentions_missing) {
$this->ms->msg_at(null, $this->conf->_("<0>Some users mentioned in the comment cannot see the comment yet, so they were not notified."), MessageSet::WARNING_NOTE);
$this->ms->msg_at(null, $this->conf->_("<0>Some mentioned users cannot currently see this comment, so they were not notified."), MessageSet::WARNING_NOTE);
}
return $xcrow;
}
Expand Down
25 changes: 10 additions & 15 deletions src/api/api_completion.php
Original file line number Diff line number Diff line change
Expand Up @@ -203,12 +203,15 @@ static function searchcompletion_api(Contact $user, $qreq) {
return ["ok" => true, "searchcompletion" => self::search_completions($user, "")];
}

const MENTION_PARSE = 0;
const MENTION_COMPLETION = 1;

/** @param Contact $user
* @param ?PaperInfo $prow
* @param int $cvis
* @param bool $include_all
* @param 0|1 $reason
* @return list<list<Contact|Author>> */
static function mention_lists($user, $prow, $cvis, $include_all = false) {
static function mention_lists($user, $prow, $cvis, $reason) {
$alist = $rlist = $pclist = [];

if ($prow
Expand Down Expand Up @@ -245,7 +248,9 @@ static function mention_lists($user, $prow, $cvis, $include_all = false) {
}

if ($user->can_view_pc()) {
if (!$prow || !$user->conf->check_track_view_sensitivity()) {
if (!$prow
|| $reason === self::MENTION_PARSE
|| !$user->conf->check_track_view_sensitivity()) {
$pclist = $user->conf->enabled_pc_members();
} else {
foreach ($user->conf->pc_members() as $p) {
Expand All @@ -256,24 +261,14 @@ static function mention_lists($user, $prow, $cvis, $include_all = false) {
}
}

$lists = [];
if ($include_all || !empty($alist)) {
$lists[] = $alist;
}
if ($include_all || !empty($rlist)) {
$lists[] = $rlist;
}
if ($include_all || !empty($pclist)) {
$lists[] = $pclist;
}
return $lists;
return [$alist, $rlist, $pclist];
}

/** @param Qrequest $qreq
* @param ?PaperInfo $prow */
static function mentioncompletion_api(Contact $user, $qreq, $prow) {
$comp = [];
$mlists = self::mention_lists($user, $prow, CommentInfo::CTVIS_AUTHOR, true);
$mlists = self::mention_lists($user, $prow, CommentInfo::CTVIS_AUTHOR, self::MENTION_COMPLETION);
$aunames = [];
foreach ($mlists as $i => $mlist) {
$skey = $i === 2 ? "sm1" : "s";
Expand Down
2 changes: 1 addition & 1 deletion src/commentinfo.php
Original file line number Diff line number Diff line change
Expand Up @@ -1048,7 +1048,7 @@ private function analyze_mentions($user) {
// enumerate desired mentions and save them
$desired_mentions = [];
$text = $this->commentOverflow ?? $this->comment;
foreach (MentionParser::parse($text, ...Completion_API::mention_lists($user, $this->prow, $this->commentType & self::CTVIS_MASK)) as $mpx) {
foreach (MentionParser::parse($text, ...Completion_API::mention_lists($user, $this->prow, $this->commentType & self::CTVIS_MASK, Completion_API::MENTION_PARSE)) as $mpx) {
$named = $mpx[0] instanceof Contact || $mpx[0]->status !== Author::STATUS_ANONYMOUS_REVIEWER;
$desired_mentions[] = [$mpx[0]->contactId, $mpx[1], $mpx[2], $named];
$this->conf->prefetch_user_by_id($mpx[0]->contactId);
Expand Down
14 changes: 11 additions & 3 deletions src/mentionparser.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,14 @@ class MentionParser {
* @param array<Contact|Author> ...$user_lists
* @return \Generator<array{Contact|Author,int,int}> */
static function parse($s, ...$user_lists) {
// filter out empty user lists
$ulists = [];
foreach ($user_lists as $ulist) {
if (!empty($ulist)) {
$ulists[] = $ulist;
}
}

$pos = 0;
$len = strlen($s);
$isascii = $collator = $strength = null;
Expand All @@ -31,7 +39,7 @@ static function parse($s, ...$user_lists) {

// check emails
if (($email = validate_email_at($s, $pos + 1))) {
foreach ($user_lists as $ulist) {
foreach ($ulists as $ulist) {
foreach ($ulist as $u) {
if (strcasecmp($u->email, $email) === 0
&& self::mention_ends_at($s, $pos + 1 + strlen($email))) {
Expand All @@ -55,7 +63,7 @@ static function parse($s, ...$user_lists) {

// Match the first word
$uset = [];
foreach ($user_lists as $listindex => $ulist) {
foreach ($ulists as $listindex => $ulist) {
foreach ($ulist as $u) {
if ($u->firstName === "" && $u->lastName === "") {
continue;
Expand All @@ -80,7 +88,7 @@ static function parse($s, ...$user_lists) {
$pos2 = $pos + 1 + strlen($m[0]);
$best_ux = $best_pos2 = $best_endpos = null;
while (count($uset) > 1 && self::word_at($s, $pos2, $isascii, $m)) {
if (count($user_lists) > 1) {
if (count($ulists) > 1) {
usort($uset, function ($a, $b) {
return $a[4] <=> $b[4];
});
Expand Down

0 comments on commit 6bea57e

Please sign in to comment.