From e0059c443d189a46d6a2f2ed131cfd8d17d4647f Mon Sep 17 00:00:00 2001 From: Nicole Madruga Date: Mon, 7 Nov 2022 14:05:48 +1300 Subject: [PATCH 01/18] Stop creating FinalDecision assignments differently, they should also be assigned prior to work started --- .../src/generateReviewAssignments.ts | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/plugins/action_generate_review_assignment_records/src/generateReviewAssignments.ts b/plugins/action_generate_review_assignment_records/src/generateReviewAssignments.ts index a55f042fb..d9afab4b6 100644 --- a/plugins/action_generate_review_assignment_records/src/generateReviewAssignments.ts +++ b/plugins/action_generate_review_assignment_records/src/generateReviewAssignments.ts @@ -389,9 +389,7 @@ const generateNextReviewAssignments: RegerenateReviewAssignments = ( // Get assignmentState with: status, isLocked and isSelfAssigned (to create new or update) const assignment = getNewOrExistingAssignmentStatus( existingReviewsAssigned, - reviewer.canMakeFinalDecision, reviewer.canSelfAssign || nextReviewLevel > 1, - sectionCodes, existingAssignment ) @@ -456,26 +454,13 @@ const constructReviewAssignmentObject = ( // Checks if existing assignment, should keep status and update if isLocked const getNewOrExistingAssignmentStatus = ( existingReviewsAssigned: ExistingReviewAssignment[], - canMakeFinalDecision: boolean, isSelfAssignable: boolean, - sectionCodes: string[], existingAssignment?: ExistingReviewAssignment ): AssignmentState => { const isReviewAssigned = existingReviewsAssigned.length > 0 const isAssigned = existingReviewsAssigned.some( ({ userId }) => userId === existingAssignment?.userId ) - // temporarily final decision shouldn't be locked if there are other reviewAssignment assigned - // Note: This logic will be updated during implementation of ISSUE #836 (front-end) to allow - // locking other reviewAssignments for finalDecision once one has been submitted. - if (canMakeFinalDecision) - return { - status: ReviewAssignmentStatus.Assigned, - isSelfAssignable: true, - isLocked: false, - assignedSections: sectionCodes, - } - // Create new OR update ReviewAssignment: // 1. If existing // - keep same status, isSelfAssignable From 520d7fc39492c8bc65a473175ed85a66b512e235 Mon Sep 17 00:00:00 2001 From: Nicole Madruga Date: Mon, 7 Nov 2022 16:40:48 +1300 Subject: [PATCH 02/18] Fix AssignmentStatus update for selfAssignment reviews setting others as unlocked on unassignment - so the newly asignee is not locked anymore --- .../src/updateReviewAssignmentsStatus.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/action_update_review_assignments_status/src/updateReviewAssignmentsStatus.ts b/plugins/action_update_review_assignments_status/src/updateReviewAssignmentsStatus.ts index acb2806df..99a8ecbb6 100644 --- a/plugins/action_update_review_assignments_status/src/updateReviewAssignmentsStatus.ts +++ b/plugins/action_update_review_assignments_status/src/updateReviewAssignmentsStatus.ts @@ -35,7 +35,7 @@ async function updateReviewAssignmentsStatus({ return otherSelfAssignments.map(({ id, assignedSections }) => ({ id, - isLocked: isLocked || assignedSections.length > 0, + isLocked: isLocked ? assignedSections.length === 0 : false, })) } From 203d601fc7039ebffc51b5f980522cd538c32da9 Mon Sep 17 00:00:00 2001 From: Nicole Madruga Date: Tue, 8 Nov 2022 11:06:21 +1300 Subject: [PATCH 03/18] Delete action not used - after decision to not lock other self-assignment ReviewAssignments. The isLocked only is set on updateReviewStatus now --- .../package.json | 13 ---- .../plugin.json | 24 ------ .../src/databaseMethods.ts | 75 ------------------- .../src/index.ts | 4 - .../src/updateReviewAssignmentsStatus.ts | 74 ------------------ .../yarn.lock | 4 - 6 files changed, 194 deletions(-) delete mode 100644 plugins/action_update_review_assignments_status/package.json delete mode 100644 plugins/action_update_review_assignments_status/plugin.json delete mode 100644 plugins/action_update_review_assignments_status/src/databaseMethods.ts delete mode 100644 plugins/action_update_review_assignments_status/src/index.ts delete mode 100644 plugins/action_update_review_assignments_status/src/updateReviewAssignmentsStatus.ts delete mode 100644 plugins/action_update_review_assignments_status/yarn.lock diff --git a/plugins/action_update_review_assignments_status/package.json b/plugins/action_update_review_assignments_status/package.json deleted file mode 100644 index 0c2c6eef3..000000000 --- a/plugins/action_update_review_assignments_status/package.json +++ /dev/null @@ -1,13 +0,0 @@ -{ - "name": "action_update_review_assignments", - "version": "0.1", - "description": "Sets other review_assignments' status to not available when one is assigned", - "author": "The mSupplyFoundation", - "license": "MIT", - "scripts": { - "build": "node ../../utils/pluginScripts/buildPluginFromPluginFolder.js", - "test": "node ../../utils/pluginScripts/testPluginFromPluginFolder.js" - }, - "devDependencies": {}, - "dependencies": {} -} diff --git a/plugins/action_update_review_assignments_status/plugin.json b/plugins/action_update_review_assignments_status/plugin.json deleted file mode 100644 index 7c0732b09..000000000 --- a/plugins/action_update_review_assignments_status/plugin.json +++ /dev/null @@ -1,24 +0,0 @@ -{ - "type": "action_plugin", - "code": "updateReviewAssignmentsStatus", - "name": "Update Review Assignment statuses", - "description": "Update status of Review Assignment records when one is Assigned", - "required_parameters": ["reviewAssignmentId"], - "optional_parameters": ["trigger"], - "output_properties": ["reviewAssignmentUpdates"], - "info": { - "author": { - "name": "The mSupply Foundation", - "url": "http://msupply.foundation" - }, - "keywords": ["action", "insert"], - "logos": { - "small": "", - "large": "" - }, - "links": [], - "screenshots": [], - "version": "1.0.0", - "updated": "2021-02-02" - } -} diff --git a/plugins/action_update_review_assignments_status/src/databaseMethods.ts b/plugins/action_update_review_assignments_status/src/databaseMethods.ts deleted file mode 100644 index 13210e931..000000000 --- a/plugins/action_update_review_assignments_status/src/databaseMethods.ts +++ /dev/null @@ -1,75 +0,0 @@ -import { ReviewAssignment } from '../../../src/generated/graphql' - -const databaseMethods = (DBConnect: any) => ({ - getReviewAssignmentById: async (reviewAssignmentId: number) => { - const text = ` - SELECT * FROM review_assignment - WHERE id = $1 - ` - try { - const result = await DBConnect.query({ - text, - values: [reviewAssignmentId], - }) - return result.rows[0] - } catch (err) { - console.log(err.message) - throw err - } - }, - getMatchingReviewAssignments: async ( - reviewAssignmentId: number, - applicationId: number, - stageNumber: number, - reviewLevel: number, - isSelfAssignable: boolean - ): Promise => { - const text = ` - SELECT - id, - assigned_sections as "assignedSections" - FROM review_assignment - WHERE application_id = $2 - AND stage_number = $3 - AND level_number = $4 - AND is_self_assignable = $5 - AND id <> $1 - ` - try { - const result = await DBConnect.query({ - text, - values: [reviewAssignmentId, applicationId, stageNumber, reviewLevel, isSelfAssignable], - }) - return result.rows - } catch (err) { - console.log(err.message) - throw err - } - }, - updateReviewAssignments: async (reviewAssignments: { id: number; isLocked: boolean }[]) => { - const reviewAssignmentUpdateResults = [] - for (const reviewAssignment of reviewAssignments) { - const { id, isLocked } = reviewAssignment - const text = ` - UPDATE review_assignment - SET is_locked = $2 - WHERE id = $1 - RETURNING id, is_locked - ` - try { - const result = await DBConnect.query({ - text, - values: [id, isLocked], - }) - reviewAssignmentUpdateResults.push(result.rows[0]) - } catch (err) { - console.log(err.message) - reviewAssignmentUpdateResults.push(err.message) - throw err - } - } - return reviewAssignmentUpdateResults - }, -}) - -export default databaseMethods diff --git a/plugins/action_update_review_assignments_status/src/index.ts b/plugins/action_update_review_assignments_status/src/index.ts deleted file mode 100644 index 4d11f82cd..000000000 --- a/plugins/action_update_review_assignments_status/src/index.ts +++ /dev/null @@ -1,4 +0,0 @@ -import { ActionPluginType } from '../../types' -import updateReviewAssignmentsStatus from './updateReviewAssignmentsStatus' -const action: ActionPluginType = updateReviewAssignmentsStatus -export { action } diff --git a/plugins/action_update_review_assignments_status/src/updateReviewAssignmentsStatus.ts b/plugins/action_update_review_assignments_status/src/updateReviewAssignmentsStatus.ts deleted file mode 100644 index 99a8ecbb6..000000000 --- a/plugins/action_update_review_assignments_status/src/updateReviewAssignmentsStatus.ts +++ /dev/null @@ -1,74 +0,0 @@ -import { ActionQueueStatus, ReviewAssignment, Trigger } from '../../../src/generated/graphql' -import { ActionPluginInput } from '../../types' -import databaseMethods from './databaseMethods' - -async function updateReviewAssignmentsStatus({ - parameters, - applicationData, - DBConnect, -}: ActionPluginInput) { - const db = databaseMethods(DBConnect) - console.log('Updating review assignment statuses...') - - try { - const trigger = parameters?.trigger ?? applicationData?.action_payload?.trigger_payload?.trigger - const { reviewAssignmentId } = parameters - - let assignments: { id: number; isLocked: boolean }[] = [] - // NB: reviewAssignmentId comes from record_id on TriggerPayload when - // triggered from review_assignment table - const { - application_id: applicationId, - stage_number: stageNumber, - level_number: reviewLevel, - } = await db.getReviewAssignmentById(reviewAssignmentId) - - const setOtherSelfAssignmentsLocked = async (isLocked: boolean) => { - const isSelfAssignable = true - const otherSelfAssignments = await db.getMatchingReviewAssignments( - reviewAssignmentId, - applicationId, - stageNumber, - reviewLevel, - isSelfAssignable - ) - - return otherSelfAssignments.map(({ id, assignedSections }) => ({ - id, - isLocked: isLocked ? assignedSections.length === 0 : false, - })) - } - - switch (trigger) { - case Trigger.OnReviewAssign: - assignments = await setOtherSelfAssignmentsLocked(true) - break - case Trigger.OnReviewUnassign: { - assignments = await setOtherSelfAssignmentsLocked(false) - break - } - } - - const reviewAssignmentUpdateResults = await db.updateReviewAssignments(assignments) - - if (reviewAssignmentUpdateResults.length > 0) { - console.log('Review Assignment status updates:', reviewAssignmentUpdateResults) - } - - return { - status: ActionQueueStatus.Success, - error_log: '', - output: { - reviewAssignmentUpdates: reviewAssignmentUpdateResults, - }, - } - } catch (error) { - console.log(error.message) - return { - status: ActionQueueStatus.Fail, - error_log: 'Problem updating review_assignment statuses.', - } - } -} - -export default updateReviewAssignmentsStatus diff --git a/plugins/action_update_review_assignments_status/yarn.lock b/plugins/action_update_review_assignments_status/yarn.lock deleted file mode 100644 index fb57ccd13..000000000 --- a/plugins/action_update_review_assignments_status/yarn.lock +++ /dev/null @@ -1,4 +0,0 @@ -# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY. -# yarn lockfile v1 - - From bbb0491f8218c2874d60330bf2e46bc27929fffe Mon Sep 17 00:00:00 2001 From: Nicole Madruga Date: Tue, 8 Nov 2022 11:24:47 +1300 Subject: [PATCH 04/18] Update docs to remove action and generate new Assignment isLocked = true --- documentation/List-of-Action-plugins.md | 18 +----------------- .../src/generateReviewAssignments.ts | 14 +++++--------- 2 files changed, 6 insertions(+), 26 deletions(-) diff --git a/documentation/List-of-Action-plugins.md b/documentation/List-of-Action-plugins.md index 58c21e9dc..a338fbb07 100644 --- a/documentation/List-of-Action-plugins.md +++ b/documentation/List-of-Action-plugins.md @@ -15,7 +15,6 @@ - [Grant Permissions](#grant-permissions) - [Revoke Permissions](#revoke-permissions) - [Generate Review Assignments](#generate-review-assignments) - - [Update Review Assignments](#update-review-assignments) - [Refresh Review Assignments](#refresh-review-assignments) - [Trim Responses](#trim-responses) - [Update Review Visibility](#update-review-visibility) @@ -461,7 +460,7 @@ See [Grant Permissions](#grant-permissions) above regarding acting on user-only ### Generate Review Assignments -Generates records in the `review_assignment` table -- i.e. which users (reviewers) are allowed to do a review for the current stage/level (and for which Sections). The records are set with `status` "Available" or "Assigned" and flags for `isSelfAssignable` and `isLocked` to help define when is allowed self-assignment. +Generates records in the `review_assignment` table -- i.e. which users (reviewers) are allowed to do a review for the current stage/level (and for which Sections). The records are set with `status` "Available" or "Assigned". With the properties to specify the type of assignament: `isSelfAssignable` if it should show for self-assignment when not assigned by another user. The `isLocked` property helps defining that the review can start but not be submitted - after the application has been back to Applicant for ammendments. It also creates records in the `review_assignment_assigner_join` table -- basically a list of users who have permission to make the _assignments_ in the review_assignment table. @@ -487,21 +486,6 @@ Should be run whenever an application or review is submitted or re-submitted, an --- -### Update Review Assignments - -When a reviewer self-assigns themselves to a review_assignment (i.e. its status changes from "Available" to "Assigned") the other review assignment records pertaining to that review/stage/level are marked as "is_locked = true" so that no other reviewer can start a review for the same thing. - -- _Action Code:_ **`updateReviewAssignmentsStatus`** - -| Input parameters
(\*required)
| Output properties | -| ---------------------------------------------------- | ----------------------------------------------------- | -| `reviewAssignmentId`\* | `reviewAssignmentUpdates` (`array` of `{id, status}`) | -| `trigger` (only executes on `ON_REVIEW_SELF_ASSIGN`) | | - -**Note:** If `trigger` is not supplied, the plugin will try to infer it from `applicationData` - ---- - ### Refresh Review Assignments A "super-action", which regenerates all `review_assignment` and `review_assignment_assigner_join` records associated with a specific user, or group of users (or all users). It does this by figuring out which active applications are associated with the input user(s), and then running [`generateReviewAssignments`](#generate-review-assignments) on each of them. diff --git a/plugins/action_generate_review_assignment_records/src/generateReviewAssignments.ts b/plugins/action_generate_review_assignment_records/src/generateReviewAssignments.ts index d9afab4b6..4e2905cdc 100644 --- a/plugins/action_generate_review_assignment_records/src/generateReviewAssignments.ts +++ b/plugins/action_generate_review_assignment_records/src/generateReviewAssignments.ts @@ -457,25 +457,21 @@ const getNewOrExistingAssignmentStatus = ( isSelfAssignable: boolean, existingAssignment?: ExistingReviewAssignment ): AssignmentState => { - const isReviewAssigned = existingReviewsAssigned.length > 0 const isAssigned = existingReviewsAssigned.some( ({ userId }) => userId === existingAssignment?.userId ) - // Create new OR update ReviewAssignment: + // Create NEW or update EXISTING ReviewAssignment: // 1. If existing // - keep same status, isSelfAssignable - // - just update isLocked = true (if already assigned to another) + // - keep same isLocked if assigned // 2. If new reviewAssignment: // - status = Available (always) - // - if review canSelfAssign set isSelfAssignable = true (Default: false) - // - if isReviewAssigned then isLocked = true (only when is self-assignable) + // - isLocked = true if Assigned & locked (Default: false - Maybe needs checking if true - when LOQ sent) + // - isSelfAssignable = true if canSelfAssign (Default: false) return { status: existingAssignment?.status ?? ReviewAssignmentStatus.Available, isSelfAssignable: existingAssignment?.isSelfAssignable ?? isSelfAssignable, - isLocked: - existingAssignment && isAssigned - ? existingAssignment.isLocked - : isReviewAssigned && isSelfAssignable, + isLocked: existingAssignment && isAssigned ? existingAssignment.isLocked : false, } } From a1db36b8180c9d484623887df9447e2324b8db54 Mon Sep 17 00:00:00 2001 From: Nicole Madruga Date: Tue, 8 Nov 2022 16:46:03 +1300 Subject: [PATCH 05/18] Remove type ASSIGN_LOCKED, remove duplicated declartion of applications_list_view and add new function to auto-generate column resulting the available_sections for one assignment --- database/buildSchema/36_action_list_types.sql | 1 - database/buildSchema/42_database_info.sql | 54 ---------------- .../43_views_functions_triggers.sql | 63 +++++++++++++++++-- database/migration/migrateData.ts | 20 ++++++ 4 files changed, 78 insertions(+), 60 deletions(-) delete mode 100644 database/buildSchema/42_database_info.sql diff --git a/database/buildSchema/36_action_list_types.sql b/database/buildSchema/36_action_list_types.sql index 6247f06af..70ecfdc51 100644 --- a/database/buildSchema/36_action_list_types.sql +++ b/database/buildSchema/36_action_list_types.sql @@ -2,7 +2,6 @@ DROP TYPE IF EXISTS public.assigner_action; CREATE TYPE public.assigner_action AS ENUM ( 'ASSIGN', - 'ASSIGN_LOCKED', 'RE_ASSIGN' ); diff --git a/database/buildSchema/42_database_info.sql b/database/buildSchema/42_database_info.sql deleted file mode 100644 index d6bc09967..000000000 --- a/database/buildSchema/42_database_info.sql +++ /dev/null @@ -1,54 +0,0 @@ --- For storing system info -CREATE TABLE public.system_info ( - id serial PRIMARY KEY, - name varchar NOT NULL, - value jsonb DEFAULT '{}', - timestamp timestamptz DEFAULT CURRENT_TIMESTAMP -); - --- For primary and foreign key -CREATE OR REPLACE VIEW constraints_info AS -SELECT - constraints_info.constraint_type, - constraints_info.table_name AS from_table_name, - from_column_info.column_name AS from_column_name, - to_column_info.table_name AS to_table_name, - to_column_info.column_name AS to_column_name -FROM - information_schema.table_constraints AS constraints_info - JOIN information_schema.key_column_usage AS from_column_info ON constraints_info.constraint_name = from_column_info.constraint_name - JOIN information_schema.constraint_column_usage AS to_column_info ON constraints_info.constraint_name = to_column_info.constraint_name; - --- For full schema info -CREATE OR REPLACE VIEW schema_columns AS -SELECT - tables_info.table_name, - tables_info.table_type AS table_type, - columns_info.column_name, - columns_info.is_nullable, - columns_info.is_generated, - columns_info.data_type, - element_types.data_type AS sub_data_type, - constraints_info.constraint_type, - constraints_info.to_table_name AS fk_to_table_name, - constraints_info.to_column_name AS fk_to_column_name -FROM - information_schema.tables AS tables_info - JOIN information_schema.columns AS columns_info ON tables_info.table_name = columns_info.table_name - LEFT JOIN information_schema.element_types AS element_types ON columns_info.dtd_identifier = element_types.collection_type_identifier - AND columns_info.table_name = element_types.object_name - LEFT JOIN constraints_info ON columns_info.table_name = constraints_info.from_table_name - AND columns_info.column_name = constraints_info.from_column_name -WHERE - tables_info.table_schema != 'pg_catalog' - AND tables_info.table_schema != 'information_schema' -ORDER BY - columns_info.table_name, - columns_info.column_name; - -CREATE VIEW postgres_row_level AS -SELECT - * -FROM - pg_policies; - diff --git a/database/buildSchema/43_views_functions_triggers.sql b/database/buildSchema/43_views_functions_triggers.sql index d383a1b25..4df68cfaa 100644 --- a/database/buildSchema/43_views_functions_triggers.sql +++ b/database/buildSchema/43_views_functions_triggers.sql @@ -700,6 +700,41 @@ CREATE TRIGGER review_assignment_validate_section_trigger WHEN (NEW.trigger IS NULL AND OLD.trigger IS NULL) EXECUTE FUNCTION public.enforce_asssigned_section_validity (); +-- FUNCTION to set `available_sections` on given review_assignments based on other assignments +CREATE OR REPLACE FUNCTION public.review_assignment_available_section (assignment public.review_assignment) + RETURNS text[] + AS $$ + SELECT + ARRAY ( WITH my_array AS ( + SELECT DISTINCT + (ts.code) available_sections + FROM + template_section ts + JOIN TEMPLATE t ON t.id = ts.template_id + JOIN application a ON a.template_id = t.id + WHERE + a.id = $1.application_id +) + SELECT + available_sections + FROM + my_array + WHERE + available_sections NOT IN ( + SELECT + unnest(assigned_sections) + FROM + review_assignment + WHERE + status = 'ASSIGNED' + AND stage_id = $1.stage_id + AND level_number = $1.level_number + AND application_id = $1.application_id + AND id <> $1.id)) +$$ +LANGUAGE sql +STABLE; + -- TRIGGER (Listener) on review_assignment table: To update trigger DROP TRIGGER IF EXISTS review_assignment_trigger ON public.review_assignment; @@ -1288,10 +1323,6 @@ CREATE OR REPLACE FUNCTION assigner_list (stage_id int, assigner_id int) AND assigned_questions_count (application_id, $1, level_number) >= reviewable_questions_count (application_id) AND submitted_assigned_questions_count (application_id, $1, level_number) < assigned_questions_count (application_id, $1, level_number) THEN 'RE_ASSIGN' - WHEN COUNT(DISTINCT (review_assignment.id)) != 0 - AND assigned_questions_count (application_id, $1, level_number) >= reviewable_questions_count (application_id) - AND submitted_assigned_questions_count (application_id, $1, level_number) >= assigned_questions_count (application_id, $1, level_number) THEN - 'ASSIGN_LOCKED' WHEN COUNT(DISTINCT (review_assignment.id)) != 0 AND assigned_questions_count (application_id, $1, level_number) < reviewable_questions_count (application_id) THEN 'ASSIGN' @@ -1383,6 +1414,29 @@ LANGUAGE sql STABLE; -- APPLICATION_LIST_VIEW +-- Aggregated VIEW method of all data required for application list page +-- Requires an empty table as setof return and smart comment to make orderBy work (https://github.com/graphile/graphile-engine/pull/378) +CREATE TABLE application_list_shape ( + id int, + "serial" varchar, + "name" varchar, + template_code varchar, + template_name varchar, + applicant varchar, + org_name varchar, + stage varchar, + stage_colour varchar, + "status" public.application_status, + outcome public.application_outcome, + last_active_date timestamptz, + applicant_deadline timestamptz, + -- TO-DO: reviewer_deadline + assigners varchar[], + reviewers varchar[], + reviewer_action public.reviewer_action, + assigner_action public.assigner_action +); + CREATE OR REPLACE FUNCTION application_list (userid int DEFAULT 0) RETURNS SETOF application_list_shape AS $$ @@ -1423,7 +1477,6 @@ LANGUAGE sql STABLE; -- (https://github.com/graphile/graphile-engine/pull/378) --- Required to make 'orderBy' work in application_list COMMENT ON FUNCTION application_list (userid int) IS E'@sortable'; -- APPLICATION_LIST_FILTERS diff --git a/database/migration/migrateData.ts b/database/migration/migrateData.ts index e7bcbdfae..eac56ebf3 100644 --- a/database/migration/migrateData.ts +++ b/database/migration/migrateData.ts @@ -6,6 +6,7 @@ import { execSync } from 'child_process' import path from 'path' import { readFileSync } from 'fs' import { getAppEntryPointDir } from '../../src/components/utilityFunctions' +import dbConnectInstance from '../../src/components/databaseConnect' // CONSTANTS const FUNCTIONS_FILENAME = '43_views_functions_triggers.sql' @@ -583,6 +584,25 @@ const migrateData = async () => { ALTER TABLE data_table ADD COLUMN data_view_code varchar; `) + + console.log(' - Update existing review_assignment to set is_locked=false when isSelfAssigned') + // TODO - Not sure how to do this + + // This logs is related to something which will be added by view_functions_triggers file + console.log(' - Add new generated column available_sections in review_assignment TABLE') + + console.log(' - Remove type ASSIGN_LOCKED from assign_action ENUM') + await DB.changeSchema(` + ALTER TYPE public.assigner_action RENAME to public.assigner_action_old; + + CREATE TYPE public.assigner_action AS ENUM + ( + 'ASSIGN', + 'RE_ASSIGN' + ); + + DROP TYPE public.assigner_action_old; + `) } // Other version migrations continue here... From 9d106bcd6ac4cb5aac00267abf87fabf570fff69 Mon Sep 17 00:00:00 2001 From: Nicole Madruga Date: Tue, 8 Nov 2022 16:48:17 +1300 Subject: [PATCH 06/18] use plural name for variable --- database/buildSchema/43_views_functions_triggers.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/database/buildSchema/43_views_functions_triggers.sql b/database/buildSchema/43_views_functions_triggers.sql index 4df68cfaa..3101c16ed 100644 --- a/database/buildSchema/43_views_functions_triggers.sql +++ b/database/buildSchema/43_views_functions_triggers.sql @@ -701,7 +701,7 @@ CREATE TRIGGER review_assignment_validate_section_trigger EXECUTE FUNCTION public.enforce_asssigned_section_validity (); -- FUNCTION to set `available_sections` on given review_assignments based on other assignments -CREATE OR REPLACE FUNCTION public.review_assignment_available_section (assignment public.review_assignment) +CREATE OR REPLACE FUNCTION public.review_assignment_available_sections (assignment public.review_assignment) RETURNS text[] AS $$ SELECT From f570ea699e7d0164316906898c4b077cc8f8960d Mon Sep 17 00:00:00 2001 From: Nicole Madruga Date: Tue, 8 Nov 2022 16:56:06 +1300 Subject: [PATCH 07/18] Revert wrong removal of file --- database/buildSchema/42_database_info.sql | 54 +++++++++++++++++++++++ 1 file changed, 54 insertions(+) create mode 100644 database/buildSchema/42_database_info.sql diff --git a/database/buildSchema/42_database_info.sql b/database/buildSchema/42_database_info.sql new file mode 100644 index 000000000..d6bc09967 --- /dev/null +++ b/database/buildSchema/42_database_info.sql @@ -0,0 +1,54 @@ +-- For storing system info +CREATE TABLE public.system_info ( + id serial PRIMARY KEY, + name varchar NOT NULL, + value jsonb DEFAULT '{}', + timestamp timestamptz DEFAULT CURRENT_TIMESTAMP +); + +-- For primary and foreign key +CREATE OR REPLACE VIEW constraints_info AS +SELECT + constraints_info.constraint_type, + constraints_info.table_name AS from_table_name, + from_column_info.column_name AS from_column_name, + to_column_info.table_name AS to_table_name, + to_column_info.column_name AS to_column_name +FROM + information_schema.table_constraints AS constraints_info + JOIN information_schema.key_column_usage AS from_column_info ON constraints_info.constraint_name = from_column_info.constraint_name + JOIN information_schema.constraint_column_usage AS to_column_info ON constraints_info.constraint_name = to_column_info.constraint_name; + +-- For full schema info +CREATE OR REPLACE VIEW schema_columns AS +SELECT + tables_info.table_name, + tables_info.table_type AS table_type, + columns_info.column_name, + columns_info.is_nullable, + columns_info.is_generated, + columns_info.data_type, + element_types.data_type AS sub_data_type, + constraints_info.constraint_type, + constraints_info.to_table_name AS fk_to_table_name, + constraints_info.to_column_name AS fk_to_column_name +FROM + information_schema.tables AS tables_info + JOIN information_schema.columns AS columns_info ON tables_info.table_name = columns_info.table_name + LEFT JOIN information_schema.element_types AS element_types ON columns_info.dtd_identifier = element_types.collection_type_identifier + AND columns_info.table_name = element_types.object_name + LEFT JOIN constraints_info ON columns_info.table_name = constraints_info.from_table_name + AND columns_info.column_name = constraints_info.from_column_name +WHERE + tables_info.table_schema != 'pg_catalog' + AND tables_info.table_schema != 'information_schema' +ORDER BY + columns_info.table_name, + columns_info.column_name; + +CREATE VIEW postgres_row_level AS +SELECT + * +FROM + pg_policies; + From a5aff908872cc5822c14527315260adcbb9ac239 Mon Sep 17 00:00:00 2001 From: Nicole Madruga Date: Tue, 8 Nov 2022 16:57:50 +1300 Subject: [PATCH 08/18] Delete table from separated file to keep in 43_view_functions_triggers only --- .../buildSchema/37_application_list_view.sql | 23 ------------------- 1 file changed, 23 deletions(-) delete mode 100644 database/buildSchema/37_application_list_view.sql diff --git a/database/buildSchema/37_application_list_view.sql b/database/buildSchema/37_application_list_view.sql deleted file mode 100644 index 7ddb808b6..000000000 --- a/database/buildSchema/37_application_list_view.sql +++ /dev/null @@ -1,23 +0,0 @@ --- Aggregated VIEW method of all data required for application list page --- Requires an empty table as setof return and smart comment to make orderBy work (https://github.com/graphile/graphile-engine/pull/378) -CREATE TABLE IF NOT EXISTS application_list_shape ( - id int, - "serial" varchar, - "name" varchar, - template_code varchar, - template_name varchar, - applicant varchar, - org_name varchar, - stage varchar, - stage_colour varchar, - "status" public.application_status, - outcome public.application_outcome, - last_active_date timestamptz, - applicant_deadline timestamptz, - -- TO-DO: reviewer_deadline - assigners varchar[], - reviewers varchar[], - reviewer_action public.reviewer_action, - assigner_action public.assigner_action -); - From 22623a103e61a4b262c1419f614ff66ee65d9670 Mon Sep 17 00:00:00 2001 From: Nicole Madruga Date: Tue, 8 Nov 2022 16:58:01 +1300 Subject: [PATCH 09/18] Delete table from separated file to keep in 43_view_functions_triggers only --- database/buildSchema/43_views_functions_triggers.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/database/buildSchema/43_views_functions_triggers.sql b/database/buildSchema/43_views_functions_triggers.sql index 3101c16ed..5768d6f88 100644 --- a/database/buildSchema/43_views_functions_triggers.sql +++ b/database/buildSchema/43_views_functions_triggers.sql @@ -1416,7 +1416,7 @@ STABLE; -- APPLICATION_LIST_VIEW -- Aggregated VIEW method of all data required for application list page -- Requires an empty table as setof return and smart comment to make orderBy work (https://github.com/graphile/graphile-engine/pull/378) -CREATE TABLE application_list_shape ( +CREATE OR REPLACE TABLE application_list_shape ( id int, "serial" varchar, "name" varchar, From 3fa2d717b72b4ac0779257eae2cdba3a21686c09 Mon Sep 17 00:00:00 2001 From: Nicole Madruga Date: Tue, 8 Nov 2022 17:08:22 +1300 Subject: [PATCH 10/18] Revert change to remove table for view. Just update on migration code --- .../buildSchema/37_application_list_view.sql | 23 +++++++++++++++++++ .../43_views_functions_triggers.sql | 21 ----------------- database/migration/migrateData.ts | 8 ++++++- 3 files changed, 30 insertions(+), 22 deletions(-) create mode 100644 database/buildSchema/37_application_list_view.sql diff --git a/database/buildSchema/37_application_list_view.sql b/database/buildSchema/37_application_list_view.sql new file mode 100644 index 000000000..7ddb808b6 --- /dev/null +++ b/database/buildSchema/37_application_list_view.sql @@ -0,0 +1,23 @@ +-- Aggregated VIEW method of all data required for application list page +-- Requires an empty table as setof return and smart comment to make orderBy work (https://github.com/graphile/graphile-engine/pull/378) +CREATE TABLE IF NOT EXISTS application_list_shape ( + id int, + "serial" varchar, + "name" varchar, + template_code varchar, + template_name varchar, + applicant varchar, + org_name varchar, + stage varchar, + stage_colour varchar, + "status" public.application_status, + outcome public.application_outcome, + last_active_date timestamptz, + applicant_deadline timestamptz, + -- TO-DO: reviewer_deadline + assigners varchar[], + reviewers varchar[], + reviewer_action public.reviewer_action, + assigner_action public.assigner_action +); + diff --git a/database/buildSchema/43_views_functions_triggers.sql b/database/buildSchema/43_views_functions_triggers.sql index 5768d6f88..709e71d2e 100644 --- a/database/buildSchema/43_views_functions_triggers.sql +++ b/database/buildSchema/43_views_functions_triggers.sql @@ -1416,27 +1416,6 @@ STABLE; -- APPLICATION_LIST_VIEW -- Aggregated VIEW method of all data required for application list page -- Requires an empty table as setof return and smart comment to make orderBy work (https://github.com/graphile/graphile-engine/pull/378) -CREATE OR REPLACE TABLE application_list_shape ( - id int, - "serial" varchar, - "name" varchar, - template_code varchar, - template_name varchar, - applicant varchar, - org_name varchar, - stage varchar, - stage_colour varchar, - "status" public.application_status, - outcome public.application_outcome, - last_active_date timestamptz, - applicant_deadline timestamptz, - -- TO-DO: reviewer_deadline - assigners varchar[], - reviewers varchar[], - reviewer_action public.reviewer_action, - assigner_action public.assigner_action -); - CREATE OR REPLACE FUNCTION application_list (userid int DEFAULT 0) RETURNS SETOF application_list_shape AS $$ diff --git a/database/migration/migrateData.ts b/database/migration/migrateData.ts index eac56ebf3..8806dd100 100644 --- a/database/migration/migrateData.ts +++ b/database/migration/migrateData.ts @@ -593,7 +593,10 @@ const migrateData = async () => { console.log(' - Remove type ASSIGN_LOCKED from assign_action ENUM') await DB.changeSchema(` - ALTER TYPE public.assigner_action RENAME to public.assigner_action_old; + ALTER TYPE public.assigner_action RENAME to assigner_action_old; + + ALTER TABLE application_list_shape + DROP COLUMN IF EXISTS assigner_action; CREATE TYPE public.assigner_action AS ENUM ( @@ -601,6 +604,9 @@ const migrateData = async () => { 'RE_ASSIGN' ); + ALTER TABLE application_list_shape + ADD COLUMN assigner_action public.assigner_action; + DROP TYPE public.assigner_action_old; `) } From 52ce0d82f8b006a65ef6272d16dff279046d1795 Mon Sep 17 00:00:00 2001 From: Carl Smith <5456533+CarlosNZ@users.noreply.github.com> Date: Wed, 9 Nov 2022 15:20:34 +1300 Subject: [PATCH 11/18] Fix functions and migration script Fix mistake in review_assignment validity trigger --- .../43_views_functions_triggers.sql | 36 ++++++++++++++++--- database/migration/migrateData.ts | 14 +------- 2 files changed, 32 insertions(+), 18 deletions(-) diff --git a/database/buildSchema/43_views_functions_triggers.sql b/database/buildSchema/43_views_functions_triggers.sql index 709e71d2e..22e8cf0ee 100644 --- a/database/buildSchema/43_views_functions_triggers.sql +++ b/database/buildSchema/43_views_functions_triggers.sql @@ -697,12 +697,13 @@ DROP TRIGGER IF EXISTS review_assignment_validate_section_trigger ON public.revi CREATE TRIGGER review_assignment_validate_section_trigger BEFORE UPDATE ON public.review_assignment FOR EACH ROW - WHEN (NEW.trigger IS NULL AND OLD.trigger IS NULL) + WHEN (NEW.trigger IS NOT NULL AND OLD.trigger IS NULL) EXECUTE FUNCTION public.enforce_asssigned_section_validity (); --- FUNCTION to set `available_sections` on given review_assignments based on other assignments +-- FUNCTION to return `available_sections` for a given review_assignment based +-- on other assignments and allowed sections CREATE OR REPLACE FUNCTION public.review_assignment_available_sections (assignment public.review_assignment) - RETURNS text[] + RETURNS varchar[] AS $$ SELECT ARRAY ( WITH my_array AS ( @@ -729,8 +730,9 @@ CREATE OR REPLACE FUNCTION public.review_assignment_available_sections (assignme status = 'ASSIGNED' AND stage_id = $1.stage_id AND level_number = $1.level_number - AND application_id = $1.application_id - AND id <> $1.id)) + AND application_id = $1.application_id) + AND (available_sections = ANY ($1.allowed_sections) + OR $1.allowed_sections IS NULL)) $$ LANGUAGE sql STABLE; @@ -1416,6 +1418,29 @@ STABLE; -- APPLICATION_LIST_VIEW -- Aggregated VIEW method of all data required for application list page -- Requires an empty table as setof return and smart comment to make orderBy work (https://github.com/graphile/graphile-engine/pull/378) +DROP TABLE IF EXISTS application_list_shape CASCADE; + +CREATE TABLE IF NOT EXISTS application_list_shape ( + id int, + "serial" varchar, + "name" varchar, + template_code varchar, + template_name varchar, + applicant varchar, + org_name varchar, + stage varchar, + stage_colour varchar, + "status" public.application_status, + outcome public.application_outcome, + last_active_date timestamptz, + applicant_deadline timestamptz, + -- TO-DO: reviewer_deadline + assigners varchar[], + reviewers varchar[], + reviewer_action public.reviewer_action, + assigner_action public.assigner_action +); + CREATE OR REPLACE FUNCTION application_list (userid int DEFAULT 0) RETURNS SETOF application_list_shape AS $$ @@ -1456,6 +1481,7 @@ LANGUAGE sql STABLE; -- (https://github.com/graphile/graphile-engine/pull/378) +-- Required to make 'orderBy' work in application_list COMMENT ON FUNCTION application_list (userid int) IS E'@sortable'; -- APPLICATION_LIST_FILTERS diff --git a/database/migration/migrateData.ts b/database/migration/migrateData.ts index 8806dd100..395686fb9 100644 --- a/database/migration/migrateData.ts +++ b/database/migration/migrateData.ts @@ -6,7 +6,6 @@ import { execSync } from 'child_process' import path from 'path' import { readFileSync } from 'fs' import { getAppEntryPointDir } from '../../src/components/utilityFunctions' -import dbConnectInstance from '../../src/components/databaseConnect' // CONSTANTS const FUNCTIONS_FILENAME = '43_views_functions_triggers.sql' @@ -588,26 +587,15 @@ const migrateData = async () => { console.log(' - Update existing review_assignment to set is_locked=false when isSelfAssigned') // TODO - Not sure how to do this - // This logs is related to something which will be added by view_functions_triggers file - console.log(' - Add new generated column available_sections in review_assignment TABLE') - console.log(' - Remove type ASSIGN_LOCKED from assign_action ENUM') await DB.changeSchema(` - ALTER TYPE public.assigner_action RENAME to assigner_action_old; - - ALTER TABLE application_list_shape - DROP COLUMN IF EXISTS assigner_action; + DROP TYPE IF EXISTS public.assigner_action CASCADE; CREATE TYPE public.assigner_action AS ENUM ( 'ASSIGN', 'RE_ASSIGN' ); - - ALTER TABLE application_list_shape - ADD COLUMN assigner_action public.assigner_action; - - DROP TYPE public.assigner_action_old; `) } // Other version migrations continue here... From 446944483b295d54e932dec6bf15c6a202a1d4c3 Mon Sep 17 00:00:00 2001 From: Nicole Madruga Date: Thu, 10 Nov 2022 12:42:09 +1300 Subject: [PATCH 12/18] Set isLocked to be false in generateReviewAssignment for existing assignment --- database/migration/migrateData.ts | 3 --- .../src/generateReviewAssignments.ts | 10 ++++------ 2 files changed, 4 insertions(+), 9 deletions(-) diff --git a/database/migration/migrateData.ts b/database/migration/migrateData.ts index 395686fb9..7b64af8c1 100644 --- a/database/migration/migrateData.ts +++ b/database/migration/migrateData.ts @@ -584,9 +584,6 @@ const migrateData = async () => { ADD COLUMN data_view_code varchar; `) - console.log(' - Update existing review_assignment to set is_locked=false when isSelfAssigned') - // TODO - Not sure how to do this - console.log(' - Remove type ASSIGN_LOCKED from assign_action ENUM') await DB.changeSchema(` DROP TYPE IF EXISTS public.assigner_action CASCADE; diff --git a/plugins/action_generate_review_assignment_records/src/generateReviewAssignments.ts b/plugins/action_generate_review_assignment_records/src/generateReviewAssignments.ts index 4e2905cdc..5beeba2ec 100644 --- a/plugins/action_generate_review_assignment_records/src/generateReviewAssignments.ts +++ b/plugins/action_generate_review_assignment_records/src/generateReviewAssignments.ts @@ -457,21 +457,19 @@ const getNewOrExistingAssignmentStatus = ( isSelfAssignable: boolean, existingAssignment?: ExistingReviewAssignment ): AssignmentState => { - const isAssigned = existingReviewsAssigned.some( - ({ userId }) => userId === existingAssignment?.userId - ) + const isAssignedAndLocked = existingReviewsAssigned.some(({ isLocked }) => isLocked) // Create NEW or update EXISTING ReviewAssignment: // 1. If existing // - keep same status, isSelfAssignable - // - keep same isLocked if assigned + // - update isLocked = false (to release reviews after Application is re-submitted) // 2. If new reviewAssignment: // - status = Available (always) - // - isLocked = true if Assigned & locked (Default: false - Maybe needs checking if true - when LOQ sent) + // - isLocked = true if Assigned & locked - when LOQ sent (Default: false) // - isSelfAssignable = true if canSelfAssign (Default: false) return { status: existingAssignment?.status ?? ReviewAssignmentStatus.Available, isSelfAssignable: existingAssignment?.isSelfAssignable ?? isSelfAssignable, - isLocked: existingAssignment && isAssigned ? existingAssignment.isLocked : false, + isLocked: existingAssignment ? false : isAssignedAndLocked, } } From ec0538414ee31a6b1f08936545ad91f3f5b7c897 Mon Sep 17 00:00:00 2001 From: Nicole Madruga Date: Thu, 10 Nov 2022 12:57:14 +1300 Subject: [PATCH 13/18] Have to also lock the ReviewAssignment linked to the Review sending a LOQ so any other ReviewAssignments created also are locked at that point. --- .../src/updateReviewsStatuses.ts | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/plugins/action_update_review_status/src/updateReviewsStatuses.ts b/plugins/action_update_review_status/src/updateReviewsStatuses.ts index c45aedbe8..c5ba477b4 100644 --- a/plugins/action_update_review_status/src/updateReviewsStatuses.ts +++ b/plugins/action_update_review_status/src/updateReviewsStatuses.ts @@ -59,8 +59,8 @@ const updateReviewsStatuses: ActionPluginType = async ({ ): Promise => (await db.getAssociatedReviews(applicationId, stageId, level)).filter( (review: Review) => - review.reviewId !== reviewId && - (!statusToUpdate || statusToUpdate.includes(review.reviewStatus)) + (review.reviewId !== reviewId && !statusToUpdate) || + statusToUpdate.includes(review.reviewStatus) ) const getReviewAssignmentsWithoutReviewByLevel = async ( @@ -95,7 +95,11 @@ const updateReviewsStatuses: ActionPluginType = async ({ // Lock reviews to avoid other reviews submitted while awaiting changes required to applicant reviewsLocked.forEach((review) => { reviewsToUpdate.push({ ...review, reviewStatus: ReviewStatus.Locked }) - // TODO: Check with team should also lock the reviewAssignment ? + // Also lock the reviewAssignment - used as check for new Assignment generated (to be locked) + reviewAssignmentsToUpdate.push({ + reviewAssignmentId: review.reviewAssignmentId, + isLocked: true, + }) }) // Get all reviewAssignments without review on previous level @@ -132,7 +136,7 @@ const updateReviewsStatuses: ActionPluginType = async ({ for (const review of reviews) { const { reviewAssignmentId, reviewStatus } = review if (reviewStatus === ReviewStatus.Locked) { - // TODO: Check with team should also lock the reviewAssignment ? + // The reviewAssignment previously locked is unlocked by generateReviewAssignment } if (await haveAssignedResponsesChanged(reviewAssignmentId)) reviewsToUpdate.push({ ...review, reviewStatus: ReviewStatus.Pending }) From 33d8b60ea70adef34f0af1cb11fa15dd10d30d54 Mon Sep 17 00:00:00 2001 From: Nicole Madruga Date: Thu, 10 Nov 2022 13:53:28 +1300 Subject: [PATCH 14/18] Revert accidental change --- .../action_update_review_status/src/updateReviewsStatuses.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/action_update_review_status/src/updateReviewsStatuses.ts b/plugins/action_update_review_status/src/updateReviewsStatuses.ts index c5ba477b4..bae3c2102 100644 --- a/plugins/action_update_review_status/src/updateReviewsStatuses.ts +++ b/plugins/action_update_review_status/src/updateReviewsStatuses.ts @@ -59,8 +59,8 @@ const updateReviewsStatuses: ActionPluginType = async ({ ): Promise => (await db.getAssociatedReviews(applicationId, stageId, level)).filter( (review: Review) => - (review.reviewId !== reviewId && !statusToUpdate) || - statusToUpdate.includes(review.reviewStatus) + review.reviewId !== reviewId && + (!statusToUpdate || statusToUpdate.includes(review.reviewStatus)) ) const getReviewAssignmentsWithoutReviewByLevel = async ( From 6f187f3233f6a6ef99deeb8add5fa047323548b9 Mon Sep 17 00:00:00 2001 From: Nicole Madruga Date: Thu, 10 Nov 2022 14:03:27 +1300 Subject: [PATCH 15/18] Set ReviewAssignment to locked for Review submitted as Changes required from Applicant to be used as model for new ReviewAssignments created after that (so those wouldn't generate an unlocked assignment in the same level/stage) --- .../src/databaseMethods.ts | 17 +++++++++++++++++ .../src/updateReviewsStatuses.ts | 11 ++++++++++- 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/plugins/action_update_review_status/src/databaseMethods.ts b/plugins/action_update_review_status/src/databaseMethods.ts index b60c1fb15..daecf7ed5 100644 --- a/plugins/action_update_review_status/src/databaseMethods.ts +++ b/plugins/action_update_review_status/src/databaseMethods.ts @@ -1,6 +1,23 @@ import { ReviewStatus } from '../../../src/generated/graphql' const databaseMethods = (DBConnect: any) => ({ + getReview: async (reviewId: number) => { + const text = ` + SELECT + review_assignment_id AS "reviewAssignmentId" + FROM review + WHERE id = $1 + ` + + try { + const result = await DBConnect.query({ text, values: [reviewId] }) + return result.row[0] + } catch (err) { + console.log(err.message) + throw err + } + }, + getAssociatedReviews: async (applicationId: number, stageId: number, level: number) => { const text = ` SELECT diff --git a/plugins/action_update_review_status/src/updateReviewsStatuses.ts b/plugins/action_update_review_status/src/updateReviewsStatuses.ts index bae3c2102..df92ac81a 100644 --- a/plugins/action_update_review_status/src/updateReviewsStatuses.ts +++ b/plugins/action_update_review_status/src/updateReviewsStatuses.ts @@ -92,10 +92,19 @@ const updateReviewsStatuses: ActionPluginType = async ({ ReviewStatus.Draft, ]) + // Lock ReviewAssignment of Review subimmet as changes required to applicant + // this is required to new ReviewAssignments created also get locked + const currentReview = await db.getReview(reviewId) + if (currentReview) + reviewAssignmentsToUpdate.push({ + reviewAssignmentId: currentReview.reviewAssignmentId, + isLocked: true, + }) + // Lock reviews to avoid other reviews submitted while awaiting changes required to applicant reviewsLocked.forEach((review) => { reviewsToUpdate.push({ ...review, reviewStatus: ReviewStatus.Locked }) - // Also lock the reviewAssignment - used as check for new Assignment generated (to be locked) + // Also lock the reviewAssignment reviewAssignmentsToUpdate.push({ reviewAssignmentId: review.reviewAssignmentId, isLocked: true, From 20a80b498ef0f71322f96138e8039c923e422ea5 Mon Sep 17 00:00:00 2001 From: Nicole Madruga Date: Thu, 10 Nov 2022 14:21:06 +1300 Subject: [PATCH 16/18] Improve documentation about action to Generate Review Assignments --- documentation/List-of-Action-plugins.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/documentation/List-of-Action-plugins.md b/documentation/List-of-Action-plugins.md index a338fbb07..9ad44a113 100644 --- a/documentation/List-of-Action-plugins.md +++ b/documentation/List-of-Action-plugins.md @@ -460,7 +460,11 @@ See [Grant Permissions](#grant-permissions) above regarding acting on user-only ### Generate Review Assignments -Generates records in the `review_assignment` table -- i.e. which users (reviewers) are allowed to do a review for the current stage/level (and for which Sections). The records are set with `status` "Available" or "Assigned". With the properties to specify the type of assignament: `isSelfAssignable` if it should show for self-assignment when not assigned by another user. The `isLocked` property helps defining that the review can start but not be submitted - after the application has been back to Applicant for ammendments. +Generates records in the `review_assignment` table -- i.e. which users (reviewers) are allowed to do a review for the current stage/level (and for which Sections). +- Records are set with `status` "Available" or "Assigned". +- Each record has properties to specify the type of assignment: + - `isSelfAssignable` if it should show for self-assignment when not assigned by another user + - `isLocked` defining that the review can start but not be submitted (Used for applications which has been sent back to Applicant for ammendments) It also creates records in the `review_assignment_assigner_join` table -- basically a list of users who have permission to make the _assignments_ in the review_assignment table. From 5131912b6a345c97383d87db0d029386364d0c02 Mon Sep 17 00:00:00 2001 From: Nicole Madruga Date: Thu, 10 Nov 2022 15:18:36 +1300 Subject: [PATCH 17/18] Reverting changed for isLocked in generateReviewAssignments and updateReviewStatuses. isLocked logic as it is currently. And new ReviewAssignments gets created always as isLocked = false --- .../src/generateReviewAssignments.ts | 10 ++++++---- .../src/databaseMethods.ts | 19 ------------------- .../src/updateReviewsStatuses.ts | 18 ++---------------- 3 files changed, 8 insertions(+), 39 deletions(-) diff --git a/plugins/action_generate_review_assignment_records/src/generateReviewAssignments.ts b/plugins/action_generate_review_assignment_records/src/generateReviewAssignments.ts index 5beeba2ec..2f1d1c610 100644 --- a/plugins/action_generate_review_assignment_records/src/generateReviewAssignments.ts +++ b/plugins/action_generate_review_assignment_records/src/generateReviewAssignments.ts @@ -457,19 +457,21 @@ const getNewOrExistingAssignmentStatus = ( isSelfAssignable: boolean, existingAssignment?: ExistingReviewAssignment ): AssignmentState => { - const isAssignedAndLocked = existingReviewsAssigned.some(({ isLocked }) => isLocked) + const isAssigned = existingReviewsAssigned.some( + ({ userId }) => userId === existingAssignment?.userId + ) // Create NEW or update EXISTING ReviewAssignment: // 1. If existing // - keep same status, isSelfAssignable - // - update isLocked = false (to release reviews after Application is re-submitted) + // - set isLocked = false // 2. If new reviewAssignment: // - status = Available (always) - // - isLocked = true if Assigned & locked - when LOQ sent (Default: false) + // - isLocked = false (unless is existing, assigned and locked) // - isSelfAssignable = true if canSelfAssign (Default: false) return { status: existingAssignment?.status ?? ReviewAssignmentStatus.Available, isSelfAssignable: existingAssignment?.isSelfAssignable ?? isSelfAssignable, - isLocked: existingAssignment ? false : isAssignedAndLocked, + isLocked: existingAssignment && isAssigned ? existingAssignment.isLocked : false, } } diff --git a/plugins/action_update_review_status/src/databaseMethods.ts b/plugins/action_update_review_status/src/databaseMethods.ts index daecf7ed5..55bc67a93 100644 --- a/plugins/action_update_review_status/src/databaseMethods.ts +++ b/plugins/action_update_review_status/src/databaseMethods.ts @@ -1,23 +1,4 @@ -import { ReviewStatus } from '../../../src/generated/graphql' - const databaseMethods = (DBConnect: any) => ({ - getReview: async (reviewId: number) => { - const text = ` - SELECT - review_assignment_id AS "reviewAssignmentId" - FROM review - WHERE id = $1 - ` - - try { - const result = await DBConnect.query({ text, values: [reviewId] }) - return result.row[0] - } catch (err) { - console.log(err.message) - throw err - } - }, - getAssociatedReviews: async (applicationId: number, stageId: number, level: number) => { const text = ` SELECT diff --git a/plugins/action_update_review_status/src/updateReviewsStatuses.ts b/plugins/action_update_review_status/src/updateReviewsStatuses.ts index df92ac81a..861ab26b5 100644 --- a/plugins/action_update_review_status/src/updateReviewsStatuses.ts +++ b/plugins/action_update_review_status/src/updateReviewsStatuses.ts @@ -92,23 +92,10 @@ const updateReviewsStatuses: ActionPluginType = async ({ ReviewStatus.Draft, ]) - // Lock ReviewAssignment of Review subimmet as changes required to applicant - // this is required to new ReviewAssignments created also get locked - const currentReview = await db.getReview(reviewId) - if (currentReview) - reviewAssignmentsToUpdate.push({ - reviewAssignmentId: currentReview.reviewAssignmentId, - isLocked: true, - }) - // Lock reviews to avoid other reviews submitted while awaiting changes required to applicant reviewsLocked.forEach((review) => { reviewsToUpdate.push({ ...review, reviewStatus: ReviewStatus.Locked }) - // Also lock the reviewAssignment - reviewAssignmentsToUpdate.push({ - reviewAssignmentId: review.reviewAssignmentId, - isLocked: true, - }) + // TODO: Check with team should also lock the reviewAssignment ? }) // Get all reviewAssignments without review on previous level @@ -145,7 +132,7 @@ const updateReviewsStatuses: ActionPluginType = async ({ for (const review of reviews) { const { reviewAssignmentId, reviewStatus } = review if (reviewStatus === ReviewStatus.Locked) { - // The reviewAssignment previously locked is unlocked by generateReviewAssignment + // TODO: Check with team should also lock the reviewAssignment ? } if (await haveAssignedResponsesChanged(reviewAssignmentId)) reviewsToUpdate.push({ ...review, reviewStatus: ReviewStatus.Pending }) @@ -153,7 +140,6 @@ const updateReviewsStatuses: ActionPluginType = async ({ reviewsToUpdate.push({ ...review, reviewStatus: ReviewStatus.Pending }) } } - console.log('Resulting reviews to update', reviewsToUpdate) // Update review statuses for (const review of reviewsToUpdate) { From eb76dcb72fdf54c9930c292a6fbd5a5b2e4f3360 Mon Sep 17 00:00:00 2001 From: Nicole Madruga Date: Thu, 10 Nov 2022 15:20:42 +1300 Subject: [PATCH 18/18] Add log of reviews to be updated --- .../action_update_review_status/src/updateReviewsStatuses.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/action_update_review_status/src/updateReviewsStatuses.ts b/plugins/action_update_review_status/src/updateReviewsStatuses.ts index 861ab26b5..98ae12f25 100644 --- a/plugins/action_update_review_status/src/updateReviewsStatuses.ts +++ b/plugins/action_update_review_status/src/updateReviewsStatuses.ts @@ -140,7 +140,7 @@ const updateReviewsStatuses: ActionPluginType = async ({ reviewsToUpdate.push({ ...review, reviewStatus: ReviewStatus.Pending }) } } - + console.log('Resulting reviews to update', reviewsToUpdate) // Update review statuses for (const review of reviewsToUpdate) { const { reviewId, reviewStatus } = review