Skip to content

Commit

Permalink
fix: RBAC errors not showing toasts (#36244)
Browse files Browse the repository at this point in the history
## Description

Updates the error handling for Access Control scenarios to show toasts
when correct permissions are not present


Fixes #36229

## Automation

/ok-to-test tags="@tag.AccessControl"

### 🔍 Cypress test results
<!-- This is an auto-generated comment: Cypress test results  -->
> [!CAUTION]
> 🔴 🔴 🔴 Some tests have failed.
> Workflow run:
<https://github.com/appsmithorg/appsmith/actions/runs/10807287223>
> Commit: 08edaee
> <a
href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=10807287223&attempt=2&selectiontype=test&testsstatus=failed&specsstatus=fail"
target="_blank">Cypress dashboard</a>.
> Tags: @tag.AccessControl
> Spec: 
> The following are new failures, please fix them before merging the PR:
<ol>
> <li>cypress/e2e/Regression/ServerSide/GenerateCRUD/MySQL2_Spec.ts</ol>
> <a
href="https://internal.appsmith.com/app/cypress-dashboard/identified-flaky-tests-65890b3c81d7400d08fa9ee3?branch=master"
target="_blank">List of identified flaky tests</a>.
> <hr>Wed, 11 Sep 2024 08:28:37 UTC
<!-- end of auto-generated comment: Cypress test results  -->


## Communication
Should the DevRel and Marketing teams inform users about this change?
- [ ] Yes
- [ ] No


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

- **New Features**
- Enhanced error handling with structured Redux actions for better state
management.
	- Improved clarity in error message construction based on error types.
- **Bug Fixes**
- Refined logic for displaying toast notifications based on the `show`
parameter.
- **Documentation**
	- Updated comments for better understanding of error handling logic.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
  • Loading branch information
hetunandu committed Sep 11, 2024
1 parent bc59bd1 commit 89154c5
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 12 deletions.
10 changes: 8 additions & 2 deletions app/client/src/sagas/ActionSagas.ts
Original file line number Diff line number Diff line change
Expand Up @@ -795,13 +795,19 @@ function* copyActionSaga(

// @ts-expect-error: type mismatch Action vs ActionCreateUpdateResponse
yield put(copyActionSuccess(payload));
} catch (e) {
} catch (e: unknown) {
const actionName = actionObject ? actionObject.name : "";
const errorMessage =
e instanceof Error
? e.message
: createMessage(ERROR_ACTION_COPY_FAIL, actionName);
yield put(
copyActionError({
...action.payload,
show: true,
error: { message: createMessage(ERROR_ACTION_COPY_FAIL, actionName) },
error: {
message: errorMessage,
},
}),
);
}
Expand Down
27 changes: 17 additions & 10 deletions app/client/src/sagas/ErrorSagas.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,16 @@ export function* validateResponse(
}

if (!response.responseMeta && response.status) {
throw Error(getErrorMessage(response.status, response.resourceType));
yield put({
type: ReduxActionErrorTypes.API_ERROR,
payload: {
error: new Error(
getErrorMessage(response.status, response.resourceType),
),
logToSentry,
show,
},
});
}

if (response.responseMeta.success) {
Expand Down Expand Up @@ -218,22 +227,20 @@ export interface ErrorActionPayload {
export function* errorSaga(errorAction: ReduxAction<ErrorActionPayload>) {
const effects = [ErrorEffectTypes.LOG_TO_CONSOLE];
const { payload, type } = errorAction;
const {
error,
logToDebugger,
logToSentry,
show = true,
sourceEntity,
} = payload || {};
const { error, logToDebugger, logToSentry, show, sourceEntity } =
payload || {};
const appMode: APP_MODE = yield select(getAppMode);

// "show" means show a toast. We check if the error has been asked to not been shown
// By making the default behaviour "true" we are ensuring undefined actions still pass through this check
if (show) {
// By checking undefined, undecided actions still pass through this check
if (show === undefined) {
// We want to show toasts for certain actions only so we avoid issues or if it is outside edit mode
if (shouldShowToast(type) || appMode !== APP_MODE.EDIT) {
effects.push(ErrorEffectTypes.SHOW_ALERT);
}
// If true is passed, show the error no matter what
} else if (show) {
effects.push(ErrorEffectTypes.SHOW_ALERT);
}

if (logToDebugger) {
Expand Down

0 comments on commit 89154c5

Please sign in to comment.