-
Notifications
You must be signed in to change notification settings - Fork 321
feat: add last login filtering support to Users API #725
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
base: master
Are you sure you want to change the base?
Conversation
cloudinary-pkoniu
left a comment
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.
Current implementation is a breaking change, it cannot be added to a minor release.
types/index.d.ts
Outdated
| function user(userId: string, options?: ProvisioningApiOptions, callback?: ResponseCallback): Promise<any>; | ||
|
|
||
| function users(pending: boolean, userIds?: string[], prefix?: string, subAccountId?: string, options?: ProvisioningApiOptions, callback?: ResponseCallback): Promise<any>; | ||
| function users(pending: boolean, userIds?: string[], prefix?: string, subAccountId?: string, lastLogin?: boolean, fromDate?: Date | string, toDate?: Date | string, options?: ProvisioningApiOptions, callback?: ResponseCallback): Promise<any>; |
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 lastLogin, fromDate and toDate to options object.
Also, extend the typescript definition of the options object, so it includes those fields and its types.
Keep in mind that it should be backwards compatible, so we aim towards extending the contract rather then changing it. Options could look like this:
function users(..., options?: ProvisioningApiOptions | { lastLogin?: boolean; fromDate?: Date | string; toDate?: Date | string })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.
Updated as requested, to ensure backward compatibility. I moved the new parameters into the options object and updated the typescript definition to include those new fields in options.
All checks passed.
…red users function
Brief Summary of Changes
What Does This PR Address?
Are Tests Included?
Reviewer, Please Note: