Optionally add reason when completing action #543

denektenina · 2021-08-17T14:49:13Z

Context: Users of BMT have asked for a feature where they can provide a reason when completing an action. After having talked with the customer, we now have the following solution for the frontend. In addition, the team decided to add a new table, called closingRemarks, so that we have a way of separating notes and closing remarks in case this is needed in the future. This PR is now using the new end point for closing remarks when saving.

Solution in steps:

An incomplete action looks like this in the edit action panel:

The user clicks the "Complete action"-button at the bottom to complete the action. A text field and confirm buttons appears. Nothing is saved until the user clicks "confirm".

Regardless of whether or not the user writes a text before clicking "confirm" - a closingRemark will be saved. If it is empty, the notes list will show the empty comment:

If the user writes a text, the text will also be in the list:

To save the complete-status and to save the closingRemark, we have to perform two mutations. The second one is only done after the first has completed successfully. If one of the two calls fail, one of the following error displays will be shown:

a) If saving complete-status fails - shows the same error display as if saving any other fields on the action fails (has been moved from top of form to bottom):

b) If saving closingRemark fails - shows a different error in almost the same place:

Comments / Improvements:

The closing comments are no longer prefixed with the text "Action completed: " or "Action un-completed", as this is not necessary now that we have a separate type for these comments.
It is no longer possible to re-open a closed action. This is why we are now using a button at the bottom and not a switch or checkbox.
If the user receives an error on saving closing remark, they may be confused about what failed: is the action completed or not?
I have now added a background color to the closing comment
When an action is successfully completed, this is shown at the top of the view with a check mark:

maiaht

It seems like the text box for the closing remark is placed too far to the left compared to the other elements in the side panel.

I also have a suggestion:
If you have set the switch to completed, and then change your mind such that you don't want to add a closing remark and set the switch back to in-completed, then perhaps the text box can disappear? So, to sum up, if the text box is empty and the switch is changed from complete to in-complete, then the text box should disappear.

frontend/src/components/Action/EditForm/ActionEditSidebar.tsx

frontend/src/components/Action/EditForm/ActionEditForm.tsx

ReedOnly · 2021-08-31T09:28:01Z

/azp run

azure-pipelines · 2021-08-31T09:28:13Z

Azure Pipelines successfully started running 2 pipeline(s).

petterwildhagen · 2021-09-02T06:56:01Z

frontend/cypress/integration/actions_spec.ts

+            })
+        }
+
+        it('Action can be completed without writing a reason', () => {


Clarify with customer if this is possible?

This was possible until yesterday, but now it seems it should be mandatory, so I think I will change it (checking with Andreas first)

Decided in daily standup that we will merge this as it is and fix the change as a new feature if they want to make it mandatory

petterwildhagen · 2021-09-02T07:06:09Z

frontend/cypress/integration/actions_spec.ts

@@ -288,6 +365,34 @@ const createEditSeed = () => {
    return { seed, existingAction, existingNotes }
 }

+const createCompleteActionSeed = () => {


Avoid coupling initial state to actual test case. This almost does the same as createEditSeed. It is better to create an evaluation that has actions , k.e. createEvaluationWithSomeActions and pass completeState as optional parameter (if not set randomize as is now) - this can then be used in all testcases, see #586

Good point! Fixed now.

maiaht

The EvaluationGQL.ts file is now an empty file, but it should be deleted

frontend/src/components/ActionTable/ActionTableForOneUserWithApi.tsx

petterwildhagen

Nice refactoring of setup of initial state.

achaikou

🐌 [world's fastest delivery service] Review started yesterday has arrived! 🐌
Nothing critical, so just take a look if there is something that is worth fixing in the next PR.

frontend/cypress/support/action.ts

frontend/cypress/integration/actions_spec.ts

frontend/src/components/Action/EditForm/ActionEditSidebar.tsx

achaikou · 2021-09-01T16:45:51Z

frontend/src/components/Action/EditForm/ActionEditForm.tsx

@@ -39,8 +52,9 @@ const ActionEditForm = ({ action, connectedQuestion, possibleAssignees, possible
    const [dueDate, setDueDate] = useState<Date>(new Date(action.dueDate))
    const [priority, setPriority] = useState<Priority>(action.priority)
    const [description, setDescription] = useState<string>(action.description)
-    const [onHold, setOnHold] = useState<boolean>(action.onHold)


Should "onHold" be removed from backend?

I guess that's something for the team to discuss

frontend/cypress/integration/actions_spec.ts

frontend/cypress/support/action.ts

frontend/cypress/integration/actions_spec.ts

denektenina linked an issue Aug 17, 2021 that may be closed by this pull request

#21.A Set a ´lukketekst´ when closing an action. #441

Closed

denektenina self-assigned this Aug 17, 2021

denektenina requested a review from achaikou August 17, 2021 15:13

denektenina added the blocked label Aug 20, 2021

denektenina marked this pull request as draft August 20, 2021 08:00

denektenina force-pushed the completed_reason branch from ec1904c to d49454c Compare August 25, 2021 11:23

lightenup removed the blocked label Aug 26, 2021

denektenina force-pushed the completed_reason branch from 205598c to 3bdc53d Compare August 27, 2021 05:59

denektenina marked this pull request as ready for review August 27, 2021 06:37

denektenina requested a review from maiaht August 27, 2021 06:44

maiaht reviewed Aug 27, 2021

View reviewed changes

frontend/src/components/Action/EditForm/ActionEditSidebar.tsx Outdated Show resolved Hide resolved

frontend/src/components/Action/EditForm/ActionEditForm.tsx Outdated Show resolved Hide resolved

denektenina force-pushed the completed_reason branch from 3bdc53d to cb60939 Compare August 27, 2021 11:26

denektenina requested review from maiaht and ErlendHaa August 30, 2021 10:57

denektenina force-pushed the completed_reason branch from b84a735 to 3b16c00 Compare August 31, 2021 08:55

denektenina force-pushed the completed_reason branch from 3b16c00 to fe6d8f7 Compare September 1, 2021 08:49

petterwildhagen reviewed Sep 2, 2021

View reviewed changes

denektenina force-pushed the completed_reason branch from df089fa to ab54038 Compare September 2, 2021 07:01

denektenina added 2 commits September 2, 2021 09:10

Optionally add reason when completing action

097ec3f

Added cypress tests

ce148ef

petterwildhagen reviewed Sep 2, 2021

View reviewed changes

Merge with master and fixed failing cypress-tests

1a2d255

denektenina force-pushed the completed_reason branch from ab54038 to 1a2d255 Compare September 2, 2021 08:15

Removed a duplicate

fec3cbe

denektenina requested a review from petterwildhagen September 2, 2021 09:05

maiaht suggested changes Sep 2, 2021

View reviewed changes

frontend/src/components/ActionTable/ActionTableForOneUserWithApi.tsx Outdated Show resolved Hide resolved

frontend/src/components/ActionTable/ActionTableForOneUserWithApi.tsx Outdated Show resolved Hide resolved

CR comments

6a38106

denektenina requested a review from maiaht September 2, 2021 09:21

maiaht approved these changes Sep 2, 2021

View reviewed changes

petterwildhagen approved these changes Sep 2, 2021

View reviewed changes

denektenina merged commit 0387b14 into equinor:master Sep 2, 2021

denektenina deleted the completed_reason branch September 2, 2021 09:37

achaikou reviewed Sep 2, 2021

View reviewed changes

achaikou mentioned this pull request Sep 8, 2021

Cypress test edit action in dashboard actions list NB: OLD - these changes will be integrated into new PRs and this will eventually be deleted #593

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Optionally add reason when completing action #543

Optionally add reason when completing action #543

denektenina commented Aug 17, 2021 •

edited

Loading

maiaht left a comment

ReedOnly commented Aug 31, 2021

azure-pipelines bot commented Aug 31, 2021

petterwildhagen Sep 2, 2021

denektenina Sep 2, 2021

denektenina Sep 2, 2021

petterwildhagen Sep 2, 2021

denektenina Sep 2, 2021

maiaht left a comment

petterwildhagen left a comment

achaikou left a comment

achaikou Sep 1, 2021

denektenina Sep 10, 2021

Optionally add reason when completing action #543

Optionally add reason when completing action #543

Conversation

denektenina commented Aug 17, 2021 • edited Loading

maiaht left a comment

Choose a reason for hiding this comment

ReedOnly commented Aug 31, 2021

azure-pipelines bot commented Aug 31, 2021

petterwildhagen Sep 2, 2021

Choose a reason for hiding this comment

denektenina Sep 2, 2021

Choose a reason for hiding this comment

denektenina Sep 2, 2021

Choose a reason for hiding this comment

petterwildhagen Sep 2, 2021

Choose a reason for hiding this comment

denektenina Sep 2, 2021

Choose a reason for hiding this comment

maiaht left a comment

Choose a reason for hiding this comment

petterwildhagen left a comment

Choose a reason for hiding this comment

achaikou left a comment

Choose a reason for hiding this comment

achaikou Sep 1, 2021

Choose a reason for hiding this comment

denektenina Sep 10, 2021

Choose a reason for hiding this comment

denektenina commented Aug 17, 2021 •

edited

Loading