Page MenuHomePhabricator

Throw when callers pass an invalid constraint to a "*.search" method
ClosedPublic

Authored by epriestley on Sep 7 2016, 2:25 PM.
Tags
None
Referenced Files
F19514989: D16507.diff
Wed, Jan 14, 7:01 AM
F19514233: D16507.id39726.diff
Tue, Jan 13, 11:45 PM
F19508006: D16507.id39726.diff
Sat, Jan 10, 12:38 AM
F19472267: D16507.id.diff
Wed, Jan 7, 3:16 AM
F19302320: D16507.diff
Tue, Dec 23, 3:29 PM
F19234701: D16507.refgetjnana.dev.diff
Sun, Dec 21, 6:09 PM
F19209889: D16507.id39726.diff
Wed, Dec 17, 12:35 AM
F19199502: D16507.id39726.diff
Tue, Dec 16, 2:44 PM
Subscribers
None

Details

Summary

Ref T11593. When you call a *.search method like maniphest.search, we don't currently validate that all the constraints you pass are recognized.

I think there were two very weak arguments for not doing this:

  • It makes compatibility in arc across versions slightly easier: if we add a new constraint, we could add it to arc but also do client-side filtering for a while.
  • Conduit parameter types could, in theory, accept multiple inputs or optional/alias inputs.

These reasons are pretty fluff and T11593 is a concrete issue caused by not validating. Just validate instead.

Test Plan
  • Made a maniphest.search call with a bogus constraint, got an explicit error about the bad constraint.
  • Made a maniphest.search call with a valid constraint ("ids").

Diff Detail

Repository
rP Phabricator
Branch
iconstraint
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 13605
Build 17528: Run Core Tests
Build 17527: arc lint + arc unit

Event Timeline

epriestley retitled this revision from to Throw when callers pass an invalid constraint to a "*.search" method.
epriestley updated this object.
epriestley edited the test plan for this revision. (Show Details)
epriestley added a reviewer: chad.
chad edited edge metadata.
This revision is now accepted and ready to land.Sep 7 2016, 3:51 PM
This revision was automatically updated to reflect the committed changes.