Feat/817 copy another user rights for a new user

[helakaraa]

No description provided.

[ptitFicus]

We don't need to update backend for this, I would prefer fetching target user rights in frontend first, and sending these rights in the user creation request.
This would keep backend code simpler.

Moreover, UI should either allow to copy user rights OR specify manual rights. Right now both are displayed, and if I select a user to copy AND specify manual rights, only manual rights are taken.

[ptitFicus]

There is still some issues with the form as it is:

• If I first set custom rights for a tenant, and then select a user to copy right for, tenant appears twice in the form with different right, which doesn't make sense. As indicated in my previous comment, I think the best way to avoid this is to either display "user to copy" with a single user select OR a complete right form (to set rights manually). For now it's not a big deal if we can't see / change rights of the selected user while copying right.
• Reselecting the same user in select result in an error:
  
• This modification breaks the UI (an error is displayed) if invitation mode is "mail" instead of "request response", I dont understand why this change was needed.

[ptitFicus]

There is no async / await syntax in Izanami right now, and I would like to keep it this way to keep things simple and avoid associated pitfalls.
Therefore could you use simple promise syntax ?

[helakaraa]

Yes, I need to remove it since I use it to fetch userData in Submit function

[ptitFicus]

This modification make "rights" label appear under the checkbox. To avoid this behaviour, you need to do () => "". This is kind of a bug in react-forms.

[helakaraa]

I did not pay attention that the label value was changed, I will replace it

[ptitFicus]

This catch simply rethrow the error, I don't understand what it's for.
Moreover it's present in this if branch, but absent from else.

[ptitFicus]

This whole type is very confusing, invitationUrl is listed but never actually used, Error is too generic. I woulc prefer to just type data as it was done before and let Typescript infer the rest.

[ptitFicus]

Since queryUser doesn't change anything server side it should be a query. However I would prefer not having a query at all, and chaining it with createInvitation as it was done before (sorry, I told you that userToCopy wasn't necessary anymore, however you'll need it to know it you need to queryUser or not).

Having this logic in mutation makes more sense to me, rather than bloating jsx with this logic.

[ptitFicus]

As I told you in my previous review, this kind of code is not very usefull, since you just rethrow the error.

[helakaraa]

What do you think about this version :

const inviteUserMutation = useMutation(
    async (data: {
      email: string;
      admin: boolean;
      rights: TRights;
      userToCopy: string;
    }) => {
      if (data.userToCopy) {
        try {
          const res: TUser = await queryUser(data.userToCopy);
          data = { ...data, rights: res.rights };
        } catch (error) {
          throw new Error("Error fetching user rights: " + error);
        }
      }
      return createInvitation(data.email, data.admin, data.rights);
    }
  );
   onSubmit={async (ctx) => {
                  const backendRights = rightStateArrayToBackendMap(ctx.rights);
                  let payload = {
                    rights: backendRights,
                    admin: ctx.admin,
                    email: ctx.email,
                    userToCopy: ctx.userToCopy?.value,
                  };
                  return inviteUserMutation
                    .mutateAsync(payload)
                    .then((response) => {
                      if (response && response.invitationUrl) {
                        setCreationUrl(response.invitationUrl);
                      }
                      setCreating(false);
                    })
                    .catch((error) => {
                      throw new Error(error);
                    });
                }}

[ptitFicus]

Its better, except that your code is using async/await (see my previous comment about that) and that it remains a catch that just rethrow the error (as I mentionned twice already, I don't see the point).

[helakaraa]

@ptitFicus , I commit a modification without Async and double throwing error.