Skip to content

Commit

Permalink
Type LPage and users
Browse files Browse the repository at this point in the history 
This uncovered an inconstency in our API where we used the
result of a void function. Explicitly added a `null` return
to keep our current behavior.

TEST=Tested all the proofing buttons (save, mark as bad, ...).
  • Loading branch information
@jchaffraix @cpeel
jchaffraix authored and cpeel committed Jan 4, 2025
1 parent ec5cd55 commit 9959c12
Show file tree
Hide file tree
Showing 4 changed files with 54 additions and 45 deletions.
3 changes: 2 additions & 1 deletion api/v1_projects.inc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -866,7 +866,8 @@ function api_v1_project_page(string $method, array $data, array $query_params)
case 'revert':
return $proof_project_page->save_and_revert(receive_project_text_from_request_body('text'));
case 'abandon':
return $proof_project_page->returnToRound($pguser);
$proof_project_page->returnToRound($pguser);
return null;
default:
throw new BadRequest("$page_action is not a valid page action.");
}
Expand Down
52 changes: 30 additions & 22 deletions pinc/LPage.inc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,10 @@ include_once($relPath.'forum_interface.inc'); // get_forum_user_id()
*
* If successful, it returns [ $imagename, $state ],
* on failure it returns NULL;
*
* @return ?array{0:string, 1:string}
*/
function get_available_page_array($project, $round, $pguser, $preceding_proofer_restriction)
function get_available_page_array(Project $project, Round $round, string $pguser, string $preceding_proofer_restriction): ?array
{
// Normally, pages are served in order of "page number"
// (i.e., by the 'image' field)
Expand Down Expand Up @@ -127,7 +129,7 @@ function get_available_page_array($project, $round, $pguser, $preceding_proofer_
* Returns an LPage, unless no page is available,
* in which case returns NULL and sets $err.
*/
function get_available_page($projectid, $proj_state, $pguser, &$err)
function get_available_page(string $projectid, string $proj_state, string $pguser, ?string &$err): ?LPage
{
// can throw NonexistentProjectException
$project = new Project($projectid);
Expand Down Expand Up @@ -157,8 +159,10 @@ function get_available_page($projectid, $proj_state, $pguser, &$err)
/**
* Returns an array [image, state] unless no page is available,
* in which case throw an exception.
*
* @return array{0:string, 1:string}
*/
function get_available_proof_page_array($project, $round, $pguser)
function get_available_proof_page_array(Project $project, Round $round, string $pguser): array
{
global $preceding_proofer_restriction;

Expand Down Expand Up @@ -190,12 +194,12 @@ function get_available_proof_page_array($project, $round, $pguser)
* in which case it throws an exception.
*/
function get_indicated_LPage(
$projectid,
$proj_state,
$imagefile,
$page_state,
$reverting_to_orig
) {
string $projectid,
string $proj_state,
string $imagefile,
string $page_state,
int $reverting_to_orig
): LPage {
// Make sure project is still in same state.
// can throw NonexistentProjectException
$project = new Project($projectid);
Expand Down Expand Up @@ -235,7 +239,7 @@ function get_indicated_LPage(
return new LPage($project, $imagefile, $page_state, $reverting_to_orig);
}

function project_continuity_test($project, $orig_state, $no_more_pages)
function project_continuity_test(Project $project, string $orig_state, bool $no_more_pages): ?string
{
$curr_state = $project->state;
if ($curr_state != $orig_state) {
Expand All @@ -252,6 +256,7 @@ function project_continuity_test($project, $orig_state, $no_more_pages)
}
return $err;
}
return null;
}

/**
Expand Down Expand Up @@ -290,7 +295,7 @@ class LPage
public int $reverting_to_orig;
public Round $round;

public function __construct($project, $imagefile, $page_state, $reverting_to_orig)
public function __construct(Project $project, string $imagefile, string $page_state, int $reverting_to_orig)
{
$this->project = $project;
$this->projectid = $project->projectid;
Expand All @@ -307,15 +312,15 @@ class LPage

// -------------------------------------------------------------------------

public function can_be_reverted_to_last_save()
public function can_be_reverted_to_last_save(): bool
{
return ($this->reverting_to_orig == 1
|| $this->page_state == $this->round->page_save_state
|| $this->page_state == $this->round->page_temp_state
);
}

public function get_text()
public function get_text(): string
{
if ($this->reverting_to_orig == 1
|| $this->page_state == $this->round->page_avail_state
Expand All @@ -328,6 +333,7 @@ class LPage
return Page_getText($this->projectid, $this->imagefile, $desired_column_name);
}

/** @return array|string */
public function get_filename()
{
return pathinfo($this->imagefile, PATHINFO_FILENAME);
Expand All @@ -347,12 +353,12 @@ class LPage
return $round_info_array;
}

public function get_language()
public function get_language(): ?string
{
return langcode2_for_langname($this->project->languages[0]);
}

public function get_username_for_round($round)
public function get_username_for_round(Round $round): string
{
$res = DPDatabase::query("
SELECT {$round->user_column_name}
Expand All @@ -365,13 +371,13 @@ class LPage

// -------------------------------------------------------------------------

public function checkout($user)
public function checkout(string $user): void
{
$this->page_state =
Page_checkout($this->projectid, $this->imagefile, $this->round, $user);
}

public function saveAsInProgress($page_text, $user)
public function saveAsInProgress(string $page_text, string $user): void
{
$this->page_state = Page_saveAsInProgress($this->projectid, $this->imagefile, $this->round, $user, $page_text);
$this->reverting_to_orig = 0;
Expand All @@ -387,8 +393,10 @@ class LPage
* Note that `[FALSE, FALSE]` is currently impossible:
* - if this function says that the page was not saved,
* - it can only be because the user already reached the daily page limit.
*
* @return array{0:bool, 1:bool}
*/
public function attemptSaveAsDone($text_data, $pguser)
public function attemptSaveAsDone(string $text_data, string $pguser): array
{
if ($this->round->has_a_daily_page_limit()) {
$pre_save_dpl_count = get_dpl_count_for_user_in_round($pguser, $this->round);
Expand All @@ -412,18 +420,18 @@ class LPage
return [true, $dpl_has_now_been_reached];
}

public function returnToRound($pguser)
public function returnToRound(string $pguser): void
{
$this->page_state =
Page_returnToRound($this->projectid, $this->imagefile, $this->round, $pguser);
}

public function revertToOriginal()
public function revertToOriginal(): void
{
$this->reverting_to_orig = 1;
}

public function revertToSaved()
public function revertToSaved(): void
{
$this->page_state = $this->round->page_temp_state;
$this->reverting_to_orig = 0;
Expand Down Expand Up @@ -466,7 +474,7 @@ class LPage

// -----------------------------------------------------------------------------

public function resume_page($pguser)
public function resume_page(string $pguser): void
{
if ($this->page_state == $this->round->page_save_state) {
// Page comes from DONE.
Expand Down
2 changes: 1 addition & 1 deletion pinc/ProofProject.inc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ class ProofProjectPage extends LPage
parent::__construct($proof_project->project, $project_page->page_name, $project_page->page_state, 0);
}

public function pp_checkout($user): array
public function pp_checkout(string $user): array
{
parent::checkout($user);
return $this->render_page_data();
Expand Down
42 changes: 21 additions & 21 deletions tools/proofers/PPage.inc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ include_once($relPath.'links.inc');

// "pi" = "proofing interface"

function url_for_pi_do_whichever_page($projectid, $proj_state, $escape_amp = false)
function url_for_pi_do_whichever_page(string $projectid, string $proj_state, bool $escape_amp = false): string
{
global $code_url;
if ($escape_amp) {
Expand All @@ -24,7 +24,7 @@ function url_for_pi_do_whichever_page($projectid, $proj_state, $escape_amp = fal
. "proj_state=$proj_state";
}

function url_for_pi_do_particular_page($projectid, $proj_state, $imagefile, $page_state, $escape_amp = false)
function url_for_pi_do_particular_page(string $projectid, string $proj_state, string $imagefile, string $page_state, bool $escape_amp = false): string
{
global $code_url;
if ($escape_amp) {
Expand Down Expand Up @@ -72,21 +72,21 @@ class PPage
public LPage $lpage;
private string $proj_state;

public function __construct(&$lpage, $proj_state)
public function __construct(LPage &$lpage, string $proj_state)
{
$this->lpage = & $lpage;
$this->proj_state = $proj_state;
}

// -----------------------------------------------------

public function url_for_image()
public function url_for_image(): string
{
global $projects_url;
return "$projects_url/{$this->lpage->projectid}/{$this->lpage->imagefile}";
}

public function url_for_display_image($escape_amp = false)
public function url_for_display_image(bool $escape_amp = false): string
{
global $code_url;
if ($escape_amp) {
Expand All @@ -100,7 +100,7 @@ class PPage
. "imagefile={$this->lpage->imagefile}";
}

public function url_for_do_another_page($escape_amp = false)
public function url_for_do_another_page(bool $escape_amp = false): string
{
global $code_url;
if ($escape_amp) {
Expand All @@ -115,7 +115,7 @@ class PPage
. "proj_state={$this->proj_state}";
}

public function url_for_project_comments($escape_amp = false)
public function url_for_project_comments(bool $escape_amp = false): string
{
global $code_url;
if ($escape_amp) {
Expand All @@ -131,7 +131,7 @@ class PPage
. "#project-comments";
}

public function urlencoded($escape_amp = false)
public function urlencoded(bool $escape_amp = false): string
{
if ($escape_amp) {
$amp = '&';
Expand All @@ -152,7 +152,7 @@ class PPage

// -----------------------------------------------------

public function echo_hidden_fields()
public function echo_hidden_fields(): void
{
foreach ([
'imagefile' => $this->lpage->imagefile,
Expand All @@ -166,7 +166,7 @@ class PPage
}
}

public function echo_info()
public function echo_info(): void
{
$round_info_array = $this->lpage->get_info();

Expand Down Expand Up @@ -201,7 +201,7 @@ class PPage
return $round_user_string;
}

public function echo_proofing_textarea()
public function echo_proofing_textarea(): void
{
$user = User::load_current();

Expand Down Expand Up @@ -261,7 +261,7 @@ class PPage
* or prevent further saves.
* If there's a problem, this function does not return to the caller.
*/
public function attempt_to_save_as_done($text_data)
public function attempt_to_save_as_done(string $text_data): void
{
global $code_url, $pguser;

Expand Down Expand Up @@ -323,47 +323,47 @@ class PPage

// The remaining functions just delegate to $this->lpage...

public function revertToOriginal()
public function revertToOriginal(): void
{
$this->lpage->revertToOriginal();
}

public function revertToSaved()
public function revertToSaved(): void
{
$this->lpage->revertToSaved();
}

public function returnToRound($user)
public function returnToRound(string $user): void
{
$this->lpage->returnToRound($user);
}

public function markAsBad(string $user, int $reason)
public function markAsBad(string $user, int $reason): bool
{
return $this->lpage->markAsBad($user, $reason);
}

public function saveAsInProgress($page_text, $user)
public function saveAsInProgress(string $page_text, string $user): void
{
$this->lpage->saveAsInProgress($page_text, $user);
}

public function attemptSaveAsDone($page_text, $user)
public function attemptSaveAsDone(string $page_text, string $user): array
{
return $this->lpage->attemptSaveAsDone($page_text, $user);
}

public function can_be_reverted_to_last_save()
public function can_be_reverted_to_last_save(): bool
{
return $this->lpage->can_be_reverted_to_last_save();
}

public function projectid()
public function projectid(): string
{
return $this->lpage->projectid;
}

public function imagefile()
public function imagefile(): string
{
return $this->lpage->imagefile;
}
Expand Down

0 comments on commit 9959c12

Please sign in to comment.