Skip to content
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.

Sign up for GitHub

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

Jump to bottom

AA Brick Configuration: show a single error for invalid configuration #8070

Closed
2 tasks
twschiller opened this issue Mar 26, 2024 · 2 comments · Fixed by #8377 or #8423
Closed
2 tasks

AA Brick Configuration: show a single error for invalid configuration #8070

twschiller opened this issue Mar 26, 2024 · 2 comments · Fixed by #8377 or #8423
Assignees
@BLoe
Labels
bug Something isn't working customer Required for a customer projct user experience Improve the user experience (UX)

Comments

@twschiller
Copy link
Contributor

twschiller commented Mar 26, 2024 •
edited

Context

Describe the bug

  • If the integration is in an invalid state (call an endpoint every authenticated user has access to), show a single error instead vs. rendering the configuration values.
    • We could modify RequireIntegrationConfig to take an optional "test" URL for testing the integration configuration works. For AA a good one would be /v1/usermanagement/users/self
    • If the integration is an invalid state because the user was logged into the CR, include a message that they can't be logged into both the Control Room and the API. (I'm not exactly sure how to differentiate this -- I think its error message includes a detail about being logged in in multiple places). Proposed copy: "The Control Room API cannot be called while you are logged into the Control Room web interface."
  • If the integration is in an invalid state, provide a button to retry getting the token again (both AA auth methods fetch a token)

Discussion

To Reproduce

Steps to reproduce the behavior:

  1. Configure the Run Bot brick with username/password auth when you have an active session in Control Room

Related Code

  1. pixiebrix-extension/src/contrib/google/sheets/ui/RequireGoogleSheet.tsx

    Line 47 in b2b87d2

    const RequireGoogleSheet: React.FC<{
@twschiller twschiller added bug Something isn't working specification required customer Required for a customer projct labels Mar 26, 2024
@twschiller twschiller changed the title AA brick configuration field error when using using username/password authentication AA brick configuration field error when using username/password authentication Apr 8, 2024
@twschiller twschiller added user experience Improve the user experience (UX) and removed specification required labels Apr 8, 2024
@twschiller twschiller changed the title AA brick configuration field error when using username/password authentication AA Brick Configuration: show a single error for invalid configuration Apr 8, 2024
@twschiller twschiller mentioned this issue Apr 25, 2024
Closed
@BLoe BLoe mentioned this issue Apr 30, 2024
Merged
6 tasks
BLoe added a commit that referenced this issue May 1, 2024
@BLoe
#8070 Page Editor formik-validation/field-annotation fixes and AA rem…
277eab7 
…ote fetch brick errors (#8377)

* fix page editor formik validation, set formik error state in async select widget, and remove the useFieldAnnotations hook

* create context to pass analysis selector

* extract the field annotation mapping and re-use in schema field

* fix context in tests

* add formik errors to schema field

* fix async select error test

* fix storyshots

* fix snapshot

* fix lint

* auto add strict null checks

* add files to strict null checks

* implement field template local error functionality, fix snapshots

* revert previous, just call formik setError(null) instead

* fix tests

* use constant for default array value

* cleanup

* restore localError functionality, and fix field template constant

* update snapshots

* fix placeholder and test

* add a describe block to explain some tests

* cleanup

* extract default selector

---------

Co-authored-by: Ben Loe <ben@pixiebrix.com>
@BLoe BLoe closed this as completed in #8377 May 1, 2024
@twschiller twschiller reopened this May 1, 2024
@twschiller
Copy link
Contributor Author

twschiller commented May 1, 2024 •
edited

@BLoe @grahamlangford I've reopened because the initial PR was a refactoring and didn't implement the change from this issue

BLoe added a commit that referenced this issue May 3, 2024
@BLoe
#8070 Part 2 - Fix other remote fetch widgets, move setLocalError to …
0ee1b96 
…a Context, and add test coverage (#8399)

* fix page editor formik validation, set formik error state in async select widget, and remove the useFieldAnnotations hook

* create context to pass analysis selector

* extract the field annotation mapping and re-use in schema field

* fix context in tests

* add formik errors to schema field

* fix async select error test

* fix storyshots

* fix snapshot

* fix lint

* auto add strict null checks

* add files to strict null checks

* implement field template local error functionality, fix snapshots

* revert previous, just call formik setError(null) instead

* fix tests

* use constant for default array value

* cleanup

* restore localError functionality, and fix field template constant

* update snapshots

* fix placeholder and test

* add a describe block to explain some tests

* cleanup

* wip

* extract default selector

* refactor other remote select widgets, move field local error to context, and write lots of tests

* add files to null checks

* fix snapshots

---------

Co-authored-by: Ben Loe <ben@pixiebrix.com>
@twschiller
Copy link
Contributor Author

@BLoe as mentioned during round-up, I think you'll want to double-check the "validate token" endpoint works for both OAuth2 and username/password-based tokens: https://www.notion.so/pixiebrix/Weekly-Roundup-for-2024-05-03-2e9e5e20e71a42c18c178a8e11a20110?pvs=4#5ca03dbdcfeb4efab7077271d4327f27

@BLoe BLoe mentioned this issue May 7, 2024
Merged
2 tasks
@BLoe BLoe closed this as completed in #8423 May 8, 2024
BLoe added a commit that referenced this issue May 8, 2024
@BLoe
#8070 Part 3 - Add separate integration config auth check in `Require…
bf1b339 
…IntegrationConfig` (#8423)

* fix page editor formik validation, set formik error state in async select widget, and remove the useFieldAnnotations hook

* create context to pass analysis selector

* extract the field annotation mapping and re-use in schema field

* fix context in tests

* add formik errors to schema field

* fix async select error test

* fix storyshots

* fix snapshot

* fix lint

* auto add strict null checks

* add files to strict null checks

* implement field template local error functionality, fix snapshots

* revert previous, just call formik setError(null) instead

* fix tests

* use constant for default array value

* cleanup

* restore localError functionality, and fix field template constant

* update snapshots

* fix placeholder and test

* add a describe block to explain some tests

* cleanup

* wip

* extract default selector

* refactor other remote select widgets, move field local error to context, and write lots of tests

* add files to null checks

* fix snapshots

* add config validation to RequireIntegrationConfig

* add link to edit config, and fix integrations page

* add retry action

* cleanup

* add some tests

* cleanup, fix tests, update snapshots

* pr feedback

* cleanup test

---------

Co-authored-by: Ben Loe <ben@pixiebrix.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@BLoe BLoe

Labels
bug Something isn't working customer Required for a customer projct user experience Improve the user experience (UX)
Projects
None yet
Milestone
No milestone
2 participants
@twschiller @BLoe