From bad98a9301c90f8866143e2889cb9dffeac414da Mon Sep 17 00:00:00 2001 From: Ray Papworth Date: Mon, 27 Nov 2023 15:46:40 +0000 Subject: [PATCH 01/11] Proofreading api with single page checkout escape in sql and rename function pagefunction -> pageaction and descriptions expanded renaming in yaml file escape page name in sql extract project and page states from request in v1_projects page function -> page action good bad text -> valid/invalid receive_text at higher level separate functions for save_as_in_progress and save_and_revert add validators for project state and page state move some validation and rename functions rename actions, revise Swagger file new page validator use LPage and DPage functions use LPage function for 'saved' and add langDir move more page state validation to validator and simplify ProofProjectPage constructor move project state validation to validator simlify ProofProject constructor re-organise exception handling remove unnecessary includes add Forbidden to Swagger file document responses for page actions remove unused line change names to be consistent with yaml file change text checking functions to static or member correct conflict resolution php fix phpstan adjustments correct mistake rebase to master and replace $PAGE_STATES_IN_ORDER escape pagename replace PROJECT_STATES_IN_ORDER move query parameter validators into functions in v1_projects remove exceptions which can only be caused by client code errors remove unused global variable text checker to not be static throw daily page limit exception only when exceeded and do not save as in progress. avoid normalising text twice, remove unused function rename functions like api actions revisions to OpenAPI file and change to language_direction remove unnecessary break found by phpstan use LPage get_info() and get_filename() remove duplicated member variables rename page_name and remove member variable use LPage checkout function use LPage resume_page() function rename the ProofProject function to page_resume because it returns an array and resume_page() doesn't re-arrange logic for attempt_checkin() rename functions and add return types return a status code for save_as_done so client can detect the status add status to yaml file --- SETUP/tests/ApiTest.php | 568 +++++++++++++++++++++++++++++++++ SETUP/tests/ProjectUtils.inc | 8 + api/dp-openapi.yaml | 258 +++++++++++++++ api/exceptions.inc | 13 + api/v1.inc | 5 + api/v1_projects.inc | 116 ++++++- api/v1_validators.inc | 8 +- pinc/Project.inc | 61 ++++ pinc/ProofProject.inc | 178 +++++++++++ pinc/User.inc | 16 + tools/proofers/processtext.php | 2 +- 11 files changed, 1225 insertions(+), 8 deletions(-) create mode 100644 pinc/ProofProject.inc diff --git a/SETUP/tests/ApiTest.php b/SETUP/tests/ApiTest.php index 29e859fda9..ad42414eb4 100644 --- a/SETUP/tests/ApiTest.php +++ b/SETUP/tests/ApiTest.php @@ -4,6 +4,75 @@ class ApiTest extends ProjectUtils { + //--------------------------------------------------------------------------- + // helper functions + + protected function checkout(string $projectid, string $project_state): array + { + $path = "v1/projects/$projectid/checkout"; + $router = ApiRouter::get_router(); + $_SERVER["REQUEST_METHOD"] = "PUT"; + return $router->route($path, ['state' => $project_state]); + } + + protected function resume(string $projectid, string $project_state, string $page_name, string $page_state): array + { + return $this->api_project_page($projectid, $project_state, $page_name, $page_state, [], "resume"); + } + + protected function return_to_round(string $projectid, string $project_state, string $page_name, string $page_state): void + { + $this->api_project_page($projectid, $project_state, $page_name, $page_state, [], "abandon"); + } + + protected function save_as_in_progress(string $projectid, string $project_state, string $page_name, string $page_state, string $text): array + { + return $this->api_project_page($projectid, $project_state, $page_name, $page_state, ["text" => $text], "save"); + } + + protected function save_revert(string $projectid, string $project_state, string $page_name, string $page_state, string $text): array + { + return $this->api_project_page($projectid, $project_state, $page_name, $page_state, ["text" => $text], "revert"); + } + + protected function save_as_done(string $projectid, string $project_state, string $page_name, string $page_state, string $text): array + { + return $this->api_project_page($projectid, $project_state, $page_name, $page_state, ["text" => $text], "checkin"); + } + + protected function get_text(string $projectid, string $project_state, string $page_name, string $page_state): array + { + $_SERVER["REQUEST_METHOD"] = "GET"; + $path = "v1/projects/$projectid/pages/$page_name"; + $router = ApiRouter::get_router(); + return $router->route($path, ['state' => $project_state, 'pagestate' => $page_state]); + } + + private function api_project_page(string $projectid, string $project_state, string $page_name, string $page_state, array $request_array = [], string $action) + { + global $request_body; + + $_SERVER["REQUEST_METHOD"] = "PUT"; + $request_body = $request_array; + $path = "v1/projects/$projectid/pages/$page_name"; + $router = ApiRouter::get_router(); + return $router->route($path, ['state' => $project_state, 'pagestate' => $page_state, 'pageaction' => $action]); + } + + protected function validate_text(string $projectid, string $text): array + { + global $request_body; + + $request_body = ["text" => $text]; + $_SERVER["REQUEST_METHOD"] = "PUT"; + $path = "v1/projects/$projectid/validatetext"; + $router = ApiRouter::get_router(); + return $router->route($path, []); + } + + //--------------------------------------------------------------------------- + // tests + public function test_get_invalid_project_info() { $this->expectExceptionCode(101); @@ -91,6 +160,505 @@ public function test_create_project_no_data() $_SERVER["REQUEST_METHOD"] = "POST"; $router->route($path, $query_params); } + + //--------------------------------------------------------------------------- + // tests for proofreading + + public function test_checkout_bad_projectid() + { + global $pguser; + $pguser = $this->TEST_USERNAME; + + $this->expectExceptionCode(101); + + $this->checkout("projectId", "P1.proj_avail"); + } + + public function test_checkout_invalid_proj_state() + { + global $pguser; + $pguser = $this->TEST_USERNAME; + + $this->expectExceptionCode(6); + + $project = $this->_create_project(); + $this->checkout($project->projectid, "project_ne"); + } + + public function test_project_no_state_given() + { + global $pguser; + + $this->expectExceptionCode(6); + + $project = $this->_create_project(); + $pguser = $this->TEST_USERNAME; + + $path = "v1/projects/$project->projectid/checkout"; + $router = ApiRouter::get_router(); + $_SERVER["REQUEST_METHOD"] = "PUT"; + $router->route($path, []); + } + + public function test_checkout_wrong_proj_state() + { + global $pguser; + $pguser = $this->TEST_USERNAME; + + $this->expectExceptionCode(120); + + $project = $this->_create_project(); + $this->checkout($project->projectid, "P1.proj_bad"); + } + + public function test_project_not_in_round() + { + global $pguser; + + $this->expectExceptionCode(110); + + $project = $this->_create_project(); + $pguser = $this->TEST_USERNAME; + + $this->checkout($project->projectid, "project_new"); + } + + public function test_project_unavailable() + { + global $pguser; + + $this->expectExceptionCode(112); + + $project = $this->_create_project(); + $pguser = $this->TEST_USERNAME; + $this->quiet_project_transition($project->projectid, PROJ_P1_UNAVAILABLE, $this->TEST_USERNAME_PM); + + $this->checkout($project->projectid, "P1.proj_unavail"); + } + + public function test_user_not_qualified_for_round() + { + global $pguser; + $pguser = $this->TEST_USERNAME; + // see comment in SettingsClass.inc about calling by reference + $settings = & Settings::get_settings($this->TEST_USERNAME); + $settings->set_boolean("P2.access", false); + + $this->expectExceptionCode(302); + + $project = $this->_create_available_project(); + $this->advance_to_round2($project->projectid); + + $this->checkout($project->projectid, "P2.proj_avail"); + } + + public function test_project_checkout_available() + { + global $pguser; + + $project = $this->_create_available_project(); + $pguser = $this->TEST_USERNAME; + + $response = $this->checkout($project->projectid, "P1.proj_avail"); + $expected = [ + 'pagename' => '001.png', + 'image_url' => "{$project->url}/001.png", + 'text' => $this->TEST_TEXT, + 'pagestate' => "P1.page_out", + 'saved' => false, + 'pagenum' => '001', + 'round_info' => [], + 'language_direction' => 'LTR', + ]; + $this->assertEquals($expected, $response); + } + + public function test_many_pages_few_days() + { + // user done many pages but few days on site + // $page_tally_threshold 500 for new projects in reserve time + + global $pguser; + $project = $this->_create_available_project(); + $pguser = $this->TEST_USERNAME; + page_tallies_add("P1", $pguser, 501); + $response = $this->checkout($project->projectid, "P1.proj_avail"); + $this->assertEquals('001.png', $response['pagename']); + } + + public function test_few_pages_many_days() + { + global $pguser; + $project = $this->_create_available_project(); + $pguser = $this->TEST_OLDUSERNAME; + $response = $this->checkout($project->projectid, "P1.proj_avail"); + $this->assertEquals('001.png', $response['pagename']); + } + + public function test_many_pages_many_days() + { + global $pguser; + $project = $this->_create_available_project(); + $pguser = $this->TEST_OLDUSERNAME; + page_tallies_add("P1", $pguser, 501); + + // reserved for new proofreaders + $this->expectExceptionCode(306); + $this->checkout($project->projectid, "P1.proj_avail"); + } + + public function test_beginner_project_checkout() + { + global $pguser; + + $this->valid_project_data["difficulty"] = "beginner"; + $project = $this->_create_available_project(); + $pguser = $this->TEST_USERNAME; + // beginners limit is 40 in user_is.inc + page_tallies_add("P1", $pguser, 50); + + $this->expectExceptionCode(303); + $this->checkout($project->projectid, "P1.proj_avail"); + } + + public function test_beginner_mentor_project_checkout() + { + global $pguser; + + $this->expectExceptionCode(305); + + $this->valid_project_data["difficulty"] = "beginner"; + $project = $this->_create_available_project(); + $this->advance_to_round2($project->projectid); + $pguser = $this->TEST_USERNAME; + $settings = & Settings::get_settings($this->TEST_USERNAME); + $settings->set_boolean("P2.access", true); + + $this->checkout($project->projectid, "P2.proj_avail"); + } + + public function test_resume_page() + { + global $pguser; + + $project = $this->_create_available_project(); + $pguser = $this->TEST_USERNAME; + + $this->checkout($project->projectid, "P1.proj_avail"); + $response = $this->resume($project->projectid, 'P1.proj_avail', '001.png', 'P1.page_out'); + $expected = + [ + 'pagename' => '001.png', + 'image_url' => "{$project->url}/001.png", + 'text' => $this->TEST_TEXT, + 'pagestate' => "P1.page_out", + 'saved' => false, + 'pagenum' => '001', + 'round_info' => [], + 'language_direction' => 'LTR', + ]; + + $this->assertEquals($expected, $response); + } + + public function test_project_return_to_round() + { + global $pguser; + + $project = $this->_create_available_project(); + $pguser = $this->TEST_USERNAME; + + // check out a page + $this->checkout($project->projectid, "P1.proj_avail"); + + // return it to round + $response = $this->return_to_round($project->projectid, 'P1.proj_avail', '001.png', 'P1.page_out'); + $this->assertEquals(null, $response); + } + + public function test_return_page_no_state() + { + global $pguser; + + $pguser = $this->TEST_USERNAME; + $project = $this->_create_available_project(); + + $this->expectExceptionCode(6); + + $_SERVER["REQUEST_METHOD"] = "PUT"; + $path = "v1/projects/$project->projectid/pages/001.png"; + $router = ApiRouter::get_router(); + $router->route($path, ['state' => "P1.proj_avail", 'pageaction' => "abandon"]); + } + + public function test_no_page_action() + { + global $pguser; + + $pguser = $this->TEST_USERNAME; + $project = $this->_create_available_project(); + // check out a page + $this->checkout($project->projectid, "P1.proj_avail"); + + $this->expectExceptionCode(2); + + $_SERVER["REQUEST_METHOD"] = "PUT"; + $path = "v1/projects/$project->projectid/pages/001.png"; + $router = ApiRouter::get_router(); + $router->route($path, ['state' => "P1.proj_avail", 'pagestate' => "P1.page_out"]); + } + + public function test_invalid_page_action() + { + global $pguser; + + $pguser = $this->TEST_USERNAME; + $project = $this->_create_available_project(); + // check out a page + $this->checkout($project->projectid, "P1.proj_avail"); + + $this->expectExceptionCode(2); + + $_SERVER["REQUEST_METHOD"] = "PUT"; + $path = "v1/projects/$project->projectid/pages/001.png"; + $router = ApiRouter::get_router(); + $router->route($path, ['state' => "P1.proj_avail", 'pagestate' => "P1.page_out", 'pageaction' => "revoke"]); + } + + public function test_return_non_existent_page() + { + global $pguser; + + $pguser = $this->TEST_USERNAME; + $project = $this->_create_available_project(); + + $this->expectExceptionCode(104); + $this->return_to_round($project->projectid, "P1.proj_avail", "002.png", "P1.page_out"); + } + + public function test_return_invalid_page_state() + { + global $pguser; + + $this->expectExceptionCode(6); + + $pguser = $this->TEST_USERNAME; + $project = $this->_create_available_project(); + + $this->return_to_round($project->projectid, "P1.proj_avail", "001.png", "P1.page_in"); + } + + public function test_return_page_state_not_as_in_project() + { + global $pguser; + + $this->expectExceptionCode(119); + + $pguser = $this->TEST_USERNAME; + $project = $this->_create_available_project(); + + $this->return_to_round($project->projectid, "P1.proj_avail", "001.png", "P1.page_out"); + } + + public function test_return_page_state_not_allowed() + { + global $pguser; + + $pguser = $this->TEST_USERNAME; + $project = $this->_create_available_project(); + $this->checkout($project->projectid, "P1.proj_avail"); + + $this->return_to_round($project->projectid, "P1.proj_avail", "001.png", "P1.page_out"); + + $this->expectExceptionCode(115); + // user still appears to own this page. Can't return from available. + $this->return_to_round($project->projectid, "P1.proj_avail", "001.png", "P1.page_avail"); + } + + public function test_return_page_not_owned() + { + global $pguser; + + $this->expectExceptionCode(307); + + $pguser = $this->TEST_USERNAME; + $project = $this->_create_available_project(); + + // check out a page + $this->checkout($project->projectid, "P1.proj_avail"); + + // let another user return it to round + $pguser = $this->TEST_USERNAME_PM; + $this->return_to_round($project->projectid, "P1.proj_avail", "001.png", "P1.page_out"); + } + + public function test_save_as_in_progress() + { + global $pguser; + + $pguser = $this->TEST_USERNAME; + $project = $this->_create_available_project(); + + // check out a page + $this->checkout($project->projectid, "P1.proj_avail"); + + // save as in progress + $response = $this->save_as_in_progress($project->projectid, "P1.proj_avail", "001.png", "P1.page_out", $this->TEST_MODIFIED_TEXT); + $expected = [ + "text" => $this->TEST_MODIFIED_TEXT, + 'pagestate' => "P1.page_temp", + 'saved' => true, + ]; + + $this->assertEquals($expected, $response); + + // get original text back + $response = $this->save_revert($project->projectid, "P1.proj_avail", "001.png", "P1.page_temp", $this->TEST_MODIFIED_TEXT); + $this->assertEquals($this->TEST_TEXT, $response["text"]); + + // get last saved text + $response = $this->get_text($project->projectid, "P1.proj_avail", "001.png", "P1.page_temp"); + $this->assertEquals(["text" => $this->TEST_MODIFIED_TEXT], $response); + + // save invalid text + $this->expectExceptionCode(125); + + $this->save_as_in_progress($project->projectid, "P1.proj_avail", "001.png", "P1.page_temp", "This is a bĀd test file"); + } + + public function test_save_no_text() + { + global $pguser; + + $pguser = $this->TEST_USERNAME; + $project = $this->_create_available_project(); + + // check out a page + $this->checkout($project->projectid, "P1.proj_avail"); + + // save as in progress + $this->expectExceptionCode(6); + $this->api_project_page($project->projectid, "P1.proj_avail", "001.png", "P1.page_out", [], "save"); + } + + public function test_save_as_done() + { + global $pguser; + + $pguser = $this->TEST_USERNAME; + $project = $this->_create_available_project(2); + + // check out a page + $this->checkout($project->projectid, "P1.proj_avail"); + + // get user's tally + $user = new User($pguser); + $user_tallyboard = new TallyBoard("P1", 'U'); + $tally = $user_tallyboard->get_current_tally($user->u_id); + + // save as done + $response = $this->save_as_done($project->projectid, "P1.proj_avail", "001.png", "P1.page_out", $this->TEST_MODIFIED_TEXT); + $this->assertEquals(0, $response["status"]); + + // check tally has increased by one + $this->assertEquals(1, $user_tallyboard->get_current_tally($user->u_id) - $tally); + + // reopen + $response = $this->resume($project->projectid, "P1.proj_avail", "001.png", "P1.page_saved"); + $expected = + [ + 'pagename' => '001.png', + 'image_url' => "{$project->url}/001.png", + 'text' => $this->TEST_MODIFIED_TEXT, + 'pagestate' => "P1.page_temp", + 'saved' => true, + 'pagenum' => '001', + 'round_info' => [], + 'language_direction' => 'LTR', + ]; + + $this->assertEquals($expected, $response); + // check tally back to original value + $this->assertEquals($user_tallyboard->get_current_tally($user->u_id), $tally); + } + + public function test_daily_limit() + { + global $pguser; + + $pguser = $this->TEST_USERNAME; + $project = $this->_create_available_project(2); + + // check out a page + $this->checkout($project->projectid, "P1.proj_avail"); + + $round = get_Round_for_round_id("P1"); + $old_limit = $round->daily_page_limit; + $round->daily_page_limit = 1; + // check reaching limit + $response = $this->save_as_done($project->projectid, "P1.proj_avail", '001.png', "P1.page_out", $this->TEST_MODIFIED_TEXT); + $this->assertEquals(1, $response["status"]); + + // checkout another page + $this->checkout($project->projectid, "P1.proj_avail"); + $this->expectExceptionCode(308); + $this->save_as_done($project->projectid, "P1.proj_avail", '002.png', "P1.page_out", $this->TEST_MODIFIED_TEXT); + + // reset daily limit + $round->daily_page_limit = $old_limit; + } + + public function test_info_for_previous_proofreaders() + { + global $pguser; + + $project = $this->_create_available_project(); + $pguser = $this->TEST_USERNAME; + $this->checkout($project->projectid, "P1.proj_avail"); + $this->save_as_done($project->projectid, "P1.proj_avail", '001.png', "P1.page_out", $this->TEST_MODIFIED_TEXT); + + $this->advance_to_round2($project->projectid); + $pguser = $this->TEST_OLDUSERNAME; + $settings = & Settings::get_settings($this->TEST_OLDUSERNAME); + $settings->set_boolean("P2.access", true); + $response = $this->checkout($project->projectid, "P2.proj_avail"); + $info = $response['round_info']; + $this->assertEquals(1, count($info)); + $this->assertEquals('P1', $info[0]->round_id); + $this->assertEquals($this->TEST_USERNAME, $info[0]->username); + + $this->save_as_done($project->projectid, "P2.proj_avail", '001.png', "P2.page_out", $this->TEST_MODIFIED_TEXT); + + $this->advance_to_round3($project->projectid); + $pguser = $this->TEST_USERNAME_PM; + $settings = & Settings::get_settings($this->TEST_USERNAME_PM); + $settings->set_boolean("P3.access", true); + $response = $this->checkout($project->projectid, "P3.proj_avail"); + $info = $response['round_info']; + $this->assertEquals(2, count($info)); + $this->assertEquals('P1', $info[0]->round_id); + $this->assertEquals($this->TEST_USERNAME, $info[0]->username); + $this->assertEquals('P2', $info[1]->round_id); + $this->assertEquals($this->TEST_OLDUSERNAME, $info[1]->username); + } + + public function test_validate_text() + { + $project = $this->_create_available_project(); + $response = $this->validate_text($project->projectid, "This is an invĀlid test file"); + $expected = [ + 'valid' => false, + 'mark_array' => [["This is an inv", 0], ["Ā", 1], ["lid test file", 0]], + ]; + $this->assertEquals($expected, $response); + + $response = $this->validate_text($project->projectid, "This is a valid test file"); + $expected = [ + 'valid' => true, + 'mark_array' => [["This is a valid test file", 0]], + ]; + $this->assertEquals($expected, $response); + } } // this mocks the function in index.php diff --git a/SETUP/tests/ProjectUtils.inc b/SETUP/tests/ProjectUtils.inc index ce917f2998..7b89d19d5b 100644 --- a/SETUP/tests/ProjectUtils.inc +++ b/SETUP/tests/ProjectUtils.inc @@ -19,6 +19,7 @@ class ProjectUtils extends PHPUnit\Framework\TestCase ]; protected $created_projectids = []; protected $TEST_TEXT = "This is a test file"; + protected $TEST_MODIFIED_TEXT = "This is a modified test file"; protected function setUp(): void { @@ -94,6 +95,13 @@ class ProjectUtils extends PHPUnit\Framework\TestCase $this->quiet_project_transition($projectid, PROJ_P2_AVAILABLE, PT_AUTO); } + protected function advance_to_round3($projectid) + { + $this->quiet_project_transition($projectid, PROJ_P2_COMPLETE, PT_AUTO); + $this->quiet_project_transition($projectid, PROJ_P3_WAITING_FOR_RELEASE, PT_AUTO); + $this->quiet_project_transition($projectid, PROJ_P3_AVAILABLE, PT_AUTO); + } + protected function _create_project_with_pages($npage = 1) { $project = $this->_create_project(); diff --git a/api/dp-openapi.yaml b/api/dp-openapi.yaml index a884f527b8..33f55c42d7 100644 --- a/api/dp-openapi.yaml +++ b/api/dp-openapi.yaml @@ -924,6 +924,138 @@ paths: default: $ref: '#/components/responses/UnexpectedError' + /projects/{projectid}/checkout: + put: + tags: + - project + description: Check out a page for proofreading. + parameters: + - $ref: '#/components/parameters/projectid' + - $ref: '#/components/parameters/state_query' + responses: + 200: + description: page data + content: + application/json: + schema: + $ref: '#/components/schemas/proof_page' + '400': + $ref: '#/components/responses/InvalidValue' + '401': + $ref: '#/components/responses/Unauthorized' + '403': + $ref: '#/components/responses/Forbidden' + '404': + $ref: '#/components/responses/NotFound' + '429': + $ref: '#/components/responses/RateLimitExceeded' + default: + $ref: '#/components/responses/UnexpectedError' + + /projects/{projectid}/validatetext: + put: + tags: + - project + description: Checks the given text for invalid characters, returns data indicating them + parameters: + - $ref: '#/components/parameters/projectid' + requestBody: + $ref: '#/components/requestBodies/page_text' + responses: + 200: + description: analysis data + content: + application/json: + schema: + $ref: '#/components/schemas/text_analysis' + '400': + $ref: '#/components/responses/InvalidValue' + '401': + $ref: '#/components/responses/Unauthorized' + '404': + $ref: '#/components/responses/NotFound' + '429': + $ref: '#/components/responses/RateLimitExceeded' + default: + $ref: '#/components/responses/UnexpectedError' + + /projects/{projectid}/pages/{pagename}: + put: + tags: + - project + description: Various operations on the page as determined by the 'pageaction_query' parameter + parameters: + - $ref: '#/components/parameters/projectid' + - $ref: '#/components/parameters/state_query' + - $ref: '#/components/parameters/pagename' + - $ref: '#/components/parameters/pagestate_query' + - name: pageaction_query + in: query + description: Action to perform on the page. +

+ resume - If the page is saved-as-done 'unsaves' it. Response:- proof_page object. +

+ checkin - Checks the page for invalid characters and if valid, saves it as done. Response:- checkin_response object. +

+ save - Checks the page for invalid characters and if valid, saves it as in progress. Response:- page_text_state object. +

+ revert - Checks the page for invalid characters and if valid, saves it as in progress. Response:- page_text_state object. +

+ abandon - Returns the page to the round, response:- none. + required: true + schema: + type: string + requestBody: + $ref: '#/components/requestBodies/page_text' + responses: + 200: + description: Page data + content: + application/json: + schema: + oneOf: + - $ref: '#/components/schemas/checkin_response' + - $ref: '#/components/schemas/proof_page' + - $ref: '#/components/schemas/page_text_state' + '400': + $ref: '#/components/responses/InvalidValue' + '401': + $ref: '#/components/responses/Unauthorized' + '403': + $ref: '#/components/responses/Forbidden' + '404': + $ref: '#/components/responses/NotFound' + '429': + $ref: '#/components/responses/RateLimitExceeded' + default: + $ref: '#/components/responses/UnexpectedError' + get: + tags: + - project + description: Gets the current (last saved) page text + parameters: + - $ref: '#/components/parameters/projectid' + - $ref: '#/components/parameters/state_query' + - $ref: '#/components/parameters/pagename' + - $ref: '#/components/parameters/pagestate_query' + responses: + 200: + description: Page text + content: + application/json: + schema: + $ref: '#/components/schemas/proof_page_text' + '400': + $ref: '#/components/responses/InvalidValue' + '401': + $ref: '#/components/responses/Unauthorized' + '404': + $ref: '#/components/responses/NotFound' + '429': + $ref: '#/components/responses/RateLimitExceeded' + default: + $ref: '#/components/responses/UnexpectedError' + /stats/site: get: tags: @@ -1009,6 +1141,50 @@ components: in: header name: X-API-KEY + parameters: + projectid: + name: projectid + in: path + description: ID of project + required: true + schema: + type: string + + state_query: + name: state + in: query + description: Project state + required: true + schema: + type: string + + pagename: + name: pagename + in: path + description: Name of page + required: true + schema: + type: string + + pagestate_query: + name: pagestate + in: query + description: Page state + required: true + schema: + type: string + + requestBodies: + page_text: + description: Text to save or analyse + content: + application/json: + schema: + type: object + properties: + text: + type: string + schemas: project: required: @@ -1370,6 +1546,82 @@ components: items: type: string + proof_page_text: + type: object + properties: + text: + description: The page text from current round. + type: string + + proof_page: + type: object + properties: + text: + description: The page text. If page state is 'out' from OCR or previous round, else from current round + type: string + pagestate: + description: The page state, for checking continuity. Either out or temp + type: string + saved: + description: True if the page state is 'temp' + type: boolean + pagename: + description: The filename of the page with extension + type: string + image_url: + description: URL of the image of the page + type: string + language_direction: + description: left-to-right or right-to-left + type: string + enum: [RTL, LTR] + pagenum: + description: Page "number" without extension + type: string + round_info_array: + description: Array of names of proofreaders in earlier rounds + type: array + items: + type: string + + checkin_response: + type: object + properties: + message: + description: message if reached limit, else empty string. + type: string + status: + description: 1 if limit reached, else 0 + type: integer + + page_text_state: + type: object + properties: + text: + description: The page text, current or original. + type: string + pagestate: + description: The page state, it is now in a 'temp' state since this is returned by save-as-in-progress or save-and-revert-to-original + type: string + saved: + description: Now true (since page has been saved) + type: boolean + enum: [true] + + text_analysis: + type: object + properties: + valid: + type: boolean + mark_array: + type: array + items: + type: array + items: + oneOf: + - type: string + - type: integer + Error: required: - message @@ -1394,6 +1646,12 @@ components: application/json: schema: $ref: '#/components/schemas/Error' + Forbidden: + description: Forbidden + content: + application/json: + schema: + $ref: '#/components/schemas/Error' InvalidValue: description: Invalid Value content: diff --git a/api/exceptions.inc b/api/exceptions.inc index 0756310038..0bbe046c77 100644 --- a/api/exceptions.inc +++ b/api/exceptions.inc @@ -81,6 +81,19 @@ class InvalidValue extends ApiException } } +class ForbiddenError extends ApiException +{ + public function __construct($message = "Forbidden", $code = 7) + { + parent::__construct($message, $code); + } + + public function getStatusCode() + { + return 403; + } +} + //--------------------------------------------------------------------------- // Exceptions that shouldn't happen unless someone is futzing with something // they shouldn't be, or there's an unexpected problem. diff --git a/api/v1.inc b/api/v1.inc index 9e80f7d2cc..e379193c1d 100644 --- a/api/v1.inc +++ b/api/v1.inc @@ -46,6 +46,11 @@ $router->add_route("GET", "v1/queues/:queueid/stats", "api_v1_queue_stats"); $router->add_route("GET", "v1/queues/:queueid/projects", "api_v1_queue_projects"); +$router->add_route("PUT", "v1/projects/:projectid/checkout", "api_v1_project_checkout"); +$router->add_route("PUT", "v1/projects/:projectid/validatetext", "api_v1_project_validatetext"); +$router->add_route("PUT", "v1/projects/:projectid/pages/:pagename", "api_v1_project_page"); +$router->add_route("GET", "v1/projects/:projectid/pages/:pagename", "api_v1_project_page"); + $router->add_route("GET", "v1/stats/site", "api_v1_stats_site"); $router->add_route("GET", "v1/stats/site/projects/stages", "api_v1_stats_site_projects_stages"); $router->add_route("GET", "v1/stats/site/projects/states", "api_v1_stats_site_projects_states"); diff --git a/api/v1_projects.inc b/api/v1_projects.inc index 4145e4b5b9..41599ff558 100644 --- a/api/v1_projects.inc +++ b/api/v1_projects.inc @@ -4,6 +4,7 @@ include_once($relPath.'wordcheck_engine.inc'); include_once($relPath.'ProjectSearchForm.inc'); include_once($relPath.'page_table.inc'); include_once($relPath.'special_colors.inc'); +include_once($relPath.'ProofProject.inc'); include_once("exceptions.inc"); include_once("api_common.inc"); @@ -548,7 +549,7 @@ function api_v1_project_page_round($method, $data, $query_params) $user_column, implode(",", $user_cols), $project->projectid, - DPDatabase::escape($data[":pagename"]) + DPDatabase::escape($data[":pagename"]->page_name) ); $result = DPDatabase::query($sql); $row = mysqli_fetch_assoc($result); @@ -770,3 +771,116 @@ function _get_enabled_filter($query_params) { return get_flag_value($query_params, "enabled"); } + +//--------------------------------------------------------------------------- +// Proofreading functions + +// checkout a page +function api_v1_project_checkout($method, array $data, array $query_params) +{ + try { + $proof_project = create_proof_project($data, $query_params); + return $proof_project->checkout(); + } catch (ProjectException $exception) { + throw new NotFoundError($exception->getMessage(), $exception->getCode()); + } catch (UserAccessException $exception) { + throw new ForbiddenError($exception->getMessage(), $exception->getCode()); + } +} + +function api_v1_project_validatetext($method, $data, array $query_params) +{ + $text_checker = new TextChecker(); + return $text_checker->analyse(receive_project_text_from_request_body('text'), $data[":projectid"]); +} + +function api_v1_project_page(string $method, array $data, array $query_params) +{ + global $pguser; + + try { + $proof_project = create_proof_project($data, $query_params); + $project_page = $data[":pagename"]; + $page_state = $query_params['pagestate'] ?? null; + + if (null === $page_state) { + throw new InvalidValue("No page state found in request."); + } + if (!in_array($page_state, Rounds::get_page_states())) { + throw new InvalidValue(sprintf("%s is not a valid page state", $page_state)); + } + if ($page_state != $project_page->page_state) { + $err = sprintf( + _('Warning: Page "%1$s" is no longer in state "%2$s"; it is now in state "%3$s".'), + $project_page->page_name, + $page_state, + $project_page->page_state + ); + throw new ProjectPageInconsistentStateException($err); + } + + $proof_project_page = new ProofProjectPage($proof_project, $project_page); + if ($method == "GET") { + return $proof_project_page->render_page_text(); + } + // must be PUT + $page_action = $query_params['pageaction'] ?? null; + if (null === $page_action) { + throw new BadRequest("No page action found in request."); + } + switch ($page_action) { + case 'resume': + return $proof_project_page->pp_resume_page(); + case 'checkin': + return $proof_project_page->attempt_checkin(receive_project_text_from_request_body('text')); + case 'save': + return $proof_project_page->save(receive_project_text_from_request_body('text')); + case 'revert': + return $proof_project_page->save_and_revert(receive_project_text_from_request_body('text')); + case 'abandon': + return $proof_project_page->returnToRound($pguser); + default: + throw new BadRequest("$page_action is not a valid page action."); + } + } catch (ProjectException $exception) { + throw new NotFoundError($exception->getMessage(), $exception->getCode()); + } catch (UserAccessException $exception) { + throw new ForbiddenError($exception->getMessage(), $exception->getCode()); + } +} + +function create_proof_project(array $data, array $query_params) +{ + $project = $data[":projectid"]; + $state = $query_params["state"] ?? null; + try { + if (null === $state) { + throw new InvalidValue("No project state found in request."); + } + if (!in_array($state, ProjectStates::get_states())) { + throw new InvalidValue("Invalid project state"); + } + if ($state != $project->state) { + $err = sprintf( + _('Warning: Project "%1$s" is not in state "%2$s"; it is now in state "%3$s".'), + $project->nameofwork, + project_states_text($state), + project_states_text($project->state) + ); + throw new ProjectNotInRequiredStateException($err); + } + } catch (ProjectException $exception) { + throw new NotFoundError($exception->getMessage(), $exception->getCode()); + } + return new ProofProject($project); +} + +function receive_project_text_from_request_body($field): string +{ + $request_data = api_get_request_body(); + $page_text = $request_data[$field] ?? null; + if (null === $page_text) { + throw new InvalidValue("There is no text"); + } + return $page_text; +} diff --git a/api/v1_validators.inc b/api/v1_validators.inc index ca5693f77e..5f37da766b 100644 --- a/api/v1_validators.inc +++ b/api/v1_validators.inc @@ -39,12 +39,8 @@ function validate_wordlist($wordlist, $data) function validate_page_name($pagename, $data) { try { - $pages = $data[":projectid"]->get_page_names_from_db(); - if (!in_array($pagename, $pages)) { - throw new NonexistentPageException("No such page in project"); - } - return $pagename; - } catch (NonexistentPageException | NoProjectPageTable $exception) { + return $data[":projectid"]->get_project_page($pagename); + } catch (NonexistentPageException $exception) { throw new NotFoundError($exception->getMessage(), $exception->getCode()); } } diff --git a/pinc/Project.inc b/pinc/Project.inc index ec7d9ddeda..0e30aa8d49 100644 --- a/pinc/Project.inc +++ b/pinc/Project.inc @@ -121,6 +121,47 @@ class ProjectNoMorePagesException extends ProjectException } } +class ProjectPageStateException extends ProjectException +{ + public function __construct($message, $code = 115) + { + parent::__construct($message, $code); + } +} + +class ProjectPageInconsistentStateException extends ProjectException +{ + public function __construct($message, $code = 119) + { + parent::__construct($message, $code); + } +} + +class ProjectNotInRequiredStateException extends ProjectException +{ + public function __construct($message, $code = 120) + { + parent::__construct($message, $code); + } +} + +class ProjectInvalidTextException extends ProjectException +{ + public function __construct($message, $code = 125) + { + parent::__construct($message, $code); + } +} + +class ProjectPage +{ + public function __construct($pagename, $pagestate) + { + $this->page_name = $pagename; + $this->page_state = $pagestate; + } +} + /** * @property-read string $url * @property-read string $dir @@ -1854,6 +1895,26 @@ class Project $this->delete_file($dir); } } + + public function get_project_page($pagename) + { + $sql = sprintf( + " + SELECT state + FROM $this->projectid + WHERE image='%s' + ", + DPDatabase::escape($pagename) + ); + $res = DPDatabase::query($sql); + $row = mysqli_fetch_row($res); + if (!$row) { + throw new NonexistentPageException("No such page in project"); + } + [$pagestate] = $row; + + return new ProjectPage($pagename, $pagestate); + } } // XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX diff --git a/pinc/ProofProject.inc b/pinc/ProofProject.inc new file mode 100644 index 0000000000..e14db32ac3 --- /dev/null +++ b/pinc/ProofProject.inc @@ -0,0 +1,178 @@ +project, $project_page->page_name, $project_page->page_state, 0); + } + + public function pp_checkout($user): array + { + parent::checkout($user); + return $this->render_page_data(); + } + + public function attempt_checkin(string $page_text): array + { + global $pguser; + + $this->validate_text($page_text); + // see comment in LPage.inc + [$saved, $dpl_reached] = $this->attemptSaveAsDone($page_text, $pguser); + + if (!$saved) { + $sentence = sprintf( + _("Your page was not checked-in because you have already reached the daily page limit for %s. You can save it and will be able to check it in after server midnight."), + $this->round->id + ); + throw new DailyLimitExceededException($sentence); + } + + if (!$dpl_reached) { + $sentence = ""; + $status_code = 0; + } else { + $sentence = sprintf( + _("Your page has been checked-in. However, you have now reached the daily page limit for %s."), + $this->round->id + ); + $status_code = 1; + } + return ["message" => $sentence, "status" => $status_code]; + } + + public function save(string $page_text): array + { + global $pguser; + $this->validate_text($page_text); + $this->saveAsInProgress($page_text, $pguser); + return $this->get_page_text_array() + $this->get_page_state_array(); + } + + public function save_and_revert(string $page_text): array + { + global $pguser; + $this->validate_text($page_text); + $this->saveAsInProgress($page_text, $pguser); + $this->revertToOriginal(); + return $this->get_page_text_array() + $this->get_page_state_array(); + } + + public function pp_resume_page(): array + { + global $pguser; + + parent::resume_page($pguser); + return $this->render_page_data(); + } + + public function render_page_text(): array + { + // validate user and page? page should be in out or temp state only + return $this->get_page_text_array(); + } + + private function render_page_data(): array + { + return + $this->get_page_text_array() + + $this->get_page_state_array() + + [ + "pagenum" => $this->get_filename(), + "round_info" => $this->get_info(), + "pagename" => $this->imagefile, + "image_url" => "{$this->project->url}/{$this->imagefile}", + "language_direction" => lang_direction(langcode2_for_langname($this->project->languages[0])), + ]; + } + + public function get_page_text_array(): array + { + return ["text" => $this->get_text()]; + } + + private function get_page_state_array(): array + { + return [ + "pagestate" => $this->page_state, + "saved" => $this->can_be_reverted_to_last_save(), + ]; + } + + private function validate_text(string $text): void + { + $pattern_string = build_character_regex_filter($this->project->get_valid_codepoints()); + foreach (split_graphemes($text) as $grapheme) { + if (1 != preg_match("/$pattern_string/u", $grapheme)) { + throw new ProjectInvalidTextException(_("The text contains characters which are not allowed for this project")); + } + } + } +} + +class ProofProject +{ + public function __construct(object $project) + { + global $pguser; + + $project_round = $project->get_project_available_round(); + $project_round->validate_user_can_access($pguser); + + $this->project = $project; + $this->round = $project_round; + } + + public function checkout(): array + { + global $pguser; + + [$page_name, $state] = get_available_proof_page_array($this->project, $this->round, $pguser); + $project_page = new ProjectPage($page_name, $state); + $proof_project_page = new ProofProjectPage($this, $project_page); + + return $proof_project_page->pp_checkout($pguser); + } +} + +class TextChecker +{ + private const VALID_TEXT = 0; + private const INVALID_TEXT = 1; + + private $text_array; + private $valid_text_block; + + private function append_valid(): void + { + if ($this->valid_text_block != "") { + $this->text_array[] = [$this->valid_text_block, self::VALID_TEXT]; + $this->valid_text_block = ""; + } + } + + public function analyse(string $page_text, object $project): array + { + $pattern_string = build_character_regex_filter($project->get_valid_codepoints()); + $this->text_array = []; + $this->valid_text_block = ""; + $all_valid = true; + foreach (split_graphemes($page_text) as $grapheme) { + if (1 === preg_match("/$pattern_string/u", $grapheme)) { + $this->valid_text_block .= $grapheme; + } else { + $all_valid = false; + $this->append_valid(); + $this->text_array[] = [$grapheme, self::INVALID_TEXT]; + } + } + // append any remaining valid text + $this->append_valid(); + return [ + "mark_array" => $this->text_array, + "valid" => $all_valid, + ]; + } +} diff --git a/pinc/User.inc b/pinc/User.inc index 48c2974329..d79b3ae94e 100644 --- a/pinc/User.inc +++ b/pinc/User.inc @@ -71,6 +71,22 @@ class ReservedForNewProofreadersException extends UserAccessException } } +class PageNotOwnedException extends UserAccessException +{ + public function __construct($message = "", $code = 307) + { + parent::__construct($message, $code); + } +} + +class DailyLimitExceededException extends UserAccessException +{ + public function __construct($message = "", $code = 308) + { + parent::__construct($message, $code); + } +} + /** * @property-read string $reg_token; * @property string $real_name; diff --git a/tools/proofers/processtext.php b/tools/proofers/processtext.php index 3ae8553b6a..a3d88f7e21 100644 --- a/tools/proofers/processtext.php +++ b/tools/proofers/processtext.php @@ -338,7 +338,7 @@ default: die("unexpected tbutton value: '$tbutton'"); } -} catch (ProjectPageException $exception) { +} catch (ProjectPageException | ProjectPageStateException | PageNotOwnedException $exception) { abort($exception->getMessage()); } From 82ee7cc0e80bf3804e4d3b003c938629e854c2ab Mon Sep 17 00:00:00 2001 From: Ray Papworth Date: Mon, 8 Apr 2024 15:41:45 +0000 Subject: [PATCH 02/11] add member declarations --- pinc/Project.inc | 5 ++++- pinc/ProofProject.inc | 7 +++++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/pinc/Project.inc b/pinc/Project.inc index 0e30aa8d49..524a3b1d55 100644 --- a/pinc/Project.inc +++ b/pinc/Project.inc @@ -155,7 +155,10 @@ class ProjectInvalidTextException extends ProjectException class ProjectPage { - public function __construct($pagename, $pagestate) + public $page_name; + public $page_state; + + public function __construct(string $pagename, string $pagestate) { $this->page_name = $pagename; $this->page_state = $pagestate; diff --git a/pinc/ProofProject.inc b/pinc/ProofProject.inc index e14db32ac3..baa340f77c 100644 --- a/pinc/ProofProject.inc +++ b/pinc/ProofProject.inc @@ -3,7 +3,7 @@ declare(strict_types=1); class ProofProjectPage extends LPage { - public function __construct(object $proof_project, object $project_page) + public function __construct(ProofProject $proof_project, ProjectPage $project_page) { parent::__construct($proof_project->project, $project_page->page_name, $project_page->page_state, 0); } @@ -114,7 +114,10 @@ class ProofProjectPage extends LPage class ProofProject { - public function __construct(object $project) + public $project; + public $round; + + public function __construct(Project $project) { global $pguser; From b2982d4be4eeb7e187033cc779c1e39f8bc4a467 Mon Sep 17 00:00:00 2001 From: Ray Papworth Date: Thu, 11 Apr 2024 14:48:41 +0000 Subject: [PATCH 03/11] changes to yaml file and warning strings --- api/dp-openapi.yaml | 8 ++++---- api/v1_projects.inc | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/api/dp-openapi.yaml b/api/dp-openapi.yaml index 33f55c42d7..011d6f173a 100644 --- a/api/dp-openapi.yaml +++ b/api/dp-openapi.yaml @@ -928,7 +928,7 @@ paths: put: tags: - project - description: Check out a page for proofreading. + description: Check out a page for proofreading and return page-level details (current text, image name, etc). parameters: - $ref: '#/components/parameters/projectid' - $ref: '#/components/parameters/state_query' @@ -983,17 +983,17 @@ paths: put: tags: - project - description: Various operations on the page as determined by the 'pageaction_query' parameter + description: Various operations on the page as determined by the 'pageaction' parameter parameters: - $ref: '#/components/parameters/projectid' - $ref: '#/components/parameters/state_query' - $ref: '#/components/parameters/pagename' - $ref: '#/components/parameters/pagestate_query' - - name: pageaction_query + - name: pageaction in: query description: Action to perform on the page.

- resume - If the page is saved-as-done 'unsaves' it. Response:- proof_page object. + resume - The page state can be 'out', 'temp' or 'saved'. If the state is 'saved' it is changed to 'temp'. Returns page-level details (current text, image name, etc). Response:- proof_page object.

checkin - Checks the page for invalid characters and if valid, saves it as done. Response:- checkin_response object.

diff --git a/api/v1_projects.inc b/api/v1_projects.inc index 41599ff558..93f672d1aa 100644 --- a/api/v1_projects.inc +++ b/api/v1_projects.inc @@ -811,7 +811,7 @@ function api_v1_project_page(string $method, array $data, array $query_params) } if ($page_state != $project_page->page_state) { $err = sprintf( - _('Warning: Page "%1$s" is no longer in state "%2$s"; it is now in state "%3$s".'), + _('Page "%1$s" is not in state "%2$s"; it is now in state "%3$s".'), $project_page->page_name, $page_state, $project_page->page_state @@ -862,7 +862,7 @@ function create_proof_project(array $data, array $query_params) } if ($state != $project->state) { $err = sprintf( - _('Warning: Project "%1$s" is not in state "%2$s"; it is now in state "%3$s".'), + _('Project "%1$s" is not in state "%2$s"; it is now in state "%3$s".'), $project->nameofwork, project_states_text($state), project_states_text($project->state) From eaba51b2bf966b0e72b9052288b7fdfb29a246c2 Mon Sep 17 00:00:00 2001 From: Ray Papworth Date: Thu, 11 Apr 2024 19:57:12 +0000 Subject: [PATCH 04/11] forgot to sve earlier --- project.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/project.php b/project.php index 9439e11213..394dee6254 100644 --- a/project.php +++ b/project.php @@ -172,7 +172,7 @@ function do_expected_state() if (!empty($expected_state) && $expected_state != $project->state) { echo "

"; echo sprintf( - _('Warning: Project "%1$s" is no longer in state "%2$s"; it is now in state "%3$s".'), + _('Project "%1$s" is not in state "%2$s"; it is now in state "%3$s".'), html_safe($project->nameofwork), project_states_text($expected_state), project_states_text($project->state) From f1699e84e681a340c69d786d347a0ceefc004701 Mon Sep 17 00:00:00 2001 From: Ray Papworth Date: Fri, 12 Apr 2024 09:41:57 +0000 Subject: [PATCH 05/11] Get page text function removed --- SETUP/tests/ApiTest.php | 4 ++-- api/dp-openapi.yaml | 33 --------------------------------- api/v1.inc | 1 - api/v1_projects.inc | 4 ---- pinc/ProofProject.inc | 21 +++++---------------- 5 files changed, 7 insertions(+), 56 deletions(-) diff --git a/SETUP/tests/ApiTest.php b/SETUP/tests/ApiTest.php index ad42414eb4..e8636f1081 100644 --- a/SETUP/tests/ApiTest.php +++ b/SETUP/tests/ApiTest.php @@ -517,8 +517,8 @@ public function test_save_as_in_progress() $this->assertEquals($this->TEST_TEXT, $response["text"]); // get last saved text - $response = $this->get_text($project->projectid, "P1.proj_avail", "001.png", "P1.page_temp"); - $this->assertEquals(["text" => $this->TEST_MODIFIED_TEXT], $response); + $response = $this->resume($project->projectid, "P1.proj_avail", "001.png", "P1.page_temp"); + $this->assertEquals($this->TEST_MODIFIED_TEXT, $response["text"]); // save invalid text $this->expectExceptionCode(125); diff --git a/api/dp-openapi.yaml b/api/dp-openapi.yaml index 011d6f173a..cca78e8a2f 100644 --- a/api/dp-openapi.yaml +++ b/api/dp-openapi.yaml @@ -1029,32 +1029,6 @@ paths: $ref: '#/components/responses/RateLimitExceeded' default: $ref: '#/components/responses/UnexpectedError' - get: - tags: - - project - description: Gets the current (last saved) page text - parameters: - - $ref: '#/components/parameters/projectid' - - $ref: '#/components/parameters/state_query' - - $ref: '#/components/parameters/pagename' - - $ref: '#/components/parameters/pagestate_query' - responses: - 200: - description: Page text - content: - application/json: - schema: - $ref: '#/components/schemas/proof_page_text' - '400': - $ref: '#/components/responses/InvalidValue' - '401': - $ref: '#/components/responses/Unauthorized' - '404': - $ref: '#/components/responses/NotFound' - '429': - $ref: '#/components/responses/RateLimitExceeded' - default: - $ref: '#/components/responses/UnexpectedError' /stats/site: get: @@ -1546,13 +1520,6 @@ components: items: type: string - proof_page_text: - type: object - properties: - text: - description: The page text from current round. - type: string - proof_page: type: object properties: diff --git a/api/v1.inc b/api/v1.inc index e379193c1d..454ec4973d 100644 --- a/api/v1.inc +++ b/api/v1.inc @@ -49,7 +49,6 @@ $router->add_route("GET", "v1/queues/:queueid/projects", "api_v1_queue_projects" $router->add_route("PUT", "v1/projects/:projectid/checkout", "api_v1_project_checkout"); $router->add_route("PUT", "v1/projects/:projectid/validatetext", "api_v1_project_validatetext"); $router->add_route("PUT", "v1/projects/:projectid/pages/:pagename", "api_v1_project_page"); -$router->add_route("GET", "v1/projects/:projectid/pages/:pagename", "api_v1_project_page"); $router->add_route("GET", "v1/stats/site", "api_v1_stats_site"); $router->add_route("GET", "v1/stats/site/projects/stages", "api_v1_stats_site_projects_stages"); diff --git a/api/v1_projects.inc b/api/v1_projects.inc index 93f672d1aa..4b33ced67a 100644 --- a/api/v1_projects.inc +++ b/api/v1_projects.inc @@ -820,10 +820,6 @@ function api_v1_project_page(string $method, array $data, array $query_params) } $proof_project_page = new ProofProjectPage($proof_project, $project_page); - if ($method == "GET") { - return $proof_project_page->render_page_text(); - } - // must be PUT $page_action = $query_params['pageaction'] ?? null; if (null === $page_action) { throw new BadRequest("No page action found in request."); diff --git a/pinc/ProofProject.inc b/pinc/ProofProject.inc index baa340f77c..8c28b2b9c3 100644 --- a/pinc/ProofProject.inc +++ b/pinc/ProofProject.inc @@ -48,7 +48,7 @@ class ProofProjectPage extends LPage global $pguser; $this->validate_text($page_text); $this->saveAsInProgress($page_text, $pguser); - return $this->get_page_text_array() + $this->get_page_state_array(); + return $this->get_page_text_data(); } public function save_and_revert(string $page_text): array @@ -57,7 +57,7 @@ class ProofProjectPage extends LPage $this->validate_text($page_text); $this->saveAsInProgress($page_text, $pguser); $this->revertToOriginal(); - return $this->get_page_text_array() + $this->get_page_state_array(); + return $this->get_page_text_data(); } public function pp_resume_page(): array @@ -68,17 +68,10 @@ class ProofProjectPage extends LPage return $this->render_page_data(); } - public function render_page_text(): array - { - // validate user and page? page should be in out or temp state only - return $this->get_page_text_array(); - } - private function render_page_data(): array { return - $this->get_page_text_array() + - $this->get_page_state_array() + + $this->get_page_text_data() + [ "pagenum" => $this->get_filename(), "round_info" => $this->get_info(), @@ -88,14 +81,10 @@ class ProofProjectPage extends LPage ]; } - public function get_page_text_array(): array - { - return ["text" => $this->get_text()]; - } - - private function get_page_state_array(): array + private function get_page_text_data(): array { return [ + "text" => $this->get_text(), "pagestate" => $this->page_state, "saved" => $this->can_be_reverted_to_last_save(), ]; From 2c04c07b56e5450c7ff0fdcc4d2844d7c9a42aff Mon Sep 17 00:00:00 2001 From: Ray Papworth Date: Sat, 13 Apr 2024 09:08:59 +0000 Subject: [PATCH 06/11] re-implement check for pages_table_exists lost in conflict res. --- api/v1_validators.inc | 2 +- pinc/Project.inc | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/api/v1_validators.inc b/api/v1_validators.inc index 5f37da766b..f1371e743b 100644 --- a/api/v1_validators.inc +++ b/api/v1_validators.inc @@ -40,7 +40,7 @@ function validate_page_name($pagename, $data) { try { return $data[":projectid"]->get_project_page($pagename); - } catch (NonexistentPageException $exception) { + } catch (NonexistentPageException | NoProjectPageTable $exception) { throw new NotFoundError($exception->getMessage(), $exception->getCode()); } } diff --git a/pinc/Project.inc b/pinc/Project.inc index 524a3b1d55..b5e4e819c3 100644 --- a/pinc/Project.inc +++ b/pinc/Project.inc @@ -1901,6 +1901,10 @@ class Project public function get_project_page($pagename) { + if (!$this->pages_table_exists) { + throw new NoProjectPageTable(_("Project page table does not exist.")); + } + $sql = sprintf( " SELECT state From 26836a7672864f174cd013f547d8321dc95e797b Mon Sep 17 00:00:00 2001 From: Ray Papworth Date: Sat, 13 Apr 2024 09:54:31 +0000 Subject: [PATCH 07/11] correction to conflict resolution --- pinc/DPage.inc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pinc/DPage.inc b/pinc/DPage.inc index cbe0aa62bb..6eec006279 100644 --- a/pinc/DPage.inc +++ b/pinc/DPage.inc @@ -544,7 +544,7 @@ function _Page_require( implode(', ', $allowed_states), $curr_page_state ); - throw new ProjectPageException($err); + throw new ProjectPageStateException($err); } if (is_null($allowed_user_colname)) { @@ -559,7 +559,7 @@ function _Page_require( _("This operation (%s) can only be done by the user who has the page checked out, which you are not."), $op_name ); - throw new ProjectPageException($err); + throw new PageNotOwnedException($err); } } } From f9cc9a5e19df1683b2443fcd9596c57af414600d Mon Sep 17 00:00:00 2001 From: Ray Papworth Date: Thu, 25 Apr 2024 14:18:46 +0000 Subject: [PATCH 08/11] add endpoint v1/projects/:projectid/pages/:pagename --- SETUP/tests/ApiTest.php | 12 ++++++++++++ api/dp-openapi.yaml | 30 ++++++++++++++++++++++++++++-- api/v1.inc | 1 + api/v1_projects.inc | 8 ++++++++ pinc/ProofProject.inc | 2 +- 5 files changed, 50 insertions(+), 3 deletions(-) diff --git a/SETUP/tests/ApiTest.php b/SETUP/tests/ApiTest.php index e8636f1081..abb8537e02 100644 --- a/SETUP/tests/ApiTest.php +++ b/SETUP/tests/ApiTest.php @@ -161,6 +161,18 @@ public function test_create_project_no_data() $router->route($path, $query_params); } + public function test_get_page_data() + { + $project = $this->_create_available_project(); + $_SERVER["REQUEST_METHOD"] = "GET"; + $path = "v1/projects/$project->projectid/pages/001.png"; + $query_params = []; + $router = ApiRouter::get_router(); + $result = $router->route($path, $query_params); + $this->assertEquals($this->TEST_TEXT, $result["text"]); + $this->assertEquals("P1.page_avail", $result["pagestate"]); + } + //--------------------------------------------------------------------------- // tests for proofreading diff --git a/api/dp-openapi.yaml b/api/dp-openapi.yaml index cca78e8a2f..d4ec37eb9d 100644 --- a/api/dp-openapi.yaml +++ b/api/dp-openapi.yaml @@ -1030,6 +1030,33 @@ paths: default: $ref: '#/components/responses/UnexpectedError' + get: + tags: + - project + description: Get page text and state + parameters: + - $ref: '#/components/parameters/projectid' + - $ref: '#/components/parameters/pagename' + responses: + 200: + description: Page data + content: + application/json: + schema: + $ref: '#/components/schemas/page_text_state' + '400': + $ref: '#/components/responses/InvalidValue' + '401': + $ref: '#/components/responses/Unauthorized' + '403': + $ref: '#/components/responses/Forbidden' + '404': + $ref: '#/components/responses/NotFound' + '429': + $ref: '#/components/responses/RateLimitExceeded' + default: + $ref: '#/components/responses/UnexpectedError' + /stats/site: get: tags: @@ -1571,9 +1598,8 @@ components: description: The page state, it is now in a 'temp' state since this is returned by save-as-in-progress or save-and-revert-to-original type: string saved: - description: Now true (since page has been saved) + description: true if the page has been saved. type: boolean - enum: [true] text_analysis: type: object diff --git a/api/v1.inc b/api/v1.inc index 454ec4973d..bff4118ccb 100644 --- a/api/v1.inc +++ b/api/v1.inc @@ -49,6 +49,7 @@ $router->add_route("GET", "v1/queues/:queueid/projects", "api_v1_queue_projects" $router->add_route("PUT", "v1/projects/:projectid/checkout", "api_v1_project_checkout"); $router->add_route("PUT", "v1/projects/:projectid/validatetext", "api_v1_project_validatetext"); $router->add_route("PUT", "v1/projects/:projectid/pages/:pagename", "api_v1_project_page"); +$router->add_route("GET", "v1/projects/:projectid/pages/:pagename", "api_v1_project_page_data"); $router->add_route("GET", "v1/stats/site", "api_v1_stats_site"); $router->add_route("GET", "v1/stats/site/projects/stages", "api_v1_stats_site_projects_stages"); diff --git a/api/v1_projects.inc b/api/v1_projects.inc index 4b33ced67a..97812008a6 100644 --- a/api/v1_projects.inc +++ b/api/v1_projects.inc @@ -775,6 +775,14 @@ function _get_enabled_filter($query_params) //--------------------------------------------------------------------------- // Proofreading functions +function api_v1_project_page_data(string $method, array $data, array $query_params) +{ + $project_page = $data[":pagename"]; + $proof_project = new ProofProject($data[":projectid"]); + $proof_project_page = new ProofProjectPage($proof_project, $project_page); + return $proof_project_page->get_page_text_data(); +} + // checkout a page function api_v1_project_checkout($method, array $data, array $query_params) { diff --git a/pinc/ProofProject.inc b/pinc/ProofProject.inc index 8c28b2b9c3..2f68fd01a3 100644 --- a/pinc/ProofProject.inc +++ b/pinc/ProofProject.inc @@ -81,7 +81,7 @@ class ProofProjectPage extends LPage ]; } - private function get_page_text_data(): array + public function get_page_text_data(): array { return [ "text" => $this->get_text(), From dafe6edc3d16a91148f04aa13ecd3a78556291ea Mon Sep 17 00:00:00 2001 From: Ray Papworth Date: Fri, 26 Apr 2024 08:03:53 +0000 Subject: [PATCH 09/11] re-arrange project state validation --- api/v1.inc | 2 +- api/v1_projects.inc | 63 +++++++++++++++++++++------------------------ 2 files changed, 31 insertions(+), 34 deletions(-) diff --git a/api/v1.inc b/api/v1.inc index bff4118ccb..e379193c1d 100644 --- a/api/v1.inc +++ b/api/v1.inc @@ -49,7 +49,7 @@ $router->add_route("GET", "v1/queues/:queueid/projects", "api_v1_queue_projects" $router->add_route("PUT", "v1/projects/:projectid/checkout", "api_v1_project_checkout"); $router->add_route("PUT", "v1/projects/:projectid/validatetext", "api_v1_project_validatetext"); $router->add_route("PUT", "v1/projects/:projectid/pages/:pagename", "api_v1_project_page"); -$router->add_route("GET", "v1/projects/:projectid/pages/:pagename", "api_v1_project_page_data"); +$router->add_route("GET", "v1/projects/:projectid/pages/:pagename", "api_v1_project_page"); $router->add_route("GET", "v1/stats/site", "api_v1_stats_site"); $router->add_route("GET", "v1/stats/site/projects/stages", "api_v1_stats_site_projects_stages"); diff --git a/api/v1_projects.inc b/api/v1_projects.inc index 97812008a6..a825f6ea6b 100644 --- a/api/v1_projects.inc +++ b/api/v1_projects.inc @@ -775,19 +775,14 @@ function _get_enabled_filter($query_params) //--------------------------------------------------------------------------- // Proofreading functions -function api_v1_project_page_data(string $method, array $data, array $query_params) -{ - $project_page = $data[":pagename"]; - $proof_project = new ProofProject($data[":projectid"]); - $proof_project_page = new ProofProjectPage($proof_project, $project_page); - return $proof_project_page->get_page_text_data(); -} - // checkout a page function api_v1_project_checkout($method, array $data, array $query_params) { try { - $proof_project = create_proof_project($data, $query_params); + $project = $data[":projectid"]; + $state = $query_params["state"] ?? null; + validate_project_state($project, $state); + $proof_project = new ProofProject($project); return $proof_project->checkout(); } catch (ProjectException $exception) { throw new NotFoundError($exception->getMessage(), $exception->getCode()); @@ -807,10 +802,19 @@ function api_v1_project_page(string $method, array $data, array $query_params) global $pguser; try { - $proof_project = create_proof_project($data, $query_params); + $project = $data[":projectid"]; $project_page = $data[":pagename"]; - $page_state = $query_params['pagestate'] ?? null; + if ($method == "GET") { + $proof_project = new ProofProject($project); + $proof_project_page = new ProofProjectPage($proof_project, $project_page); + return $proof_project_page->get_page_text_data(); + } + // $method is "PUT" + $state = $query_params["state"] ?? null; + validate_project_state($project, $state); + $proof_project = new ProofProject($project); + $page_state = $query_params['pagestate'] ?? null; if (null === $page_state) { throw new InvalidValue("No page state found in request."); } @@ -853,30 +857,23 @@ function api_v1_project_page(string $method, array $data, array $query_params) } } -function create_proof_project(array $data, array $query_params) +function validate_project_state($project, $state) { - $project = $data[":projectid"]; - $state = $query_params["state"] ?? null; - try { - if (null === $state) { - throw new InvalidValue("No project state found in request."); - } - if (!in_array($state, ProjectStates::get_states())) { - throw new InvalidValue("Invalid project state"); - } - if ($state != $project->state) { - $err = sprintf( - _('Project "%1$s" is not in state "%2$s"; it is now in state "%3$s".'), - $project->nameofwork, - project_states_text($state), - project_states_text($project->state) - ); - throw new ProjectNotInRequiredStateException($err); - } - } catch (ProjectException $exception) { - throw new NotFoundError($exception->getMessage(), $exception->getCode()); + if (null === $state) { + throw new InvalidValue("No project state found in request."); + } + if (!in_array($state, ProjectStates::get_states())) { + throw new InvalidValue("Invalid project state"); + } + if ($state != $project->state) { + $err = sprintf( + _('Project "%1$s" is not in state "%2$s"; it is now in state "%3$s".'), + $project->nameofwork, + project_states_text($state), + project_states_text($project->state) + ); + throw new ProjectNotInRequiredStateException($err); } - return new ProofProject($project); } function receive_project_text_from_request_body($field): string From 9b79f7cd1b0988e197a957ebdded9d12915b1d93 Mon Sep 17 00:00:00 2001 From: Ray Papworth Date: Sat, 27 Apr 2024 09:56:24 +0000 Subject: [PATCH 10/11] adjustments suggested by review --- api/dp-openapi.yaml | 2 +- api/v1_projects.inc | 7 +++---- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/api/dp-openapi.yaml b/api/dp-openapi.yaml index d4ec37eb9d..46af1529b2 100644 --- a/api/dp-openapi.yaml +++ b/api/dp-openapi.yaml @@ -1572,7 +1572,7 @@ components: pagenum: description: Page "number" without extension type: string - round_info_array: + round_info: description: Array of names of proofreaders in earlier rounds type: array items: diff --git a/api/v1_projects.inc b/api/v1_projects.inc index a825f6ea6b..99eb115b9b 100644 --- a/api/v1_projects.inc +++ b/api/v1_projects.inc @@ -823,7 +823,7 @@ function api_v1_project_page(string $method, array $data, array $query_params) } if ($page_state != $project_page->page_state) { $err = sprintf( - _('Page "%1$s" is not in state "%2$s"; it is now in state "%3$s".'), + _('Page "%1$s" is not in state "%2$s"; it is now in state "%3$s".'), $project_page->page_name, $page_state, $project_page->page_state @@ -833,10 +833,9 @@ function api_v1_project_page(string $method, array $data, array $query_params) $proof_project_page = new ProofProjectPage($proof_project, $project_page); $page_action = $query_params['pageaction'] ?? null; - if (null === $page_action) { - throw new BadRequest("No page action found in request."); - } switch ($page_action) { + case null: + throw new BadRequest("No page action found in request."); case 'resume': return $proof_project_page->pp_resume_page(); case 'checkin': From 7001fb68b68267d1a5dedeab70931c4352c79459 Mon Sep 17 00:00:00 2001 From: Ray Papworth Date: Sat, 27 Apr 2024 10:42:20 +0000 Subject: [PATCH 11/11] add test for resume page not owned --- SETUP/tests/ApiTest.php | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/SETUP/tests/ApiTest.php b/SETUP/tests/ApiTest.php index abb8537e02..e79727a1ca 100644 --- a/SETUP/tests/ApiTest.php +++ b/SETUP/tests/ApiTest.php @@ -373,6 +373,23 @@ public function test_resume_page() $this->assertEquals($expected, $response); } + public function test_resume_page_not_owned() + { + global $pguser; + + $this->expectExceptionCode(307); + + $pguser = $this->TEST_USERNAME; + $project = $this->_create_available_project(); + + // check out a page + $this->checkout($project->projectid, "P1.proj_avail"); + + // let another user try to resume it + $pguser = $this->TEST_USERNAME_PM; + $this->resume($project->projectid, 'P1.proj_avail', '001.png', 'P1.page_out'); + } + public function test_project_return_to_round() { global $pguser;