add api v1 actors gate endpoint #181
Conversation
AlexWheeler
force-pushed
the
api-v1-actors-gate
branch
from
fa7078c
to
d255d4a
Compare
Agreed. We should probably plug that in flipper as well if it does actually allow that. It was not intentional.
Sounds good to me. I'll look this over now quick. |
| @@ -24,6 +24,7 @@ def as_json | |||
| feature_not_found: Error.new(1, "Feature not found.", "", 404), | |||
| group_not_registered: Error.new(2, "Group not registered.", "", 404), | |||
| percentage_invalid: Error.new(3, "Percentage must be a positive number less than or equal to 100.", "", 400), | |||
| flipper_id_invalid: Error.new(4, "Required parameter flipper_id cannot be nil.", "", 400), | |||
There was a problem hiding this comment.
What do you think about "Required parameter flipper_id is missing."? I'm not sure that we need to mention nil.
There was a problem hiding this comment.
yes, I've gone back and forth on this. I was thinking that saying its missing sounds like the key is missing, but definitely see how nil doesn't really translate to JSON. I like your suggestion as it being missing could also be interpreted as the key or value is missing. Will update thanks!
* POST / DELETE to enable disable with 'flipper_id' parameter * adds specs
AlexWheeler
force-pushed
the
api-v1-actors-gate
branch
from
d255d4a
to
f139e5a
Compare
completes #169
Returns error if flipper_id parameter is missing or nil. Flipper allows enabling an actor with a nil flipper_id, and I thought about allowing this and only rendering error on missing flipper_id parameter, but the more I think about it I don't think its ideal for a user to do this. A flipper id should ideally be unique. It opens the door for accidentally enabling a feature for anything with a nil flipper_id and users wanting to do this are better off using the group gate right? Let me know your thoughts
Figured its easier to review small PRs so will update invalid error responses to 422, or can do it in this PR as a separate commit if you'd like