[keyserver] add migration to populate `policy_acknowledgments` table
ClosedPublic
Actions

Authored by kamil on Nov 17 2022, 2:59 AM.

Details

Reviewers

tomek
marcin
atul

Commits

rCOMMb987bdc86942: [keyserver] add migration to populate `policy_acknowledgments` table

Summary

A solution suggested here: D5309#158058. Creates an entry for every already existing user.

Note: there are two users (ashoat and commbot) who will be present in the database on fresh creation and they will not have an entry in the acknowledgment_table but I am not sure if this is needed.

Test Plan

Run migration
this code should add rows to policy_acknowledgments for all users with terms of use policy, current time, and confirmed set to 1.

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

kamil created this revision.Nov 17 2022, 2:59 AM

kamil held this revision as a draft.

Herald added subscribers: • abosh, atul, ashoat. · View Herald TranscriptNov 17 2022, 2:59 AM

kamil published this revision for review.Nov 17 2022, 3:05 AM

kamil edited the summary of this revision. (Show Details)

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

kamil added reviewers: marcin, atul.

kamil added a parent revision: D5308: [keyserver] add code to create `policy_acknowledgments` table on database setup.

kamil mentioned this in D5309: [keyserver] add script to populate `policy_acknowledgments` table.Nov 17 2022, 3:08 AM

Harbormaster completed remote builds in B13510: Diff 18519.Nov 17 2022, 3:10 AM

kamil added a child revision: D5311: [keyserver] add logic to update acknowledgment table.Nov 17 2022, 3:12 AM

Ran successfully for me:

Should be good to be accepted + landed once you've addressed the feedback (added a semicolon to the end of the SQL query).

IMPORTANT: (CC @ginsu) You guys are both working on adding migrations to migration-config so you'll need to coordinate to figure out who is landing what when + rebase as necessary. Will also need to make sure that all migrations have run in your local environment. If both of you have run a separate "migration 10" in your respective local environments your DB may get out of sync.

keyserver/src/database/migration-config.js
101 ↗	(On Diff #18519)	Let's add a semicolon to the end of the query to match the rest of the queries in this file.

This revision now requires changes to proceed.Nov 17 2022, 11:24 AM

ashoat added inline comments.Nov 17 2022, 11:57 AM

keyserver/src/database/migration-config.js
101 ↗	(On Diff #18519)	I think we only need semicolons when `multipleStatements: true`

atul accepted this revision.Nov 17 2022, 12:00 PM

atul added inline comments.

keyserver/src/database/migration-config.js
101 ↗	(On Diff #18519)	Makes sense, I'm just used to idea from whatever previous SQL dialect I used whenever that valid queries should end in semicolon + figured it'd be good to match rest of file... but yeah if it doesn't matter I'll accept this diff to unblock.