feat: users can save their progress #18511

douglasbouttell-camunda · 2024-05-14T13:28:16Z

Description

Some tasks require significant effort from the user or due to circumstances tasks may need to be transferred to other people. Camunda should support allowing users to save their progress in order to help their job.

Related issues

closes #17609

github-actions · 2024-05-14T14:09:42Z

Tasklist Test Results

143 files 143 suites 1h 36m 41s ⏱️
549 tests 544 ✅ 5 💤 0 ❌
547 runs 542 ✅ 5 💤 0 ❌

Results for commit 9102846.

♻️ This comment has been updated with latest results.

vsgoulart

@douglasbouttell-camunda This PR is already a bit too, next time please it into smaller ones (a PR for the visual component and one for the mutation logic for example)

vsgoulart · 2024-05-15T14:41:01Z

tasklist/client/src/modules/mutations/useSaveDraftVariables.ts

+      const {error} = await request(api.saveVariables({taskId, variables}));
+
+      if (error !== null) {
+        throw error ?? new Error('Could not complete task');


Suggested change

throw error ?? new Error('Could not complete task');

throw error;

vsgoulart · 2024-05-15T14:42:33Z

tasklist/client/src/modules/api.ts

+    return new Request(getFullURL(`/v1/tasks/${taskId}/variables`), {
+      ...BASE_REQUEST_OPTIONS,
+      method: 'POST',
+      body: JSON.stringify(body),


Nitpick, but there's no need to create a var since it's no reused

Suggested change

body: JSON.stringify(body),

body: JSON.stringify({

variables,

}),

vsgoulart · 2024-05-15T14:43:33Z

tasklist/client/src/Tasks/Task/FormJS/index.tsx

+function formatFormDataToVariables(data: object) {
+  return Object.keys(data).map((key) => ({
+    name: key,
+    // @ts-expect-error TS7053: data[key] can be anything


Let's fix the types instead of ignoring them

vsgoulart · 2024-05-15T14:45:10Z

tasklist/client/src/Tasks/Task/Variables/VariableEditor/index.tsx

@@ -79,6 +79,10 @@ const VariableEditor: React.FC<Props> = ({
 }) => {
  const {dirtyFields} = useFormState<FormValues, Partial<FormValues>>();

+  const nonDraftVariables = variables.filter(
+    (v) => v.value !== null || v.previewValue !== null,


Suggested change

(v) => v.value !== null || v.previewValue !== null,

({value, previewValue}) => value !== null || previewValue !== null,

vsgoulart · 2024-05-15T17:32:29Z

tasklist/client/src/modules/components/SaveButton.tsx

+        })}
+        description="Saving..."
+        iconDescription="Saving"
+        status={'active'}


Suggested change

status={'active'}

status="active"

vsgoulart · 2024-05-15T18:31:57Z

tasklist/client/src/Tasks/Task/FormJS/index.test.tsx

+      vi.advanceTimersByTime(5000);
+    });
+
+    expect(screen.getByRole('status')).toBeEmptyDOMElement();


Isn't it bad for a11y to have it empty? maybe we should stop rending it

vsgoulart · 2024-05-15T18:33:01Z

tasklist/client/src/Tasks/Task/FormJS/index.test.tsx

+    await waitFor(() => {
+      expect(screen.getByTitle(/save current progress/i)).not.toHaveAttribute(
+        'disabled',
+      );
+    });


Suggested change

await waitFor(() => {

expect(screen.getByTitle(/save current progress/i)).not.toHaveAttribute(

'disabled',

);

});

await waitFor(() => {

expect(screen.getByTitle(/save current progress/i)).toBeDisabled();

});

vsgoulart · 2024-05-15T19:12:03Z

tasklist/client/src/Tasks/Task/Variables/index.tsx

+                    savingState === 'finished' ? (
+                      <SaveSuccessMessage />
+                    ) : savingState === 'error' ? (
+                      <SaveFailedMessage />
+                    ) : undefined
+                  }


We already have the AsyncActionButton, I see no reason to not reuse.

the AsyncActionButton doesn't do what the UX design wants.

vsgoulart · 2024-05-15T19:13:15Z

tasklist/client/src/Tasks/Task/Variables/index.tsx

+  const newVariables: Array<Pick<Variable, 'name' | 'value'>> = [];
+
+  variables.forEach((variable) => {
+    if (variable.draft) {


Let's invert the logic to use a explicit check like everywhere else

Suggested change

if (variable.draft) {

if (variable.draft === null) {

vsgoulart · 2024-05-15T19:14:39Z

tasklist/client/src/Tasks/Task/FormJS/index.tsx

+              save(
+                formatFormDataToVariables(
+                  formManagerRef.current
+                    ?.get('form')
+                    ._getSubmitData() as object,
+                ),


We shouldn't be using form-js internal methods like this, part of the epic should have been changing adapting form-js and make this public...

vsgoulart · 2024-05-30T18:35:18Z

@esraagamal6 Could you please test this PR?

…able

douglasbouttell-camunda requested a review from a team May 14, 2024 13:28

douglasbouttell-camunda self-assigned this May 14, 2024

github-actions bot added the component/tasklist Related to the Tasklist component/team label May 14, 2024

douglasbouttell-camunda marked this pull request as ready for review May 15, 2024 08:19

vsgoulart reviewed May 15, 2024

View reviewed changes

douglasbouttell-camunda requested a review from vsgoulart May 16, 2024 15:30

douglasbouttell-camunda assigned vsgoulart May 17, 2024

vsgoulart force-pushed the tasklist/17609-manual-save branch 4 times, most recently from 6f16f98 to 114af82 Compare May 30, 2024 18:23

vsgoulart approved these changes May 30, 2024

View reviewed changes

vsgoulart requested a review from esraagamal6 May 30, 2024 18:35

douglasbouttell-camunda and others added 15 commits May 31, 2024 15:31

feat: users can save their progress

2648974

fix: update snapshots

1b1291f

fix: pr comments

e3b9de7

fix: form-js types

d8f7122

fix: visual regression mock response

7c4c349

fix: tests which check for save status

1844402

test: Use better assertion

29325d6

refactor: Fix type

b9c97c3

test: Use runOnlyPendingTimers

57aa41e

refactor: Use render props pattern

243cfd7

refactor: Use event listener instead of internal API

1016035

refactor: Fix warnings

89d1d1f

fix: Prevent save from being disabled after removing a new saved vari…

70165f2

…able

test: Fix selector

dee16f0

fix: Fix task state save message breaking line

5b0902c

feat: Load truncated draft variables

9102846

vsgoulart force-pushed the tasklist/17609-manual-save branch from db2cbff to 9102846 Compare May 31, 2024 15:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: users can save their progress #18511

feat: users can save their progress #18511

douglasbouttell-camunda commented May 14, 2024

github-actions bot commented May 14, 2024 •

edited

vsgoulart left a comment

vsgoulart May 15, 2024

vsgoulart May 15, 2024

vsgoulart May 15, 2024

vsgoulart May 15, 2024

vsgoulart May 15, 2024

vsgoulart May 15, 2024

vsgoulart May 15, 2024

vsgoulart May 15, 2024

douglasbouttell-camunda May 16, 2024

vsgoulart May 15, 2024

vsgoulart May 15, 2024

vsgoulart commented May 30, 2024

	throw error ?? new Error('Could not complete task');
	throw error;

-      body: JSON.stringify(body),
+      body: JSON.stringify({
+      variables,
+    }),

	(v) => v.value !== null \|\| v.previewValue !== null,
	({value, previewValue}) => value !== null \|\| previewValue !== null,

feat: users can save their progress #18511

Are you sure you want to change the base?

feat: users can save their progress #18511

Conversation

douglasbouttell-camunda commented May 14, 2024

Description

Related issues

github-actions bot commented May 14, 2024 • edited

Tasklist Test Results

vsgoulart left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vsgoulart commented May 30, 2024

github-actions bot commented May 14, 2024 •

edited