feat: user officer experiment safety review workflow #917
Conversation
…one. Now we can move to frontend a bit
yoganandaness
changed the title
yoganandaness
force-pushed
the
SWAP-4396-user-officer-experiment-safety-review-workflow
branch
8 times, most recently
from
270832e to
de6b18a
Compare
yoganandaness
requested review from
William-Edwards-STFC
and removed request for
a team
yoganandaness
force-pushed
the
SWAP-4396-user-officer-experiment-safety-review-workflow
branch
from
7a22bf6 to
4f2a54d
Compare
yoganandaness
force-pushed
the
SWAP-4396-user-officer-experiment-safety-review-workflow
branch
5 times, most recently
from
302773a to
c08d013
Compare
yoganandaness
force-pushed
the
SWAP-4396-user-officer-experiment-safety-review-workflow
branch
2 times, most recently
from
8bde1dd to
6dd03ff
Compare
…re into SWAP-4396-user-officer-experiment-safety-review-workflow
yoganandaness
force-pushed
the
SWAP-4396-user-officer-experiment-safety-review-workflow
branch
from
6dd03ff to
657061c
Compare
yoganandaness
requested review from
jekabs-karklins,
martin-trajanovski and
TCMeldrum
Screen.Recording.2025-01-31.at.11.11.50.2.mov |
|
Is ELI planning on using this feature? If so, perhaps @gnyiri would be better suited to review this than STFC. |
| ALTER TABLE proposal_statuses RENAME TO statuses; | ||
| ALTER TABLE proposal_status_actions RENAME TO status_actions; |
There was a problem hiding this comment.
Question: Is there a reason why these are not workflow_statuses and workflow_status_actions?
There was a problem hiding this comment.
So the idea here is to make a generic table for statuses and workflows, and use it for various purposes including Proposal and Experiment and much more.
This is the reason, why we have the new column entity_type
|
|
||
| BEGIN | ||
| /* Creating enum for entity_type */ | ||
| CREATE TYPE entity_type AS ENUM ('PROPOSAL', 'EXPERIMENT'); |
There was a problem hiding this comment.
suggestion: Should we call this EXPERIMENT_SAFETY ? Calling it just EXPERIMENT could be a little too general
There was a problem hiding this comment.
Agreed. Good point. Will update in the next PR.
| @@ -79,6 +79,11 @@ export enum Event { | |||
| INSTRUMENTS_ASSIGNED_TO_TECHNIQUE = 'INSTRUMENTS_ASSIGNED_TO_TECHNIQUE', | |||
| INSTRUMENTS_REMOVED_FROM_TECHNIQUE = 'INSTRUMENTS_REMOVED_FROM_TECHNIQUE', | |||
| PROPOSAL_ASSIGNED_TO_TECHNIQUES = 'PROPOSAL_ASSIGNED_TO_TECHNIQUES', | |||
| EXPERIMENT_ESF_SUBMITTED = 'EXPERIMENT_ESF_SUBMITTED', | |||
There was a problem hiding this comment.
Question: I do not see these events being sent to the event bus, is that intentional?
There was a problem hiding this comment.
This part is not yet done. Coming soon.
There was a problem hiding this comment.
One nice improvement would be if it would be possible to make sure to use typescript discriminated unions to make sure an invalid combination of entityType and shortcode is not possible.
Something of this type was done in
apps/frontend/src/models/questionary/QuestionarySubmissionState.ts
and then when used in the reducer
apps/frontend/src/models/questionary/QuestionarySubmissionState.ts
when you do a switch or an if statement or in any other way let TS know to reduce the datatype, it prevents you from accessing attributes of the event that do not exist. workflows case it would prevent expecting status shortCode that is impossible, e.g. ESF approved for proposal workflow
| @@ -2,11 +2,11 @@ query getCallSubmissionDetails($callId: Int!) { | |||
| call(callId: $callId) { | |||
| proposalWorkflowId | |||
| submissionMessage | |||
| proposalWorkflow { | |||
| proposalWorkflowConnectionGroups { | |||
| workflow { | |||
There was a problem hiding this comment.
nitpick: I think you might have left the proposal workflow here as that is what the call will have.
There was a problem hiding this comment.
proposalWorkflow resolver has been replaced with workflow
…re into SWAP-4396-user-officer-experiment-safety-review-workflow
Even though it is a ESS exclusive feature for now, this involves changes to the existing system and architecture. So i think it is a good idea to go through the change and let me know in case of any concerns. The most significant changes are in the tables: proposal_statuses, proposal_workflows |
|
@gnyiri Requesting you to review this as well. |
Description
Workflow for Experiment Safety Review
Motivation and Context
Leveraging Proposal workflow to implement the experiment workflow. This process includes generalizing the Backend architecture to make it agnostic to Proposal and extensible for Experiment as well.
How Has This Been Tested
Unit test and e2e
Fixes
Changes
Depends on
Tests included/Docs Updated?