let's move those lines into a hook - I think a new hook useProductSupplierActions could be a good candidate.
I see that in most list pages we had this type of logic in useXListTableData hook, but I think a separate hook is a better place for such logic.

shouldn't it be replaced by the translation key?

I forgot to change it

should it stay empty or should be replaced by the translatoin key?

I don't mind this version, but couldn't we potentially refactor the findActions util to include the new condition (roles)?
There is also a possibility to pass customFilter to findActions so maybe it's just a matter of passing your hasPermissionsToProductSourceActions as customFilter prop.

just wanted to mention that I am not a fan of passing props using the spread operator. It makes the code difficult to debug if something breaks (you won't be able to search by a specific prop using ctrl+f, because it might be hidden inside ...clickableActions). I experienced such a nightmare while debugging our Select component where it is mostly based on spread operators as props.

yeah, but in this case clickableActions are created above it, I think in the Select component it was much more complicated

it was meant to be a "general" comment why I am against the spread operator even though as you said - it doesn't make it difficult to read in this case.
No need to change, but we should use it carefully overall.

maybe to make it shorter/more readable let's define the labels as an object that could look like:

const labels = {
  title: {
   label: '...',
   defaultLabel: '...',
  },
  content: {
    label: '...',
    defaultLabel: '...',
  }
}

here, the actions are not that big yet, but just wanted to mention that we used to add the useMemo for actions.
The actions used to be a const, not a method, and the eventual id has only been passed to the onClick by ActionDots component and its id prop., e.g.:

Cell: row => (
        <ActionDots
          dropdownPlacement="right"
          dropdownClasses="action-dropdown-offset"
          actions={findActions(actions, row, { customFilter: customActionFilter, highestRole })}
          id={row.original.id}
        />),

and an action:

{
    defaultLabel: 'Publish stock list',
    label: 'react.stocklists.publish.label',
    leftIcon: <RiFile3Line />,
    isPublished: false,
    onClick: publishStocklists,
    minimumRequiredRole: 'Admin',
},

In this case as the actions are not that big, I don't know if re-rendering of them will impact the performance much (I mean, if adding the useMemo would help or rather make the performance get worse).

I think it's better to leave to the current version because now it's just a ternary operator for now. In findActions we have much more complexity. Additionally, we can't pass row to the hasPermissionsToProductSourceActions and this function needs to be refactored especially for this one use case

how about buttons?.map?.((button))?

key is missing

you have this part available in getAppContext (isUserAdmin).

To be honest, I am not a big fan of getting all of those: isUserAdmin, isUserManager, etc. from the getAppContext, I think it should be calculated on the frontend part based on the users' roles. For me it is a little bit different when I have a function hasPermissionsTo... and I am passing, let's say, half of the things that should be checked within this function (In this example isAdmin should be passed from getAppContext, but I think it is a purpose of this function - to check if user has admin role and if has a product manager permission). But If the isAdmin is available from the getAppContext, I am going to change it.

neither am I, it probably needs a deeper refactor, but just wanted to mention that we have it available.
I like your idea of adding roles property for the user dto and I hope this will go through.

it was meant to be a "general" comment why I am against the spread operator even though as you said - it doesn't make it difficult to read in this case.
No need to change, but we should use it carefully overall.

I don't think we need this component.
I think you can use the confirmAlert from react-confirm-alert package which we have been using everywhere.

@drodzewicz thanks for catching that, but I wouldn't agree that we do not need this component. I proposed such a solution in the ticket (check the dev note), that we should use the confirmAlert but it'd be good to create a reusable component (redesigned) as @alannadolny did, because there is a new design for the confirm modal, and just use it in the customUI prop of confirmAlert.

so in a nutshell @alannadolny the component is fine, but you could remove the <Modal> and all the logic related to opening/closing the modal, since confirmAlert should handle that.

I would very much like to avoid cases where we have a number of different styled components that serve the same purpose.
The design for the new and the old confirm modal alert are basically identical

OLD	NEW

I think it would be better to create some kind of a wrapper around the existing react-confirm-alert which adds the custom styles on top of the library component and which is used in a similar way that we use the confirmAler() at the moment.
Later gradually we can replace the old confirmAlert with the new one, since that is most likely where we are heading with the redesign.

The „wrapper” you mention is exactly what the customUI prop does. You use the confirmAlert in the old way and just add one additional prop (customUI) where you can pass a component.
For me it sounds pretty simple, but I am not sure if we are on the same page.

this should probably be in permissionUtils.js

I think it would be better to achieve this with <button disabled={!actions} /> and in css target .action-dots:disabled sudo class

I assume you wanted to prevent the dropdown from appearing on button click of there are no actions available.
You can achieve this with <button disabled={!actions} /> it should prevent the click event and should not activate the dropdown

As I mentioned before, this should probably be handled with :disabled sudo class instead of separate classes.

Additionally I think that we should add styling opacity: 0.5 to the button when it is disabled and maybe remove the hover effect (currently action-dots button has blue background on hover). This way user will not be confused why the dropdown is not appearing when clicking the action-dots button

(currently action-dots button has blue background on hover)

I removed this effect by using the action-dots-enabled class. After changing it as you suggested I just used pointer-events: none; when the button is disabled

I think this logic should be moved to the useProductSupplierActions hook where we initialize the getActions() callback

I think it is unnecessary to return these labels from a hook, they should be defined in places where they are used that is in ProductSupplierListTable components

nitpicky: fix indentations - here, and also lines: 68 (array), 69 (object), 77 (object). Hasn't eslint caught it? 🤔 (I know it's broken but...)

Eslint didn't catch it 👍

this is what I wanted to see 👍

I'm not sure if we want to refetch the data if something goes wrong with the delete. I think it should be placed inside the try. But I do not have a strong opinion on that.
Since we return 204 no matter what, the only exception I might imagine would be if we lost the connection.

doesn't it break anything? It made sense when canEditRequest was the only method there.
I assume it doesn't, because you de-structured the object.

Yeah, it doesn't break anything

nitpicky: reminder - the && should be placed at the beginning of a line:
.
The ESLint didn't shout there probably due to the (.

plus shouldn't the dependency of the useCallback be the currentUser and isAdmin?
The getAppContext seems to run on every refresh, hence there is a low risk that we might not get a fresh user there, especially assuming that you were playing with the permissions in the mean time.

I think we need to create a separate utils method for it that we can call instead of importing all necessary components and every time.
When I said a wrapper I meant something like notification that we already have in the project

notification(NotificationType.ERROR_OUTLINED)({
  message: 'Not found',
  details: errorMessage || errorMessages,
});

So instead of calling

confirmAlert({
  customUI: ({ onClose }) => (
   <CustomConfirmModal
      labels={modalLabels}
      onClose={onClose}
      buttons={deleteConfirmationModalButtons(onClose, productSupplierId)}
    />
    ),
});

we could call

confirm({
  title: "Confirm?",
  message: "Are you sure you want to confirm?"
  buttons:  {
      variant: 'transparent',
      defaultLabel: 'Cancel',
      label: 'react.productSupplier.deleteConfirmation.cancel.label',
      onClick: onClose,
    },
    {
      variant: 'danger',
      defaultLabel: 'Delete',
      label: 'react.productSupplier.deleteConfirmation.delete.label',
      onClick: () => deleteProductSupplier(onClose, productSupplierId),
    },
})

we can also move this import to that util function and not worry about it much
import 'react-confirm-alert/src/react-confirm-alert.css';

We can go one step further and implement a promise based confirm dialog which returns boolean values based on clicked button, I implemented something similar in this PR #4454 (comment)

Let me explain...
There is a javascript confirmation modal build in the browser, to call it we do

const isConfirmed = window.confirm("Are you sure?)

upon calling it stalls the application until the user responds by confirming or canceling the confirmation dialog.
We can implement something similar with Promises.
The code would look something like

const confirm = ({ modalLabels }) => new Promise((resolve) => confirmAlert({
 willUnmount: () => resolve(false),
  customUI: ({ onClose }) => (
   <CustomConfirmModal
      labels={modalLabels}
      onClose={ () => resolve(false)}
      buttons={{
        variant: 'transparent',
        defaultLabel: 'Cancel',
        label: 'react.productSupplier.deleteConfirmation.cancel.label',
        onClick:  () => resolve(false),
      },
      {
        variant: 'danger',
        defaultLabel: 'Delete',
        label: 'react.productSupplier.deleteConfirmation.delete.label',
        onClick: () => resolve(true),
      }}
    />
    ),
}));

and then we use it the following way

const isConfirmed = await confirm({ modalLabels })

You might say that this restricts us to having the confirmationAlert have only two options since it will return either true or false. That is true and I think that's the point, and looking through our project we have been using confirmationAlert only with two buttons.

Additionally I think labels for Yes and Cancel could be default labels and have the ability to change them like

const isConfirmed = await confirm({ modalLabels, yesLabel: "new yes label" , cancelLabel: "new cancel label" })

This is an idea and not a must, but since we are at the point of creating a new confirmation alert component wanted to use this as an opportunity to implement it the bets way possible.
What do you think guys @awalkowiak @alannadolny @kchelstowski ?

Regarding the promised-based, I see the idea, but I do not see a case at the moment, where it could be useful.
Personally I like passing proper onClick actions to the confirmAlert, it's pretty easy to read.
Additionally, I'd say that the current version also "holds" the application until you click a button, because you can't click anything else while having the modal opened.

As for the yesLabel, cancelLabel, we need to be aware that it doesn't always have to be Yes/Cancel. Even now, we have Cancel/Delete, not Cancel/Yes.

it could be confirm and cancel, I just noticed in our project 90% of the time we used Yes or No labels.
also in this case Cancel/Delete is basically cancel/Yes logically speaking. The question states Are you sure you want to delete this Product Source? where delete button corresponds to confirming this actions.
Maybe a better an for it would be confirmLabel instead of yesLabel

one of the usecases for readability for using the promised-based confirmationAlert would be code repetition.
An example is addItems page where we make sure the user wants to continue if the form is invalid

with callbacks

if (invalid) {
  confirmationAler({
   buttons: {
     label: "Yes",
    onClick: () => callMethodA();
    },
   buttons: {
     label: "No",
    }
  })
} else {
callMethodA()
}

with promises

if(invalid) {
  const shoudlContinue = await confirmAler();
  if (!shoudlContinue) {
    return;
  }
}
 callMethodA()

but this is just my preference, I like reading synchounouse code you probably like callbacks 😄
As I stated, this was just a suggestion and a idea submission not a change request for Alan

It actually looks interesting, but I would still need to see it "on a real example", e.g. here, but the question is if we still want to "torture" Alan in this PR.
If everyone gives his opinion, we can make a decision. I'm neither against nor pro implementing this now (in this PR).

Yeah, we were using only two buttons, but I implemented the custom modal (passed as a customUi prop) to have the ability to have more than two buttons (and each of them is customizable), so I am not pro this second change.