fix #6625 feat(nimbus): Add isRollout to v6 api #7476
Conversation
| @@ -3040,6 +3040,10 @@ | |||
| "type": "string", | |||
| "readOnly": true | |||
| }, | |||
| "isRollout": { | |||
| "type": "string", | |||
There was a problem hiding this comment.
Question: should this be string or boolean? I was going off the example of isEnrollmentPaused right above, which is boolean other places but string here 🤔
There was a problem hiding this comment.
The Experiment JSONSchema has boolean
https://github.com/mozilla/nimbus-shared/blob/main/types/experiments.ts#L50-L57
There was a problem hiding this comment.
Yeah that's a bit wonky. We should look into that separately.
There was a problem hiding this comment.
Approved thnx @eliserichards ! You can probably take those two extra test cases out but up to you.
| def test_serializer_rollouts_true(self): | ||
| experiment = NimbusExperimentFactory.create_with_lifecycle( | ||
| NimbusExperimentFactory.Lifecycles.LAUNCH_APPROVE, | ||
| publish_status=NimbusExperiment.PublishStatus.APPROVED, | ||
| targeting_config_slug=NimbusExperiment.TargetingConfig.NO_TARGETING, | ||
| application=NimbusExperiment.Application.FENIX, | ||
| firefox_min_version=NimbusExperiment.Version.FIREFOX_94, | ||
| is_rollout=True, | ||
| ) | ||
|
|
||
| serializer = NimbusExperimentSerializer(experiment) | ||
| self.assertEqual(serializer.data["isRollout"], True) | ||
| check_schema("experiments/NimbusExperiment", serializer.data) | ||
|
|
||
| def test_serializer_rollouts_false(self): | ||
| experiment = NimbusExperimentFactory.create_with_lifecycle( | ||
| NimbusExperimentFactory.Lifecycles.LAUNCH_APPROVE, | ||
| publish_status=NimbusExperiment.PublishStatus.APPROVED, | ||
| targeting_config_slug=NimbusExperiment.TargetingConfig.NO_TARGETING, | ||
| application=NimbusExperiment.Application.FENIX, | ||
| firefox_min_version=NimbusExperiment.Version.FIREFOX_94, | ||
| is_rollout=False, | ||
| ) | ||
|
|
||
| serializer = NimbusExperimentSerializer(experiment) | ||
| self.assertEqual(serializer.data["isRollout"], False) | ||
| check_schema("experiments/NimbusExperiment", serializer.data) |
There was a problem hiding this comment.
I don't think you need these tests, the field is using entirely built in serializer functionality so just updating the above tests that the field is included should be sufficient.
| @@ -3040,6 +3040,10 @@ | |||
| "type": "string", | |||
| "readOnly": true | |||
| }, | |||
| "isRollout": { | |||
| "type": "string", | |||
There was a problem hiding this comment.
Yeah that's a bit wonky. We should look into that separately.
Because...
isRolloutto the schema, and fix #6625 feat(nimbus): initial support for is_rollout flag in experiment data model #6635 addedisRolloutto the v5 APIThis commit...
isRolloutto the v6 API so that it can be accessed by jetstream/clients#6625