- 
                Notifications
    
You must be signed in to change notification settings  - Fork 5.5k
 
Register fields with form to enable complete error checking #14869
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
338e3aa
              6e5e297
              94f75d3
              ab8d310
              44e42b8
              d0c6e7a
              689a497
              a22d1f5
              0387ab4
              927a7d3
              ea8eba0
              55725f6
              3fbd7eb
              198cc38
              6b3d1d4
              2226c51
              File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
| 
          
            
          
           | 
    @@ -2,7 +2,10 @@ | |
| useQuery, type UseQueryOptions, | ||
| } from "@tanstack/react-query"; | ||
| import { useFrontendClient } from "./frontend-client-context"; | ||
| import type { AppRequestResponse } from "@pipedream/sdk"; | ||
| import type { | ||
| AppRequestResponse, AppResponse, ConfigurablePropApp, | ||
| PropValue, | ||
| } from "@pipedream/sdk"; | ||
| 
     | 
||
| /** | ||
| * Get details about an app | ||
| 
        
          
        
         | 
    @@ -23,3 +26,65 @@ | |
| app: query.data?.data, | ||
| }; | ||
| }; | ||
| 
     | 
||
| type AppResponseWithExtractedCustomFields = AppResponse & { | ||
| extracted_custom_fields_names: string[] | ||
| } | ||
| 
     | 
||
| type AppCustomField = { | ||
| name: string | ||
| optional?: boolean | ||
| } | ||
| 
     | 
||
| type OauthAppPropValue = PropValue<"app"> & { | ||
| oauth_access_token?: string | ||
| } | ||
| 
     | 
||
| function getCustomFields(app: AppResponse): AppCustomField[] { | ||
| const isOauth = app.auth_type === "oauth" | ||
| const userDefinedCustomFields = JSON.parse(app.custom_fields_json || "[]") | ||
| 
         There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Handle potential JSON parsing errors Using  Apply this diff to add error handling:  const isOauth = app.auth_type === "oauth"
-let userDefinedCustomFields = JSON.parse(app.custom_fields_json || "[]")
+let userDefinedCustomFields = [];
+try {
+  userDefinedCustomFields = JSON.parse(app.custom_fields_json || "[]");
+} catch (error) {
+  console.error("Failed to parse custom fields JSON:", error);
+}
  | 
||
| if ("extracted_custom_fields_names" in app && app.extracted_custom_fields_names) { | ||
| const extractedCustomFields = ((app as AppResponseWithExtractedCustomFields).extracted_custom_fields_names || []).map( | ||
| (name) => ({ | ||
| name, | ||
| }), | ||
| ) | ||
| userDefinedCustomFields.push(...extractedCustomFields) | ||
| } | ||
| return userDefinedCustomFields.map((cf: AppCustomField) => { | ||
| return { | ||
| ...cf, | ||
| // if oauth, treat all as optional (they are usually needed for getting access token) | ||
| optional: cf.optional || isOauth, | ||
| } | ||
| }) | ||
| } | ||
| 
     | 
||
| export function appPropError(opts: { value: any, app: AppResponse | undefined }): string | undefined { | ||
| const { app, value } = opts | ||
| 
         Check failure on line 64 in packages/connect-react/src/hooks/use-app.tsx 
    
   | 
||
| if (!app) { | ||
| return "app field not registered" | ||
| } | ||
| if (!value) { | ||
| return "no app configured" | ||
| } | ||
| if (typeof value !== "object") { | ||
| return "not an app" | ||
| } | ||
| const _value = value as PropValue<"app"> | ||
| if ("authProvisionId" in _value && !_value.authProvisionId) { | ||
| if (app.auth_type) { | ||
| if (app.auth_type === "oauth" && !(_value as OauthAppPropValue).oauth_access_token) { | ||
| return "missing oauth token" | ||
| } | ||
| if (app.auth_type === "oauth" || app.auth_type === "keys") { | ||
| for (const cf of getCustomFields(app)) { | ||
| if (!cf.optional && !_value[cf.name]) { | ||
| return "missing custom field" | ||
| } | ||
| } | ||
| } | ||
| return "no auth provision configured" | ||
| } | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct the logic for 'disabled' and 'opacity' properties
The current logic may not correctly determine the
disabledstate due to type differences. Also, theopacityshould reflect the disabled state.Apply this diff to fix the logic:
const disabled = propsNeedConfiguring.length > 0 || submitting; return <input type="submit" value={submitting ? "Submitting..." - : "Submit"} {...getProps("controlSubmit", baseStyles(propsNeedConfiguring.length || submitting), props)} disabled={propsNeedConfiguring.length || submitting} />; + : "Submit"} {...getProps("controlSubmit", baseStyles(disabled), props)} disabled={disabled} />;Update the
baseStylesfunction to usedisabledfor opacity: