-
Notifications
You must be signed in to change notification settings - Fork 173
feat: Implement migration for deprecated client fields in configuration #1219
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
Conversation
|
| ClientExpressConfiguration, | ||
| ClientOrganizationRequireBehaviorEnum, | ||
| } from 'auth0'; | ||
| import _ from 'lodash'; |
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.
Please import functions instead of _ for lodash.
| const sanitizedClients = this.normalizeClientFields({ | ||
| clients, | ||
| fields: [{ newField: 'cross_origin_authentication', deprecatedField: 'cross_origin_auth' }], | ||
| }); |
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.
move this logic under existing to
// Sanitize client fields
const sanitizeClientFields = (list: Client[]): Client[] => ....
| * and preventing loss of configuration data during schema transitions. | ||
| * @returns Client[] | ||
| */ | ||
| normalizeClientFields = ({ |
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.
There is already a function named sanitizeClientFields . please be specific with the function name
| * @description Maps deprecated client fields to their new counterparts and removes the deprecated field. | ||
| * If a deprecated field exists, its value is always used for the new field, ensuring data migration | ||
| * and preventing loss of configuration data during schema transitions. |
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.
Logic is not correctly handled.
-
If only
cross_origin_authexists, use that value. (with a warn log) -
if both exists
cross_origin_auth: false
cross_origin_authentication: false
Use the value of cross_origin_authentication (with a warn log for cross_origin_auth)
- if both exists
cross_origin_auth: true
cross_origin_authentication: false
Use the value of cross_origin_authentication (with a warn log for cross_origin_auth)
Always give preference to the new key cross_origin_authentication.
| }): Client[] => | ||
| clients.map( | ||
| (client) => | ||
| fields.reduce( | ||
| (acc, { deprecatedField, newField }) => { | ||
| const hasDeprecated = _.has(acc, deprecatedField); | ||
| const hasNew = _.has(acc, newField); | ||
|
|
||
| if (hasDeprecated) { | ||
| // If deprecated exists and new is missing, log a warning and copy the value | ||
| if (!hasNew) { | ||
| log.warn( | ||
| `Client '${client.name}': The '${deprecatedField}' field is deprecated. Migrating value to '${newField}'.` | ||
| ); | ||
| acc[newField] = acc[deprecatedField]; | ||
| } | ||
| // Remove the deprecated field | ||
| return _.omit(acc, deprecatedField); | ||
| } | ||
|
|
||
| return acc; | ||
| }, | ||
| { ...client } as Record<string, unknown> | ||
| ) as Client | ||
| ); |
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.
reduce complexity, read more on link
|
closing in favor of #1223 |
🔧 Changes
This change specifically addresses backward compatibility for client application settings. We've enhanced the tool to reliably handle the old cross_origin_auth field and smoothly migrate its value to the new cross_origin_authentication field.
🔬 Testing
This change is purely a code style/best practice fix within a unit test. It ensures the existing logic for handling deprecated client fields during YAML load remains correct and reliable. No change in core functionality is expected.
📝 Checklist