-
Notifications
You must be signed in to change notification settings - Fork 2
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
feat: Allow DID controller switching [DEV-3587] #481
Conversation
lampkin-diet
commented
Jan 24, 2024
•
edited
Loading
edited
- Add an ability to pass not the default keys to sign fr did update and deactivate and resource creation
Task linked: DEV-3587 Implement fix within Credential Service |
function mustIncludeKey(didResponse: ResponseBody, keyRef: string): boolean { | ||
return didResponse.controllerKeys.some((key) => key.kid === keyRef); | ||
} |
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.
The naming convention is ambiguous here.
The expected must<perform-action>
literal indicates the closure throws if the operation fails, which is not the case here.
I appreciate this is a test helper, therefore won't probably confuse end users, hence why marking as non-blocking to promote.
function mustIncludeKeyControllerRef(didResponse: ResponseBody, keyRef: string): boolean { | ||
return didResponse.controllerKeyRefs.some((key) => key === keyRef); | ||
} |
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.
Similar naming convention logic remark as above.
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.
Very good demonstration on TDD defining + addressing interconnected and / or interchangeable identifier update scenarios.
There are relatively minor remarks to resolve, which should be quick to modify, before that's ready to promote.
PS: Cool + exhaustive testing sequence!
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.
Looks well polished.
Approved.
## [2.17.0-develop.1](2.16.1-develop.1...2.17.0-develop.1) (2024-02-14) ### Features * Allow DID controller switching [DEV-3587] ([#481](#481)) ([2ef5b09](2ef5b09))
🎉 This PR is included in version 2.17.0-develop.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
🎉 This PR is included in version 2.17.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |