-
Notifications
You must be signed in to change notification settings - Fork 2.2k
fix: device attributes type #14520
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
fix: device attributes type #14520
Conversation
@@ -89,11 +88,14 @@ export type AutoSignInEventData = | |||
| { | |||
event: 'autoSignIn'; | |||
}; | |||
|
|||
type DeviceAttributeKey = 'device_status' | 'device_name' | 'last_ip_used'; |
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.
Thanks a lot @soberm for looking into this so quickly. It looks like this may not be the full list of predefined attributes (based on the existing type). There also seem to be some custom attributes involved. Could you double check this with the reviewers?
export type UserAttributeKey = AuthStandardAttributeKey | CustomAttribute; |
export type AuthStandardAttributeKey = |
export type CustomAttribute = string & NonNullable<unknown>; |
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.
Hi @kturczyski-lz, will double check but the previous type mistakenly used UserAttributes and not DeviceAttributes which are different.
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.
Ahh, yeah, sorry, I actually see only those 3 keys in the result. I assumed the old type wasn’t simply ported to the new structure.
Description of changes
The runtime structure returned by
fetchDevices()
contains device attributes as a plain object with keys likedevice_status
,device_name
, andlast_ip_used
, but the TypeScript definition incorrectly defines attributes asAuthUserAttribute<UserAttributeKey>
(a single object withattributeKey
andvalue
properties). This PR updates the type for device attributes.Issue #, if available
#14517
Description of how you validated changes
Manually verified that the types are correct.
Checklist
yarn test
passesChecklist for repo maintainers
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.