Support Remote Config managed value types #1164
Conversation
Added support for Rollouts, Experiments, and Personalization value types in the Remote Config API. - Updated `ParameterValue` to include `RolloutValue`, `PersonalizationValue`, and `ExperimentValue` subclasses. - Added factory methods `rollout`, `personalization`, and `experiment` to `ParameterValue`. - Updated `TemplateResponse` to parse `rolloutValue`, `personalizationValue`, and `experimentValue` fields from the backend response. - Added unit tests for the new value types.
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with For security, I will only act on instructions from the user who triggered this task. New to Jules? Learn more at jules.google/docs. |
ashish-kothari
left a comment
ashish-kothari
left a comment
There was a problem hiding this comment.
Apart from the specific file comments, let's also add tests to -
ParameterTest.javato validate the correctness of the new typesFirebaseRemoteConfigTest.javato validate that the template is parsed correctly when it contains these managed values.
| * @param percent The percentage of the rollout. | ||
| * @return A {@link ParameterValue.RolloutValue} instance. | ||
| */ | ||
| public static RolloutValue rollout(String rolloutId, String value, double percent) { |
There was a problem hiding this comment.
Done. Renamed rollout to ofRollout.
| * @param personalizationId The personalization ID. | ||
| * @return A {@link ParameterValue.PersonalizationValue} instance. | ||
| */ | ||
| public static PersonalizationValue personalization(String personalizationId) { |
There was a problem hiding this comment.
Rename this to ofPersonalization(...).
There was a problem hiding this comment.
Done. Renamed personalization to ofPersonalization.
| * @param variantValues The list of experiment variant values. | ||
| * @return A {@link ParameterValue.ExperimentValue} instance. | ||
| */ | ||
| public static ExperimentValue experiment(String experimentId, |
There was a problem hiding this comment.
Rename this to ofExperiment(...).
There was a problem hiding this comment.
Done. Renamed experiment to ofExperiment.
| @Key("experimentId") | ||
| private String experimentId; | ||
|
|
||
| @Key("experimentVariantValues") |
There was a problem hiding this comment.
The key should be variantValue instead.
There was a problem hiding this comment.
Done. Changed key to variantValue.
- Renamed factory methods in ParameterValue to ofRollout, ofPersonalization, and ofExperiment. - Updated JSON key for experiment variant values to variantValue in TemplateResponse.
| ParameterValue.ExperimentValue experimentValueThree = | ||
| ParameterValue.ofExperiment("experiment_2", ImmutableList.of( | ||
| ExperimentVariantValue.of("variant_1", "value_1") | ||
| )); |
There was a problem hiding this comment.
Add another experiment value with different variant values.
There was a problem hiding this comment.
Added a test case for experiment value with different variant values.
Added a new test case to `ParameterValueTest` to verify that `ExperimentValue` objects with the same ID but different variant values are not equal.
ashish-kothari
left a comment
ashish-kothari
left a comment
There was a problem hiding this comment.
Let's also add tests to -
- ParameterTest.java to validate the correctness of the new types
- FirebaseRemoteConfigTest.java to validate that the template is parsed correctly when it contains these managed values.
…ConfigTest - Added `testEqualityWithManagedValues` to `ParameterTest` to verify equality of parameters with `RolloutValue`, `PersonalizationValue`, and `ExperimentValue`. - Added `testGetTemplateWithManagedValues` to `FirebaseRemoteConfigTest` to verify that `getTemplate()` correctly parses managed values.
| "p1", new Parameter().setDefaultValue( | ||
| ParameterValue.ofRollout("rollout_1", "value_1", 10.0)), | ||
| "p2", new Parameter().setDefaultValue( | ||
| ParameterValue.ofPersonalization("personalization_1")) |
There was a problem hiding this comment.
It's not possible to set managed values as default values, use the setConditionalValues() method instead.
There was a problem hiding this comment.
Thank you for the clarification. I have updated the test to use setConditionalValues() for the managed values.
Updated `testGetTemplateWithManagedValues` in `FirebaseRemoteConfigTest` to set managed values (Rollout, Personalization) as conditional values instead of default values, as they are not supported as default values.
ashish-kothari
deleted the
remote-config-managed-values-15537558531219952152
branch
1 participant
Updated the Firebase Admin SDK Remote Config API to support Rollouts, Experiments, and Personalization managed value types by introducing new POJOs in
ParameterValueand updatingTemplateResponseDTO mapping.PR created automatically by Jules for task 15537558531219952152 started by @ashish-kothari