Skip to content

Conversation

@cniackz
Copy link
Collaborator

@cniackz cniackz commented Aug 22, 2023

Objective:

To properly show error in AddAccessRule.tsx

Additional Info:

How and where to test:

Change the code to simulate the error:

  const createProcess = () => {
    api.bucket
      .setAccessRuleWithBucket(bucket, {
        prefix: prefix,
        access: "tester",  // invalid access rule.
      })

Administrator > Buckets > Anonymous > Add Access Rule > Save

Screenshot 2023-08-23 at 5 49 27 PM

How it should looks:

Screenshot 2023-08-23 at 5 52 36 PM

@cniackz cniackz changed the title Fixing the shown errors [WIP] - Fixing the shown errors Aug 22, 2023
@cniackz cniackz self-assigned this Aug 22, 2023
@cniackz cniackz added the bug this needs to be fixed label Aug 22, 2023
@cniackz cniackz force-pushed the display-errors-1 branch 7 times, most recently from 1a9d586 to cecc246 Compare August 22, 2023 20:17
@cniackz cniackz changed the title [WIP] - Fixing the shown errors Fixing the shown errors Aug 22, 2023
Copy link
Collaborator

@cesnietor cesnietor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cniackz I think that similar to what you needed to do for setAccessRuleWithBucket request, the others responses should not be casted. Same comment from here #3008 (comment) applies.

You can test this by triggering an error on the response if possible.

@cesnietor
Copy link
Collaborator

cesnietor commented Aug 23, 2023

What I believe we needed to do for this issue is just to make sure we do the dispatch of the snackbar message which catching the error.

@cniackz cniackz force-pushed the display-errors-1 branch 2 times, most recently from 735ed5e to 0b80d25 Compare August 23, 2023 21:42
@cesnietor
Copy link
Collaborator

cesnietor commented Aug 23, 2023

@cniackz Checking the way you did the find (thanks for adding it to the description), we'd need to do something different cause the issue is that the errors are not being shown, meaning there is no dispatch so try doing a search on catch only potentially (some requests could also not have catch so probably also then) and seeing which ones don't have dispatch(setErrorSnackMessage

@cniackz cniackz force-pushed the display-errors-1 branch 2 times, most recently from 9344054 to 4102e03 Compare August 23, 2023 21:46
@cniackz
Copy link
Collaborator Author

cniackz commented Aug 23, 2023

Let's fix and test one by one to get it right.

@cniackz cniackz requested a review from cesnietor August 23, 2023 21:47
@cniackz cniackz changed the title Fixing the shown errors Fix error in AddAccessRule.tsx Aug 23, 2023
@cniackz cniackz changed the title Fix error in AddAccessRule.tsx To properly show error in AddAccessRule.tsx Aug 23, 2023
@cniackz cniackz merged commit f4a9420 into minio:master Aug 24, 2023
@cniackz cniackz deleted the display-errors-1 branch August 24, 2023 12:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug this needs to be fixed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants