-
Notifications
You must be signed in to change notification settings - Fork 210
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(scripts): improve inactive account uids script #17951
Conversation
@@ -161,19 +159,25 @@ const init = async () => { | |||
]) | |||
.as('securityEventUids'); | |||
|
|||
const accountCustomerUids = AccountCustomers.query() | |||
.distinct('uid') |
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.
Are there non-distinct uids?
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.
No, I just got copy-paste happy. Thank you for catching that.
@@ -231,6 +230,8 @@ const init = async () => { | |||
return 0; | |||
} | |||
|
|||
const fd = fs.openSync(filepath, 'a'); |
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.
node:fs/promises
supports async/await for file io, not that it really matters here.
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.
LGTM
Because: - there were a few small issues with the inactive account uids script This commit: - stops opening an output file in dry run mode - fixes an import so that the tsc'd js won't error - eliminates the accounts with Stripe subscriptions in the initial query
0b70ccc
to
914222a
Compare
Because:
This commit: