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

Move app details to main AuthAdminState #309

Merged
merged 2 commits into from
Feb 14, 2024
Merged

Conversation

scotttrinh
Copy link
Contributor

@scotttrinh scotttrinh commented Jan 30, 2024

We use these details in several places around the app now, so we should read and write the new values from the main AuthConfig object.

Note: the old app details in the UIConfig still exist, but we should consider them deprecated and ignore the existing values there.


As far as a deployment plan goes, I'm not sure how to make sure this gets sequenced correctly. It should only be merged once the server change that it depends on is merged, and it should not be backported to 4.x. I mostly did this work so I can actually test the server change, so apologies for the weird sequencing here.

We use these details in several places around the app now, so we should
read and write the new values from the main `AuthConfig` object.

Note: the old app details in the `UIConfig` still exist, but we should
consider them deprecated and ignore the existing values there.
@scotttrinh
Copy link
Contributor Author

See edgedb/edgedb#6754

@jaclarke
Copy link
Member

jaclarke commented Feb 8, 2024

This looks good, except the ui is also used in the cloud, where it could be used with a 4.x version of edgedb, so we need to keep both forms and conditionally show one depending on the version. Also it would probably be better to have a single set of form update/clear buttons like the smtp config, but I can work on these changes.

@jaclarke jaclarke marked this pull request as ready for review February 14, 2024 16:15
@jaclarke jaclarke merged commit d2dd5bf into main Feb 14, 2024
2 checks passed
@jaclarke jaclarke deleted the auth-config-app-details branch February 14, 2024 16:16
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