ms2/431-process-list-view

[Ph4t3]

Summary

Introduces new routes /processes/editor and /processes/list. /editor has the same functionality as /processes did before while processes still exists in it's former state and as it's own page but is subject to change later. /list is a new read-only view of the process-list, with changed buttons and filtering to only display processes with released versions.

Details

Addresses #431

/list is now a read-only version of /processes and /editor, with different buttons in the process list, not being able to move objects in the list, and when opening a process not being able to edit the process in the modeller. The properties panel in the modeller still displays but all functions are not clickable.

Only processes with a released version get shown in /list, but all folders do, no matter if they contain a released version or not.
A message in the top action bar is displayed if one is in a folder that does not contain a process with a released version. This message only pertains to the individual folder one is in, not to potential released versions in sub-folders of it.

Always open the last released version when opening a process in the /list view.

Introduced readOnly booleans to all necessary components to achieve the read-only capability while using the same components for /editor and /list.

As /processes still exists in it's former state, and /editor is just a copy of it, the current playwright test should still be sufficient. A new test that addresses the split between /editor and /list and makes the current one for /processes redundant will be handled in a separate PR, especially once /processes will be turned into a different kind of page.

[FelipeTrost]

Solid overall, but there are many little details that need to addressed.

Also, the structure for the process list is quite repetitive, there are three options (edit, list, and no option selecte) and each file is almost identical.

For me it makes more sense to have one directory like this
.../[mode]/
.../[mode]/folder/[folderId]
.../[mode]/[processId]

And to handle the case where there is no mode selected do a rewrite in the next config something like this

{
source: '/processes'
destination: '/my/processes/edit/'
}

Also there was quite a lot of prop drilling, in this case I think it would have been better to use a react context

[FelipeTrost]

Where did these colors come from? If they're from ant design, I think it would be better to use them through the context with theme.useToken()

[FelipeTrost]

Here the prop should probably be called disabled, as this component can be used in other places where disabling it doesn't mean that its read only

[FelipeTrost]

I'm not so sure if this is the best way to go about, but its fine for now

[FelipeTrost]

edit and delete buttons need to be disabled

[FelipeTrost]

This should be removed (I forgot to take it out)

[FelipeTrost]

This is more a personal opinion, disregard it if you don't like it.
On these buttons it would be cleaner to have the ternary operator on the title instead of the hole button.
Then the only issue would be the update/view prop, for that you could write the action button like this

      const ActionButton: React.FC<ActionButtonProps> = ({
        title,
        action,
        icon,
        permission,
        ...actions
      }) => {
        if (permission) (actions as any)[permission] = true;

        return (
          <AuthCan {...resource} {...actions}>
            <Tooltip placement="top" title={title}>
              <Button
                className={classNames(styles.ActionButton)}
                type="text"
                icon={icon}
                onClick={() => action(record)}
              />
            </Tooltip>
          </AuthCan>
        );
      };

[FelipeTrost]

Here I'm not so sure if disabling the inputs is the right solution, I think the question should be how would the uesr open the Modal for copying/editing/importing if he is in the readOnly list, does this happen?

[FelipeTrost]

To filter what is shown in the context menu you check if the component is being used in the list and you also don't pass in actions that modify processes. The second one is a bit redundant, you could just check wether you're in the list and based on that filter out actions that are unwanted.

[FelipeTrost]

• This component takes in a readOnly prop and also has a isReadOnlyListView variable inside, do these refer to different things?
• Although drag and drop doesn't work when readOnly=true, I think we should still disable it, I think this can be done in a future pr

[FelipeTrost]

You could call

  const process = await getProcess(processId, !selectedVersionId);

Above the if statement, to avoid fetching the process twice, this is a tradeoff between

• Possibly fetching versions that are not needed
• Fetching the process twice when it does have versions

I think it's more common that the page is going to be opened when it has versions

[OhKai]

@Ph4t3 Did you see Felipe's comments?

[FelipeTrost]

Almost there

In the read only mode you should still be able to see user tasks and script tasks in the modeler, but this should be done in a separate pr.

[FelipeTrost]

Some cases are missing:

• What is expected when going to /processes/, the redirect should check if process id is not "editor" or "list" and redirect
• No handling for when a space id is also in the url

Also, the rewrite that you wrote works, because then another rewrite changes /process/editor to /my/process/editor, I'm not so sure if that is the best way to go about it, because how it works isn't immediately obvious from reading the code.

[Ph4t3]

/processes/ is only left in temporarily because it is the current starting page and removing it will be done in a separate PR once a new starting page was built, so is it okay to just leave the redirects like this in the meantime? I will address the issue with the space id so that works correctly in the meantime though.

[FelipeTrost]

Many files use this context to build urls, you could return processContextPath from here to avoid having to do a lot of comparisons in other places. Additionally, this makes it so that if we change urls again, we have to do less changes.

[github-actions]

✅ Successfully created Preview Deployment.

https://pr-635---ms-server-staging-c4f6qdpj7q-ew.a.run.app