Skip to content

Commit

Permalink
Contacts are truly a special case...
Browse files Browse the repository at this point in the history 
It's too hard to ensure the creating user is always a contact
using modularized code. Instead use a big hammer and always add
this user as a contact.

Sorry folks.
  • Loading branch information
@kohler
kohler committed Jul 31, 2020
1 parent a4e349c commit 5538134
Show file tree
Hide file tree
Showing 5 changed files with 41 additions and 37 deletions.
32 changes: 13 additions & 19 deletions src/intrinsicvalue.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -452,30 +452,22 @@ class Contacts_PaperOption extends PaperOption {
function __construct(Conf $conf, $args) {
parent::__construct($conf, $args);
}
/** @param array<int,Author> $ca */
static function value_fill(PaperValue $ov, $ca) {
function value_force(PaperValue $ov) {
// $ov->value_list: contact IDs
// $ov->data_list: emails
// $ov->anno("users"): list<Author>
// $ov->anno("bad_users"): list<Author>
$ca = array_filter($ca, function ($c) { return $c->contactId > 0; });
$va = array_map(function ($c) { return $c->email; }, $ca);
/** @phan-suppress-next-line PhanTypeMismatchArgument */
// NB fake papers start out with this user as contact
$ca = $va = [];
foreach ($ov->prow->contacts(true) as $cflt) {
if ($cflt->contactId > 0) {
$ca[] = $cflt;
$va[$cflt->contactId] = $cflt->email;
}
}
$ov->set_value_data(array_keys($va), array_values($va));
$ov->set_anno("users", array_values($ca));
}
function value_force(PaperValue $ov) {
// Papers with 0 paperId have contacts array prepopulated
// with the paper creator, so that permission checks work.
// We can't use that prepopulated array: the database says there
// are no contacts.
self::value_fill($ov, $ov->prow->paperId ? $ov->prow->contacts(true) : []);
}
function value_initial(PaperInfo $prow) {
$ov = PaperValue::make($prow, $this);
self::value_fill($ov, $ov->prow->contacts(true));
return $ov;
}
function value_unparse_json(PaperValue $ov, PaperStatus $ps) {
$ca = [];
foreach ($ov->anno("users") ?? [] as $c) {
Expand All @@ -495,7 +487,9 @@ function value_unparse_json(PaperValue $ov, PaperStatus $ps) {
}
function value_check(PaperValue $ov, Contact $user) {
if ($ov->anno("modified")) {
if (!count($ov->value_list())) {
if (count($ov->value_list()) === 0
&& $ov->prow->paperId > 0
&& count($ov->prow->contacts()) > 0) {
$ov->error($this->conf->_("Each submission must have at least one contact."));
}
if (!$user->allow_administer($ov->prow)
Expand Down Expand Up @@ -531,7 +525,7 @@ function value_by_email(PaperValue $ov, $email) {
$i = self::ca_index($ca, $email);
return $i !== false ? $ca[$i] : null;
}
static function apply_new_users(PaperValue $ov, $new_ca, &$ca) {
static private function apply_new_users(PaperValue $ov, $new_ca, &$ca) {
$bad_ca = [];
foreach ($new_ca as $c) {
$c->contactId = 0;
Expand Down
10 changes: 5 additions & 5 deletions src/paperinfo.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -428,7 +428,7 @@ class PaperInfo {
/** @var bool */
private $_conflict_array_email;
/** @var ?Contact */
private $_paper_owner;
private $_paper_creator;
/** @var ?array<int,ReviewInfo> */
private $_review_array;
/** @var int */
Expand Down Expand Up @@ -526,7 +526,7 @@ static function make_new(Contact $user) {
$prow->topicIds = "";
$prow->leadContactId = $prow->shepherdContactId = "0";
$prow->blind = "1";
$prow->_paper_owner = $user;
$prow->_paper_creator = $user;
$prow->check_rights_version();
$ci = PaperContactInfo::make_empty($prow, $user);
$ci->conflictType = CONFLICT_CONTACTAUTHOR;
Expand Down Expand Up @@ -1324,10 +1324,10 @@ function load_conflicts($email) {
$this->_conflict_array[$cflt->contactId] = $cflt;
}
}
} else if ($this->paperId === 0 && $this->_paper_owner) {
$cflt = new Author($this->_paper_owner);
} else if ($this->paperId === 0 && $this->_paper_creator) {
$cflt = new Author($this->_paper_creator);
$cflt->paperId = $this->paperId;
$cflt->contactId = $this->_paper_owner->contactId;
$cflt->contactId = $this->_paper_creator->contactId;
$cflt->conflictType = CONFLICT_CONTACTAUTHOR;
$this->_conflict_array = [$cflt->contactId => $cflt];
$this->_conflict_array_email = true;
Expand Down
4 changes: 0 additions & 4 deletions src/paperoption.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1018,10 +1018,6 @@ function takes_multiple() {

function value_force(PaperValue $ov) {
}
/** @return ?PaperValue */
function value_initial(PaperInfo $prow) {
return null;
}
/** @return bool */
function value_present(PaperValue $ov) {
return !!$ov->value;
Expand Down
10 changes: 9 additions & 1 deletion src/paperstatus.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -810,7 +810,7 @@ static function execute_topics(PaperStatus $ps) {

static function check_one_option(PaperOption $opt, PaperStatus $ps, $oj) {
if ($oj === null) {
$ov = $ps->prow ? null : $opt->value_initial($ps->_nnprow);
$ov = null;
} else if ($oj instanceof PaperValue) {
$ov = $oj;
} else {
Expand Down Expand Up @@ -1020,6 +1020,14 @@ private function check_contacts_last($pj) {
}
Dbl::free($result);
}
// if creating a paper, user must always be contact
if (!$this->_nnprow->paperId
&& $this->user->contactId > 0) {
// NB ok to have multiple inserters for same user
$this->_conflict_ins = $this->_conflict_ins ?? [];
$this->_conflict_ins[] = [$this->user->contactId, CONFLICT_CONTACTAUTHOR, CONFLICT_CONTACTAUTHOR];
$this->diffs["contacts"] = true;
}
}

private function execute_conflicts() {
Expand Down
22 changes: 14 additions & 8 deletions src/papertable.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1413,19 +1413,25 @@ function echo_editable_contact_author($option, $ov, $reqov) {
$foundreqau = [];
foreach ($contacts as $au) {
$foundreqau[] = $reqau = $option->value_by_email($reqov, $au->email);
$fixed = $au->contactId > 0 && ($au->conflictType & CONFLICT_AUTHOR) !== 0;
echo '<div class="',
$reqau && $reqau->author_index
? $this->control_class("contacts:{$reqau->author_index}", "checki")
: "checki",
'"><label><span class="checkc">',
Ht::hidden("contacts:email_$cidx", $au->email),
Ht::checkbox("contacts:active_$cidx", 1, $reqau || $fixed,
["data-default-checked" => $au->contactId > 0 && $au->conflictType >= CONFLICT_AUTHOR,
"disabled" => $fixed, "id" => false]),
'</span>',
Text::nameo_h($au, NAME_E),
$au->conflictType & CONFLICT_AUTHOR ? '' : ' (<em>non-author</em>)';
Ht::hidden("contacts:email_$cidx", $au->email);
if (($au->contactId > 0 && ($au->conflictType & CONFLICT_AUTHOR) !== 0)
|| ($au->contactId === $this->user->contactId && $ov->prow->paperId <= 0)) {
echo Ht::hidden("contacts:active_$cidx", 1),
Ht::checkbox(null, 1, true, ["disabled" => true, "id" => false]);
} else {
echo Ht::checkbox("contacts:active_$cidx", 1, !!$reqau,
["data-default-checked" => $au->contactId > 0 && $au->conflictType >= CONFLICT_AUTHOR, "id" => false]);
}
echo '</span>', Text::nameo_h($au, NAME_E);
if (($au->conflictType & CONFLICT_AUTHOR) === 0
&& $ov->prow->paperId > 0) {
echo ' (<em>non-author</em>)';
}
if ($this->user->privChair
&& $au->contactId !== $this->user->contactId) {
echo ' ', actas_link($au);
Expand Down

0 comments on commit 5538134

Please sign in to comment.