272 move filepreview permissions

[ekraffmiller]

What this PR does / why we need it:

The original purpose of this PR was to the file permissions hook, which was believed to be causing a timing error in Cypress e2e tests.
After refactoring the code to remove the hook, the error is still occuring! For example in this run:
https://github.com/IQSS/dataverse-frontend/actions/runs/7671731412/job/20910744807

I did some more reading about Cypress handling of click actions, and found this article, which explains that sometimes click() fails because Cypress clicks the button before the event handler is attached:
https://www.cypress.io/blog/2019/01/22/when-can-the-test-click.

This article suggests using cypress-pipe to retry the click until the event handler is attached, but I couldn't get it to work in Electron, and when I tried to switch headless mode to Chrome I ran into another issue: cypress-io/cypress#14175

So I added a cy.wait() after finding the button, and before clicking, and that fixed it.
I added another library, cypress-grep, that allows a flaky test to be run multiple times. Without the cy.wait(), it would fail about 25% of the time, and with cy.wait() it succeeded for up to 50 runs, so I think that fixed it.

Also on this PR another unrelated Cypress failure is happening, I created an issue for it here #291

Which issue(s) this PR closes:

• Closes Move loading of File Permissions out of FilePermissionsProvider #272

Special notes for your reviewer:

Suggestions on how to test this:

Does this PR introduce a user interface change? If mockups are available, please link/include them here:

Is there a release notes update needed for this change?:

Additional documentation:

[coveralls]

coverage: 97.413% (+0.2%) from 97.216%
when pulling 460304c on 272-move-filepreview-permissions
into 4e949d3 on develop.

[MellyGray]

Looks good, this refactor gives me such peace of mind. I think the permissions code is much easier to read now.

Regarding the Cypress timeout error, I believe that in end-to-end tests, it's nearly impossible not to add some wait() here and there, although it's not advisable. I think we'll have to rely on extending the timeout for specific cy.wrap() calls and on using it('test description', { retries: 3 }) in some cases.

I've left a few comments; the main one is that we need to merge the FilePermissions and FileUserPermissions models

[MellyGray]

Can you remove the ^ symbol to avoid unexpected upgrades?

[MellyGray]

I couldn't find where the grep library is being used, is it to use the cy.wait(500) command in the integration test?

[ekraffmiller]

I used it from the command line when I was testing locally. For example,
npx cypress run --spec tests/e2e-integration/e2e/sections/dataset/Dataset.spec.tsx --browser chrome --env grep="loads the restricted files when the user is logged in as owner",burn=10

Even though it's not a part of the automated test, I thought it would be good to have in the project, to make it easier to run tests like this locally, for timing errors. Does that sound ok, or should I keep it part of my local environment?

[MellyGray]

Sounds good! We could document this command in the Dataverse Frontend Guide so that everyone is aware that tests can be run with this command

[ekraffmiller]

Sounds good, in the meantime I will update the current README to document that.

[MellyGray]

Looks good! Just a few remaining methods that need to be removed

[MellyGray]

Sounds good! We could document this command in the Dataverse Frontend Guide so that everyone is aware that tests can be run with this command

[MellyGray]

LGTM!

[GPortas]

@ekraffmiller

Can you resolve merge conflicts?

If I am not wrong, they are related to #285 merge