Page MenuHomePhabricator

Migrate old commit saved queries to new audit status constants
ClosedPublic

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

Details

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
Branch
audit9
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 20776
Build 28250: Run Core Tests
Build 28249: arc lint + arc unit

Event Timeline

Owners added a subscriber: Restricted Owners Package.Sep 10 2018, 7:50 PM
amckinley added inline comments.
resources/sql/autopatches/20180910.audit.01.searches.php
6–13

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?

28–31

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 inline comments.
resources/sql/autopatches/20180910.audit.01.searches.php
6–13

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.

28–31

Yeah, this is mostly just defensive -- "statuses": 1 should be impossible. It also probably doesn't work.

This revision was automatically updated to reflect the committed changes.