chore: Pass errors to widgets #24760

rajatagrawal · 2023-06-22T14:32:53Z

Summary

Widget should have access to evaluation errors because that allows it to make a decision on how to handle errors in property pane and give users a more visual cue to take a corrective action for fixing errors in the property pane.

The current pop up isn't too conspicuous to notice and also doesn't appear when the user deselects the property pane and reselects it.

Why should this be worked on?

This aligns with Widget Development API. Accessing the evaluation errors from evaluations isn't recommended because it is an internal property. An internal property is not advisable to be used given our roadmap for community widgets.
Showing default data instead may not be the right way to handle errors in every use case.
Loom Video describing the requirement : https://www.loom.com/share/04c228b06bb34b97894e345b46cf0abe?sid=f855ac90-74fa-4ee3-b882-ec70b1b2d9bf

rajatagrawal · 2023-06-22T14:33:16Z

/ok-to-test sha=7619207

github-actions · 2023-06-22T14:33:41Z

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/5346851783.
Workflow: Appsmith External Integration Test Workflow.
Commit: 7619207.
PR: 24760.
Perf tests will be available at https://app.appsmith.com/app/performance-infra-dashboard/pr-details-638dd7cd2913ba43778b915e?pr=24760&runId=5346851783_1

github-actions · 2023-06-22T15:29:27Z

Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/5346851783.
Commit: 7619207.
The following are new failures, please fix them before merging the PR:

cypress/e2e/Regression/Apps/CommunityIssues_Spec.ts
cypress/e2e/Regression/Apps/MongoDBShoppingCart_spec.js
cypress/e2e/Regression/Apps/PgAdmin_spec.js
cypress/e2e/Regression/ClientSide/DynamicHeight/DynamicHeight_Visibility_spec.js
cypress/e2e/Regression/ClientSide/DynamicHeight/Form_With_SwitchGroup_spec.js
cypress/e2e/Regression/ClientSide/DynamicHeight/Invisible_Widgets_spec.ts
cypress/e2e/Regression/ClientSide/DynamicHeight/Overlap_Test_spec.ts
cypress/e2e/Regression/ClientSide/Git/GitSync/GitSyncedApps_spec.js
cypress/e2e/Regression/ClientSide/MobileResponsiveTests/AutoFillWidgets_Basic_2_spec.js
cypress/e2e/Regression/ClientSide/OtherUIFeatures/PreviewMode_spec.js
cypress/e2e/Regression/ClientSide/Widgets/Chart/ChartDataPoint_Spec.ts
cypress/e2e/Regression/ClientSide/Widgets/Checkbox/CheckBox_spec.js
cypress/e2e/Regression/ClientSide/Widgets/Checkbox/CheckboxGroup2_spec.js
cypress/e2e/Regression/ClientSide/Widgets/CurrencyInput/CurrencyInputDynamicCurrencyCode_spec.js
cypress/e2e/Regression/ClientSide/Widgets/CurrencyInput/CurrencyInput_spec.js
cypress/e2e/Regression/ClientSide/Widgets/Form/FormWidget_Nested_HasChanges_spec.js
cypress/e2e/Regression/ClientSide/Widgets/Form/FormWidget_spec.js
cypress/e2e/Regression/ClientSide/Widgets/Form/FormWithSwitch_spec.js
cypress/e2e/Regression/ClientSide/Widgets/ListV2/Listv2_BasicChildWidgetInteraction_spec.js
cypress/e2e/Regression/ClientSide/Widgets/Switch/SwitchGroup2_spec.js
cypress/e2e/Regression/ClientSide/Widgets/Switch/Switch_spec.js
cypress/e2e/Regression/ClientSide/Widgets/Tab/Tab_new_scenario_spec.js
cypress/e2e/Regression/ClientSide/Widgets/Tab/Tab_spec.js
cypress/e2e/Regression/ClientSide/Widgets/TableV1/Table_Switch_spec.js
cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2_Column_Order_spec.js
cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2_Switch_spec.js
cypress/e2e/Regression/ServerSide/GenerateCRUD/Mongo_Spec.ts
cypress/e2e/Regression/ServerSide/GenerateCRUD/MySQL1_Spec.ts
cypress/e2e/Regression/ServerSide/GenerateCRUD/MySQL2_Spec.ts
cypress/e2e/Regression/ServerSide/GenerateCRUD/Postgres1_Spec.ts
cypress/e2e/Regression/ServerSide/GenerateCRUD/Postgres2_Spec.ts
cypress/e2e/Regression/ServerSide/QueryPane/Mongo_Spec.ts
cypress/e2e/Regression/ServerSide/QueryPane/S3_1_spec.js
cypress/e2e/Regression/ServerSide/QueryPane/S3_2_spec.js
cypress/e2e/Sanity/Datasources/MsSQL_Basic_Spec.ts

To know the list of identified flaky tests - Refer here

app/client/src/selectors/dataTreeSelectors.ts

app/client/src/widgets/BaseWidget.tsx

rajatagrawal · 2023-06-26T06:49:21Z

/ok-to-test sha=ddcee7d

github-actions · 2023-06-26T06:49:45Z

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/5374720480.
Workflow: Appsmith External Integration Test Workflow.
Commit: ddcee7d.
PR: 24760.
Perf tests will be available at https://app.appsmith.com/app/performance-infra-dashboard/pr-details-638dd7cd2913ba43778b915e?pr=24760&runId=5374720480_1

rajatagrawal · 2023-06-26T07:56:51Z

/ok-to-test sha=c17d840

github-actions · 2023-06-26T07:57:15Z

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/5375314933.
Workflow: Appsmith External Integration Test Workflow.
Commit: c17d840.
PR: 24760.
Perf tests will be available at https://app.appsmith.com/app/performance-infra-dashboard/pr-details-638dd7cd2913ba43778b915e?pr=24760&runId=5375314933_1

rajatagrawal · 2023-06-30T07:43:35Z

/ok-to-test sha=873a843

github-actions · 2023-06-30T07:44:04Z

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/5420532019.
Workflow: Appsmith External Integration Test Workflow.
Commit: 873a843.
PR: 24760.
Perf tests will be available at https://app.appsmith.com/app/performance-infra-dashboard/pr-details-638dd7cd2913ba43778b915e?pr=24760&runId=5420532019_1

github-actions · 2023-06-30T10:14:33Z

Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/5420532019.
Commit: 873a843.
Cypress dashboard: Click here!
The following are new failures, please fix them before merging the PR:

cypress/e2e/Regression/ClientSide/MobileResponsiveTests/ConversionFlow_Corner_Cases_spec.ts
cypress/e2e/Regression/ClientSide/Onboarding/CreateNewApp_spec.js

To know the list of identified flaky tests - Refer here

rajatagrawal · 2023-07-04T08:28:34Z

/ok-to-test sha=f63b833

github-actions · 2023-07-04T08:28:59Z

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/5452516398.
Workflow: Appsmith External Integration Test Workflow.
Commit: f63b833.
PR: 24760.
Perf tests will be available at https://app.appsmith.com/app/performance-infra-dashboard/pr-details-638dd7cd2913ba43778b915e?pr=24760&runId=5452516398_1

rajatagrawal · 2023-07-04T08:44:25Z

/ok-to-test sha=f68f7b6

github-actions · 2023-07-04T08:44:48Z

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/5452665772.
Workflow: Appsmith External Integration Test Workflow.
Commit: f68f7b6.
PR: 24760.
Perf tests will be available at https://app.appsmith.com/app/performance-infra-dashboard/pr-details-638dd7cd2913ba43778b915e?pr=24760&runId=5452665772_1

github-actions · 2023-07-04T09:40:53Z

Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/5452665772.
Commit: f68f7b6.
Cypress dashboard url: Click here!
All cypress tests have passed 🎉🎉🎉

app/client/src/utils/widgetRenderUtils.tsx

riodeuno · 2023-07-04T10:06:25Z

app/client/src/widgets/BaseWidget.tsx

@@ -863,6 +863,12 @@ export const WIDGET_DISPLAY_PROPS = {
  isDisabled: true,
  backgroundColor: true,
 };
+export interface WidgetError extends Error {


There are changes coming to the BaseWidget and I was hoping that we can move these type definitions to a separate file in the widgets directory. Thoughts?

We can have a folder called BaseWidgets and have two files in it, index.ts and constants.ts. index.ts can have the definition of BaseWidget and constants.ts can have the types and interfaces declarations.

@riodeuno
I can make this change if this suggestion seems reasonable.

We are looking at changing what goes into the BaseWidget. The general idea is that we'll have "pluggable" and modular layout mechanisms (Fixed mode, Auto Layout mode and the new Auto Layout mode). This will change how we organise the components in the editor. The mobile pod will share the engg tech spec for your review. In the meantime, I will recommend targeting the fact that we've the types mixed up with the BaseWidget definition, making it difficult to restructure. As far as the way we do it, please do it as you see fit.

@riodeuno

To manage the scope of this github issue, I have created another issue to refactor the organization of BaseWidget definitions here.

We can merge this pull request and address organization changes in the new issue above.

If everything else looks good in this PR, please let me know!

@rajatagrawal Thanks for heeding my suggestions. I've taken a look at the code. Looks good to me.

riodeuno

The code looks fine to me. A few areas have a concerned comment. Please look into them.

rajatagrawal · 2023-07-06T08:12:01Z

/ok-to-test sha=8e854f6

github-actions · 2023-07-06T08:12:26Z

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/5473115047.
Workflow: Appsmith External Integration Test Workflow.
Commit: 8e854f6.
PR: 24760.
Perf tests will be available at https://app.appsmith.com/app/performance-infra-dashboard/pr-details-638dd7cd2913ba43778b915e?pr=24760&runId=5473115047_1

riodeuno

Please update the PR description. This description is going to be useful for the following scenarios:

During git blame, we asynchronously know exactly why these changes were made
During release promotion, the PR description gives the team an idea on how to describe these changes to our users.

Finally, I would also ask that we create an issue to update the widget development API, letting developers know about the errors API now available in widgets.

github-actions · 2023-07-06T09:06:09Z

Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/5473115047.
Commit: 8e854f6.
Cypress dashboard url: Click here!
All cypress tests have passed 🎉🎉🎉

rajatagrawal · 2023-07-06T11:43:32Z

Please update the PR description. This description is going to be useful for the following scenarios:

During git blame, we asynchronously know exactly why these changes were made

During release promotion, the PR description gives the team an idea on how to describe these changes to our users.

Finally, I would also ask that we create an issue to update the widget development API, letting developers know about the errors API now available in widgets.

Issue for adding widgetErrors to Widget Development API is here

Also updated the PR description. QQ : Can the details of the PR be picked from the issue instead ? The detailed summary is described in the issue first and these details are simply duplicated in the PR description.

rajatagrawal requested review from a team and shastryy and removed request for a team June 22, 2023 14:32

github-actions bot added the skip-changelog Adding this label to a PR prevents it from being listed in the changelog label Jun 22, 2023

vsvamsi1 reviewed Jun 22, 2023

View reviewed changes

app/client/src/selectors/dataTreeSelectors.ts Outdated Show resolved Hide resolved

sbalaji1192 reviewed Jun 23, 2023

View reviewed changes

app/client/src/widgets/BaseWidget.tsx Outdated Show resolved Hide resolved

rajatagrawal force-pushed the chore/add-widgeterrorprops-to-widgets branch from ddcee7d to c17d840 Compare June 26, 2023 07:56

rajatagrawal removed the request for review from shastryy June 27, 2023 08:24

rajatagrawal force-pushed the chore/add-widgeterrorprops-to-widgets branch from c17d840 to 873a843 Compare June 30, 2023 07:43

rajatagrawal force-pushed the chore/add-widgeterrorprops-to-widgets branch from 873a843 to f63b833 Compare July 4, 2023 08:26

rajatagrawal requested review from a team, riodeuno and sbalaji1192 and removed request for a team July 4, 2023 08:26

github-actions bot added Widgets Product This label groups issues related to widgets Chart Widget Enhancement New feature or request labels Jul 4, 2023

github-actions bot removed the Enhancement New feature or request label Jul 4, 2023

Pass errors in widget props

f68f7b6

rajatagrawal force-pushed the chore/add-widgeterrorprops-to-widgets branch from f63b833 to f68f7b6 Compare July 4, 2023 08:43

github-actions bot added the Enhancement New feature or request label Jul 4, 2023

riodeuno reviewed Jul 4, 2023

View reviewed changes

app/client/src/utils/widgetRenderUtils.tsx Outdated Show resolved Hide resolved

riodeuno reviewed Jul 4, 2023

View reviewed changes

rajatagrawal requested a review from riodeuno July 5, 2023 15:18

typecast evaluation error as DataTreeError

8e854f6

rajatagrawal force-pushed the chore/add-widgeterrorprops-to-widgets branch from 54c6ace to 8e854f6 Compare July 5, 2023 15:27

riodeuno approved these changes Jul 6, 2023

View reviewed changes

github-actions bot removed the Enhancement New feature or request label Jul 6, 2023

rajatagrawal self-assigned this Jul 6, 2023

github-actions bot added the Enhancement New feature or request label Jul 6, 2023

rajatagrawal added the Test Plan Approved Manual/Cypress tests covers changes made on the PR. Else, add skip-testPlan label if not applicable label Jul 6, 2023

rajatagrawal merged commit 6e9e974 into release Jul 6, 2023

rajatagrawal deleted the chore/add-widgeterrorprops-to-widgets branch July 6, 2023 11:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: Pass errors to widgets #24760

chore: Pass errors to widgets #24760

rajatagrawal commented Jun 22, 2023 •

edited

Loading

rajatagrawal commented Jun 22, 2023

github-actions bot commented Jun 22, 2023

github-actions bot commented Jun 22, 2023

rajatagrawal commented Jun 26, 2023

github-actions bot commented Jun 26, 2023

rajatagrawal commented Jun 26, 2023

github-actions bot commented Jun 26, 2023

rajatagrawal commented Jun 30, 2023

github-actions bot commented Jun 30, 2023

github-actions bot commented Jun 30, 2023

rajatagrawal commented Jul 4, 2023

github-actions bot commented Jul 4, 2023

rajatagrawal commented Jul 4, 2023

github-actions bot commented Jul 4, 2023

github-actions bot commented Jul 4, 2023

riodeuno Jul 4, 2023

rajatagrawal Jul 5, 2023

rajatagrawal Jul 6, 2023

riodeuno Jul 6, 2023

rajatagrawal Jul 6, 2023

riodeuno Jul 6, 2023

riodeuno left a comment

rajatagrawal commented Jul 6, 2023

github-actions bot commented Jul 6, 2023

riodeuno left a comment

github-actions bot commented Jul 6, 2023

rajatagrawal commented Jul 6, 2023

chore: Pass errors to widgets #24760

chore: Pass errors to widgets #24760

Conversation

rajatagrawal commented Jun 22, 2023 • edited Loading

rajatagrawal commented Jun 22, 2023

github-actions bot commented Jun 22, 2023

github-actions bot commented Jun 22, 2023

rajatagrawal commented Jun 26, 2023

github-actions bot commented Jun 26, 2023

rajatagrawal commented Jun 26, 2023

github-actions bot commented Jun 26, 2023

rajatagrawal commented Jun 30, 2023

github-actions bot commented Jun 30, 2023

github-actions bot commented Jun 30, 2023

rajatagrawal commented Jul 4, 2023

github-actions bot commented Jul 4, 2023

rajatagrawal commented Jul 4, 2023

github-actions bot commented Jul 4, 2023

github-actions bot commented Jul 4, 2023

riodeuno Jul 4, 2023

Choose a reason for hiding this comment

rajatagrawal Jul 5, 2023

Choose a reason for hiding this comment

rajatagrawal Jul 6, 2023

Choose a reason for hiding this comment

riodeuno Jul 6, 2023

Choose a reason for hiding this comment

rajatagrawal Jul 6, 2023

Choose a reason for hiding this comment

riodeuno Jul 6, 2023

Choose a reason for hiding this comment

riodeuno left a comment

Choose a reason for hiding this comment

rajatagrawal commented Jul 6, 2023

github-actions bot commented Jul 6, 2023

riodeuno left a comment

Choose a reason for hiding this comment

github-actions bot commented Jul 6, 2023

rajatagrawal commented Jul 6, 2023

rajatagrawal commented Jun 22, 2023 •

edited

Loading