Page MenuHomePhabricator

Migrate all Differential comment text into new storage
ClosedPublic

Authored by epriestley on Feb 11 2014, 6:07 PM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Nov 21, 12:52 AM
Unknown Object (File)
Sat, Nov 16, 11:10 AM
Unknown Object (File)
Tue, Nov 12, 3:51 PM
Unknown Object (File)
Sun, Nov 10, 7:08 PM
Unknown Object (File)
Fri, Nov 8, 2:43 AM
Unknown Object (File)
Oct 18 2024, 12:45 PM
Unknown Object (File)
Oct 18 2024, 11:57 AM
Unknown Object (File)
Oct 17 2024, 8:28 AM
Subscribers

Details

Summary

Ref T2222. Currently, DifferentialComment stores both (a) the text of comments and (b) various other transaction details. This data needs to map to both Transactions and TransactionComments in the long run. This diff separates out all the data which is bound for the TransactionComment table, so that when we migrate DifferentialComment itself it will only need to migrate into the Transactions table. This is a much simpler migration than the inline comment one was, partly because it set up infrastructure and partly because the data is less complex.

Basically, I'm just proxying the read/write for the comment text into the other table. All readers already go through the Query class, and there are only three writers (preview, comment, implicit comment on diff update) which are all highly regular and straightforward to test.

We can also back out of this diff very easily: doing double writes cost only one line of code ($this->content = $content;) so we have proper double writes and a trivial revert path.

Test Plan
  • Without migrating, added comments and saw them show up.
  • Migrated.
  • Saw all the old comments, and no damage to the new ones.
  • Added new comments.
  • Used comment preview.
  • Updated a revision to implicitly create an update comment and verified it looked OK.

Diff Detail

Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

resources/sql/autopatches/20140211.dx.1.nullablechangesetid.sql
2–3

This is just making the column nullable. Previously, it was not, since this is always non-null for inlines and I overlooked it when creating the table.