Migrate old commit saved queries to new audit status constants
ClosedPublic
Actions

Authored by epriestley on Sep 10 2018, 7:50 PM.

Details

Reviewers

amckinley

Maniphest Tasks

T13197: 2018 Week 37 Bonus Content

Commits

rP09703938fb44: Migrate old commit saved queries to new audit status constants

Summary

Depends on D19651. Ref T13197. The application now accepts either numeric or string values. However, for consistency and to reduce surprise in the future, migrate existing saved queries to use string values.

Test Plan

Saved some queries on master with numeric constants, ran the migration, saw string constants in the database and equivalent behavior in the UI.

Diff Detail

Repository

rP Phabricator

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

epriestley created this revision.Sep 10 2018, 7:50 PM

Owners added a subscriber: Restricted Owners Package.Sep 10 2018, 7:50 PM

Harbormaster completed remote builds in B20776: Diff 46966.Sep 10 2018, 7:52 PM

epriestley requested review of this revision.Sep 10 2018, 7:52 PM

amckinley accepted this revision.Sep 10 2018, 9:26 PM

amckinley added inline comments.

resources/sql/autopatches/20180910.audit.01.searches.php
7–14	I sanity-checked this against the code and it's correct, but isn't there an easy way to pull this array out of `PhabricatorAuditCommitStatusConstants`?
29–32	So `"statuses": 1` as a constraint doesn't work? Or just malformed because `diffusion.commit.search` and `audit.query` are both expecting `statuses` to be `list<string>`, and this is just very defensive programming?

This revision is now accepted and ready to land.Sep 10 2018, 9:26 PM

epriestley added a child revision: D19655: Migrate remaining Audit database status constants.Sep 11 2018, 2:05 PM

epriestley marked 2 inline comments as done.Sep 11 2018, 2:28 PM

epriestley added inline comments.

resources/sql/autopatches/20180910.audit.01.searches.php
7–14	I'm hard-coding the list in this migration (and two later migrations) so that I can remove the constants from `PhabricatorAuditCommitStatusConstants` without retaining any confusion/ambiguity going forward.
29–32	Yeah, this is mostly just defensive -- `"statuses": 1` should be impossible. It also probably doesn't work.

Closed by commit rP09703938fb44: Migrate old commit saved queries to new audit status constants (authored by epriestley). · Explain WhySep 12 2018, 7:21 PM

This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Path

Size

resources/

sql/

autopatches/

20180910.audit.01.searches.php

54 lines

Diff 46992

View Options

resources/sql/autopatches/20180910.audit.01.searches.php

Migrate old commit saved queries to new audit status constantsClosedPublicActions