Skip to content

Commit

Permalink
Internal: Replace round_mask references.
Browse files Browse the repository at this point in the history
Want to move away from round_mask as the only thing that determines
whether a review field is present on a review. So refer to
ReviewInfo::has_field instead.
  • Loading branch information
kohler committed Feb 22, 2021
1 parent 173d0c4 commit fd7d770
Show file tree
Hide file tree
Showing 8 changed files with 73 additions and 68 deletions.
2 changes: 1 addition & 1 deletion batch/reviewcsv.php
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ function add_row($x) {
} else {
$x["format"] = $rrow->reviewFormat;
}
foreach ($prow->viewable_review_fields($rrow, $user) as $fid => $f) {
foreach ($rrow->viewable_fields($user) as $fid => $f) {
$fv = $f->unparse_value($rrow->$fid ?? null, ReviewField::VALUE_TRIM | ReviewField::VALUE_STRING);
if ($fv === "") {
// ignore
Expand Down
5 changes: 3 additions & 2 deletions src/listactions/la_getjsonrqc.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,11 @@ function allow(Contact $user, Qrequest $qreq) {
function run(Contact $user, Qrequest $qreq, SearchSelection $ssel) {
$old_overrides = $user->add_overrides(Contact::OVERRIDE_CONFLICT);
$results = ["hotcrp_version" => HOTCRP_VERSION];
if (($git_data = Conf::git_status()))
if (($git_data = Conf::git_status())) {
$results["hotcrp_commit"] = $git_data[0];
}
$rf = $user->conf->review_form();
$results["reviewform"] = $rf->unparse_json(0, VIEWSCORE_REVIEWERONLY);
$results["reviewform"] = $rf->unparse_form_json($rf->bound_viewable_fields(VIEWSCORE_REVIEWERONLY));
$pj = [];
$ps = new PaperStatus($user->conf, $user, ["hide_docids" => true]);
foreach ($ssel->paper_set($user, ["topics" => true, "options" => true]) as $prow) {
Expand Down
2 changes: 1 addition & 1 deletion src/listactions/la_getreviewcsv.php
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ function run(Contact $user, Qrequest $qreq, SearchSelection $ssel) {
$text["email"] = $rrow->email;
$text["reviewername"] = Text::nameo($rrow, 0);
}
foreach ($prow->viewable_review_fields($rrow, $viewer) as $f) {
foreach ($rrow->viewable_fields($viewer) as $f) {
$fields[$f->id] = true;
$text[$f->name] = $f->unparse_value($rrow->{$f->id}, ReviewField::VALUE_TRIM);
}
Expand Down
2 changes: 1 addition & 1 deletion src/listactions/la_getscores.php
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ function run(Contact $user, Qrequest $qreq, SearchSelection $ssel) {
if ($rrow->reviewSubmitted) {
$this_scores = false;
$b = $a;
foreach ($row->viewable_review_fields($rrow, $user) as $field => $f) {
foreach ($rrow->viewable_fields($user) as $field => $f) {
if ($f->has_options && ($rrow->$field || $f->allow_empty)) {
$b[$f->search_keyword()] = $f->unparse_value($rrow->$field);
$any_scores[$f->search_keyword()] = $this_scores = true;
Expand Down
11 changes: 0 additions & 11 deletions src/paperinfo.php
Original file line number Diff line number Diff line change
Expand Up @@ -2504,17 +2504,6 @@ function has_author_seen_any_review() {
return false;
}

/** @param ?ReviewInfo $rrow
* @return array<string,ReviewField> */
function viewable_review_fields($rrow, Contact $user) {
$bound = $user->view_score_bound($this, $rrow);

return array_filter($this->conf->all_review_fields(), function ($f) use ($bound, $rrow) {
return $f->view_score > $bound
&& (!$f->round_mask || $f->is_round_visible($rrow));
});
}


function load_review_requests($always = false) {
if ($this->_row_set && ($this->_request_array === null || $always)) {
Expand Down
22 changes: 14 additions & 8 deletions src/papertable.php
Original file line number Diff line number Diff line change
Expand Up @@ -2799,20 +2799,26 @@ function resolveReview($want_review) {
$this->prow->ensure_full_reviews();
$this->all_rrows = $this->prow->reviews_by_display();

$this->viewable_rrows = array();
$round_mask = 0;
$min_view_score = VIEWSCORE_EMPTYBOUND;
$this->viewable_rrows = [];
$rf = $this->conf->review_form();
$unresolved_fields = $rf->all_fields();
foreach ($this->all_rrows as $rrow) {
if ($this->user->can_view_review($this->prow, $rrow)) {
$this->viewable_rrows[] = $rrow;
if ($rrow->reviewRound !== null) {
$round_mask |= 1 << (int) $rrow->reviewRound;
if (!empty($unresolved_fields)) {
$bound = $this->user->view_score_bound($this->prow, $rrow);
$this_resolved_fields = [];
foreach ($unresolved_fields as $f) {
if ($f->view_score > $bound && $rrow->has_nonempty_field($f))
$this_resolved_fields[] = $f;
}
foreach ($this_resolved_fields as $f) {
unset($unresolved_fields[$f->id]);
}
}
$min_view_score = min($min_view_score, $this->user->view_score_bound($this->prow, $rrow));
}
}
$rf = $this->conf->review_form();
Ht::stash_script("hotcrp.set_review_form(" . json_encode_browser($rf->unparse_json($round_mask, $min_view_score)) . ")");
Ht::stash_script("hotcrp.set_review_form(" . json_encode_browser($rf->unparse_form_json(array_diff_key($rf->all_fields(), $unresolved_fields))) . ")");

$want_rid = $want_rordinal = -1;
$rtext = (string) $this->qreq->reviewId;
Expand Down
79 changes: 35 additions & 44 deletions src/review.php
Original file line number Diff line number Diff line change
Expand Up @@ -223,23 +223,12 @@ function unparse_visibility() {
/** @param ?int|string $value
* @return bool */
function value_empty($value) {
// see also ReviewInfo::has_nonempty_field
return $value === null
|| $value === ""
|| ($this->has_options && (int) $value === 0);
}

/** @return bool */
function is_round_visible(ReviewInfo $rrow = null) {
if (!$this->round_mask) {
return true;
} else {
// NB missing $rrow is only possible for PC reviews
$round = $rrow ? $rrow->reviewRound : $this->conf->assignment_round(false);
return $round === null
|| ($this->round_mask & (1 << $round));
}
}

/** @return bool */
function include_word_count() {
return $this->displayed
Expand Down Expand Up @@ -606,18 +595,19 @@ function field($fid) {
function all_fields() {
return $this->forder;
}
/** @return array<string,ReviewField> */
function viewable_fields(Contact $user) {
$bound = $user->permissive_view_score_bound();
return array_filter($this->forder, function ($f) use ($bound) {
return $f->view_score > $bound;
});
/** @param int $bound
* @return array<string,ReviewField> */
function bound_viewable_fields($bound) {
$fs = [];
foreach ($this->forder as $fid => $f) {
if ($f->view_score > $bound)
$fs[$fid] = $f;
}
return $fs;
}
/** @return array<string,ReviewField> */
function editable_fields(ReviewInfo $rrow = null) {
return array_filter($this->forder, function ($f) use ($rrow) {
return !$f->round_mask || $f->is_round_visible($rrow);
});
function viewable_fields(Contact $user) {
return $this->bound_viewable_fields($user->permissive_view_score_bound());
}
/** @return list<ReviewField> */
function example_fields(Contact $user) {
Expand Down Expand Up @@ -667,14 +657,13 @@ function jsonSerialize() {
}
return $fmap;
}
function unparse_json($round_mask, $view_score_bound) {
$fmap = array();
foreach ($this->forder as $f) {
if ($f->view_score > $view_score_bound
&& (!$round_mask || !$f->round_mask
|| ($f->round_mask & $round_mask))) {
$fmap[$f->uid()] = $f->unparse_json();
}

/** @param array<string,ReviewField> $fields
* @return array<string,array> */
function unparse_form_json($fields) {
$fmap = [];
foreach ($fields as $f) {
$fmap[$f->uid()] = $f->unparse_json();
}
return $fmap;
}
Expand All @@ -695,7 +684,7 @@ private function webFormRows(PaperInfo $prow, ReviewInfo $rrow, Contact $contact
$format_description = $fi->description_preview_html();
}
echo '<div class="rve">';
foreach ($prow->viewable_review_fields($rrow, $contact) as $fid => $f) {
foreach ($rrow->viewable_fields($contact) as $fid => $f) {
$rval = "";
if ($rrow) {
$rval = $f->unparse_value($rrow->$fid ?? null, ReviewField::VALUE_STRING);
Expand Down Expand Up @@ -759,8 +748,8 @@ private function webFormRows(PaperInfo $prow, ReviewInfo $rrow, Contact $contact
/** @return int */
function nonempty_view_score(ReviewInfo $rrow) {
$view_score = VIEWSCORE_EMPTY;
foreach ($this->editable_fields($rrow) as $fid => $f) {
if (!$f->value_empty($rrow->$fid ?? null)) {
foreach ($this->forder as $fid => $f) {
if ($rrow->has_nonempty_field($f)) {
$view_score = max($view_score, $f->view_score);
}
}
Expand All @@ -770,8 +759,8 @@ function nonempty_view_score(ReviewInfo $rrow) {
/** @return int */
function word_count(ReviewInfo $rrow) {
$wc = 0;
foreach ($this->editable_fields($rrow) as $fid => $f) {
if ($f->include_word_count() && !$f->value_empty($rrow->$fid ?? null)) {
foreach ($this->forder as $fid => $f) {
if ($f->include_word_count() && $rrow->has_nonempty_field($f)) {
$wc += count_words($rrow->$fid);
}
}
Expand All @@ -781,8 +770,8 @@ function word_count(ReviewInfo $rrow) {
/** @return ?int */
function full_word_count(ReviewInfo $rrow) {
$wc = null;
foreach ($this->editable_fields($rrow) as $fid => $f) {
if (!$f->has_options) {
foreach ($this->forder as $fid => $f) {
if (!$f->has_options && $rrow->has_field($f)) {
$wc = $wc ?? 0;
if (!$f->value_empty($rrow->$fid ?? null)) {
$wc += count_words($rrow->$fid);
Expand Down Expand Up @@ -913,7 +902,7 @@ function text_form(PaperInfo $prow = null, ReviewInfo $rrow_in = null, Contact $
$i++; // XXX remove $i
assert($i === $f->display_order);
if ($f->view_score <= $revViewScore
|| ($f->round_mask && !$f->is_round_visible($rrow))) {
|| !$rrow->has_field($f)) {
continue;
}

Expand Down Expand Up @@ -1007,7 +996,7 @@ function unparse_text(PaperInfo $prow, ReviewInfo $rrow, Contact $contact,
$x[] = "* Updated: " . $this->conf->unparse_time($time) . "\n";
}

foreach ($prow->viewable_review_fields($rrow, $contact) as $fid => $f) {
foreach ($rrow->viewable_fields($contact) as $fid => $f) {
$fval = "";
if (isset($rrow->$fid)) {
$fval = $f->unparse_value($rrow->$fid, ReviewField::VALUE_STRING | ReviewField::VALUE_TRIM);
Expand Down Expand Up @@ -1404,7 +1393,7 @@ function unparse_review_json(Contact $viewer, PaperInfo $prow,

// review text
// (field UIDs always are uppercase so can't conflict)
foreach ($prow->viewable_review_fields($rrow, $viewer) as $fid => $f) {
foreach ($rrow->viewable_fields($viewer) as $fid => $f) {
if ($f->view_score > VIEWSCORE_REVIEWERONLY
|| !($flags & self::RJ_NO_REVIEWERONLY)) {
$fval = $rrow->$fid ?? null;
Expand Down Expand Up @@ -1456,7 +1445,7 @@ function unparse_flow_entry(PaperInfo $prow, ReviewInfo $rrow, Contact $contact)
} else {
$xbarsep = "";
}
foreach ($prow->viewable_review_fields($rrow, $contact) as $fid => $f) {
foreach ($rrow->viewable_fields($contact) as $fid => $f) {
if ($f->has_options && !$f->value_empty($rrow->$fid ?? null)) {
$t .= $xbarsep . $f->name_html . "&nbsp;"
. $f->unparse_value((int) $rrow->$fid, ReviewField::VALUE_SC);
Expand Down Expand Up @@ -1970,8 +1959,7 @@ private function check(ReviewInfo $rrow) {

foreach ($this->rf->forder as $fid => $f) {
if (!isset($this->req[$fid])
&& !($submit
&& (!$f->round_mask || $f->is_round_visible($rrow)))) {
&& (!$submit || !$rrow->has_field($f))) {
continue;
}
list($old_fval, $fval) = $this->fvalues($f, $rrow);
Expand Down Expand Up @@ -2144,7 +2132,10 @@ private function do_save(Contact $user, PaperInfo $prow, ReviewInfo $rrow) {
$view_score = VIEWSCORE_EMPTY;
$diffinfo = new ReviewDiffInfo($prow, $rrow);
$wc = 0;
foreach ($this->rf->editable_fields($rrow) as $fid => $f) {
foreach ($this->rf->all_fields() as $fid => $f) {
if (!$rrow->has_field($f)) {
continue;
}
list($old_fval, $fval) = $this->fvalues($f, $rrow);
if ($fval === false) {
$fval = $old_fval;
Expand Down
18 changes: 18 additions & 0 deletions src/reviewinfo.php
Original file line number Diff line number Diff line change
Expand Up @@ -548,6 +548,12 @@ function unparse_ordinal_id() {
}
}


/** @return bool */
function has_field(ReviewField $f) {
return !$f->round_mask || ($f->round_mask & (1 << $this->reviewRound)) !== 0;
}

/** @return bool */
function has_nonempty_field(ReviewField $f) {
return (!$f->round_mask || ($f->round_mask & (1 << $this->reviewRound)) !== 0)
Expand All @@ -556,6 +562,18 @@ function has_nonempty_field(ReviewField $f) {
&& (!$f->has_options || (int) $x !== 0);
}

/** @return array<string,ReviewField> */
function viewable_fields(Contact $user) {
$bound = $user->view_score_bound($this->prow, $this);
$fs = [];
foreach ($this->conf->all_review_fields() as $fid => $f) {
if ($f->view_score > $bound
&& $this->has_field($f))
$fs[$fid] = $f;
}
return $fs;
}


/** @param Contact $c
* @param list<Contact> &$assigned */
Expand Down

0 comments on commit fd7d770

Please sign in to comment.