Details

Reviewers

Commits

Summary

The script is for the case where already present policy was updated and we need to force user to acknowledge it again.

Test Plan

Run yarn script dist/scripts/force-policy-acknowledgment.js
Check if in all policy_acknowledgments rows with given policy and confirmation date lower than the date when policy was published confirmed column was flipped to 0.

Diff Detail

Repository

rCOMM Comm

Lint

No Lint Coverage

Unit

No Test Coverage

kamil held this revision as a draft.

kamil edited the test plan for this revision. (Show Details)

kamil added a reviewer: atul.

keyserver/src/scripts/force-policy-acknowledgment.js
9 ↗	(On Diff #17381)	This is needed for case when there is a gap between publishing policy and running script, if within this time range eg. a new user will signs in, we don't want to force him to do it again. If this is too minor case and there is no possibility this can be removed.

kamil published this revision for review.Oct 6 2022, 4:50 AM

tomek edited reviewers, added: marcin; removed: tomek.Oct 6 2022, 5:31 AM

Looks good, but same three comments as in D5309

This revision now requires changes to proceed.Oct 6 2022, 6:15 AM

As in D5309 - shouldn't we iterate?

keyserver/src/scripts/force-policy-acknowledgment.js
11 ↗	(On Diff #17381)	I assume that as in D5309 this one is also intended to be one-time script so I am not going to pursue my idea of iterating over policies array. Nevertheless defining `const policy = policyTypes.tosAndPrivacyPolicy` (specific policy) and using it in function called `forcePolicyAcknowledgment` (suggests policies in general) doesn't seem right to me. We should either call this function `forceTosAndPrivacyPolicyAcknowledgement` or parameterize it with policy.

fix formating, add new line, remove const

keyserver/src/scripts/force-policy-acknowledgment.js
11 ↗	(On Diff #17381)	I assume that as in D5309 this one is also intended to be one-time script so I am not going to pursue my idea of iterating over policies array. Nevertheless defining ` It's not a one-time script, it will be executed every time a policy will be updated and we will need another confirmation from users (I don't think all policies will be updated at once that's why there is no iterating). const policy = policyTypes.tosAndPrivacyPolicy` (specific policy) and using it in function called `forcePolicyAcknowledgment` (suggests policies in general) doesn't seem right to me. We should either call this function `forceTosAndPrivacyPolicyAcknowledgement` or parameterize it with policy. Looking through other scripts in keyserver I think this is the way of parametrization, by setting a const and then running a script.

Please address nits before landing

keyserver/src/scripts/force-policy-acknowledgment.js
13–15	Encourage you to match existing conventions as much as possible. Whenever you're writing something new, try to look for prior examples in the codebase and compare Three nits here: Removed the semicolon, which we only use with `multipleStatements: true` Removed the indentation for `SET` and `WHERE` No line the codebase should exceed an 80 character width. Your third line currently exceeds this

Looks good, should be good to land once feedback is addressed

marcin added a reviewer: tomek.

marcin added inline comments.

keyserver/src/scripts/force-policy-acknowledgment.js
11 ↗	(On Diff #17381)	Looking through other scripts in keyserver I think this is the way of parametrization, by setting a const and then running a script. In such a case I should not block this revision on refactor.