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.

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

View and edit SSO config #4072

Merged
merged 12 commits into from
Aug 12, 2024
Merged

View and edit SSO config #4072

merged 12 commits into from
Aug 12, 2024

Conversation

fiskus
Copy link
Member

@fiskus fiskus commented Aug 8, 2024

  • Check if there is SSO config (without fetching it) and show "individual assignment" icon button
  • Show YAML editor in popup dialog. Editor is heavy, but it is lazy loaded

Screenshot from 2024-08-09 14-15-03
Screenshot from 2024-08-09 14-15-39

  • Changelog entry (skip if change is not significant to end users, e.g. docs only)

Copy link

codecov bot commented Aug 8, 2024

Codecov Report

Attention: Patch coverage is 0% with 89 lines in your changes missing coverage. Please review.

Project coverage is 38.33%. Comparing base (1953dcf) to head (aec4242).

Files Patch % Lines
...g/app/containers/Admin/UsersAndRoles/SsoConfig.tsx 0.00% 65 Missing and 1 partial ⚠️
...talog/app/containers/Admin/UsersAndRoles/Roles.tsx 0.00% 11 Missing ⚠️
catalog/app/utils/GraphQL/Provider.tsx 0.00% 4 Missing ⚠️
catalog/app/components/Lock/Lock.tsx 0.00% 2 Missing ⚠️
.../Admin/UsersAndRoles/gql/HasSsoConfig.generated.ts 0.00% 2 Missing ⚠️
.../Admin/UsersAndRoles/gql/SetSsoConfig.generated.ts 0.00% 2 Missing ⚠️
...ers/Admin/UsersAndRoles/gql/SsoConfig.generated.ts 0.00% 2 Missing ⚠️
Additional details and impacted files
@@                    Coverage Diff                    @@
##           role-mapping-frontend    #4072      +/-   ##
=========================================================
- Coverage                  38.43%   38.33%   -0.10%     
=========================================================
  Files                        717      718       +1     
  Lines                      33060    33045      -15     
  Branches                    4859     4878      +19     
=========================================================
- Hits                       12706    12669      -37     
- Misses                     19207    19228      +21     
- Partials                    1147     1148       +1     
Flag Coverage Δ
api-python 90.75% <ø> (ø)
catalog 11.45% <0.00%> (-0.05%) ⬇️
lambda 89.06% <ø> (+1.09%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@fiskus fiskus marked this pull request as ready for review August 9, 2024 12:19
@fiskus fiskus requested a review from nl0 August 9, 2024 12:22
Copy link
Member

@nl0 nl0 left a comment

Choose a reason for hiding this comment

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

looks ok, tho i wouldn't use a separate partial query.
let's merge this into the upstream PR anyways and finish it off there,
since i feel quite confident we're shipping this one too.


function TextField({ errors, input, meta }: TextFieldProps) {
// TODO: lint yaml
const errorMessage = meta.submitFailed && errors[meta.error]
Copy link
Member

Choose a reason for hiding this comment

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

does this handle submit errors properly? i vaguely remember i had some confusion with form errors vs submit errors -- they were under different fields and i had to check both these fields, but i don't remember which api this was, maybe form-level 🤷

Copy link
Member Author

Choose a reason for hiding this comment

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

This is for field-level errors. And I just don't show the validation error when form hadn't submitted.
This piece of code is the same as in other TextField across app.

On the other hand, form-specific error. There I made a "shortcut" and since this form is one-field, I show the error from API directly, not from final-form.

@fiskus fiskus merged commit 66f7d0a into role-mapping-frontend Aug 12, 2024
36 of 38 checks passed
@fiskus fiskus deleted the sso-config-frontend branch August 12, 2024 11:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants