Migrate Audit inline comments to new storage
ClosedPublic
Actions

Authored by epriestley on Jul 22 2014, 7:40 PM.

Details

Reviewers

joshuaspence
btrahan

Maniphest Tasks

T4896: Move Audit to ApplicationTransactions

Commits

Restricted Diffusion Commit
rPc01aa794c1f2: Migrate Audit inline comments to new storage

Summary

Ref T4896. This is substantially identical to the process which Differential followed, and mostly copied from the original Differential migration and the Differential proxy object.

Basically, we move all the data over but the application can't tell, and the same APIs do reads and writes to the new table.

Test Plan

Browsed UI before migrating, everything looked fine (but no inlines).
Ran migration.
Verified draft and published comments survived migration.
Added a draft.
Previewed draft.
Submitted draft.
Viewed standalone with drafts and published comments.
Sanity checked data in database, didn't see anything unusual.

Diff Detail

Repository

rP Phabricator

Branch

audit3

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 1840
Build 1841: [Placeholder Plan] Wait for 30 Seconds

Event Timeline

epriestley updated this revision to Diff 24077.Jul 22 2014, 7:40 PM

epriestley retitled this revision from to Migrate Audit inline comments to new storage.

epriestley updated this object.

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

epriestley added reviewers: joshuaspence, btrahan.

epriestley added a task: T4896: Move Audit to ApplicationTransactions.

Herald added a subscriber: epriestley. · View Herald TranscriptJul 22 2014, 7:40 PM

Harbormaster completed remote builds in B1840: Diff 24077.Jul 22 2014, 7:41 PM

Map update.

Harbormaster completed remote builds in B1841: Diff 24078.Jul 22 2014, 7:58 PM

joshuaspence added a parent revision: D10017: Add storage for new audit transactions and comments.Jul 22 2014, 10:48 PM

Some initial comments, looks good so far

resources/sql/autopatches/20140722.audit.3.miginlines.php
25	This will get renamed today
26	As above
32	Oh gross, we don't have a `PHIDType` class for this?
33	Will need to be renamed

joshuaspence accepted this revision.Jul 23 2014, 1:35 PM

joshuaspence edited edge metadata.

This revision is now accepted and ready to land.Jul 23 2014, 1:35 PM

epriestley added inline comments.Jul 25 2014, 12:04 AM

resources/sql/autopatches/20140722.audit.3.miginlines.php
32	Yeah, there are a handful of types which still haven't migrated. This sequence of diffs will wipe out ACMT ("Audit Comment"). Phortune has query classes now (at least mostly) and we can wipe out these trivially: const PHID_TYPE_ACNT = 'ACNT'; const PHID_TYPE_PDCT = 'PDCT'; const PHID_TYPE_PRCH = 'PRCH'; const PHID_TYPE_PAYM = 'PAYM'; const PHID_TYPE_CHRG = 'CHRG'; const PHID_TYPE_CART = 'CART'; I think LEGB is Legalpad and trivial to wipe now. Doorkeeper can trivially migrate, I think. That should mostly just leave weird magic.