Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle both V1 and V2 of Cirrus' data structure #5508

Merged
merged 4 commits into from
Jan 17, 2025
Merged

Handle both V1 and V2 of Cirrus' data structure #5508

merged 4 commits into from
Jan 17, 2025

Conversation

Vinnl
Copy link
Collaborator

@Vinnl Vinnl commented Jan 16, 2025

getExperiments pretended that it always returned experiment data in the Cirrus V2 format, but in reality, it returned it in V1 format (by taking the Features property from the V2 format if that's what it got). The reason TypeScript didn't warn about this, is because response.json() resolves to an any, and thus we could just assert that to be the expected type even when it wasn't.

In this PR, I've updated it to either return the V2 format directly, or, if the CirrusV2 flag is disabled, return the V1 object wrapped in { Features: <the V1 object> }. And I created a new type ExperimentData_V2_Or_V2LikeV1 that reflects that, i.e. that is like the V2 format, expect that the Enrollments property can be undefined.

Once we remove the CirrusV2 flag, we can look at where that type is used and update it to just ExperimentData.

How to test

Well... I think we can only test this on stage, because I don't think we can run Cirrus locally. But we'll have to make sure to test both with the flag on and off :)

Checklist (Definition of Done)

  • Localization strings (if needed) have been added.
  • Commits in this PR are minimal and have descriptive commit messages.
  • I've added or updated the relevant sections in readme and/or code comments
  • I've added a unit test to test for potential regressions of this bug.
  • If this PR implements a feature flag or experimentation, the Ship Behind Feature Flag status in Jira has been set
  • Product Owner accepted the User Story (demo of functionality completed) or waived the privilege.
  • All acceptance criteria are met.
  • Jira ticket has been updated (if needed) to match changes made during the development process.
  • Jira ticket has been updated (if needed) with suggestions for QA when this PR is deployed to stage.

@Vinnl Vinnl added the Review: XS Code review time: up to 30min label Jan 16, 2025
@Vinnl Vinnl requested a review from rhelmer January 16, 2025 12:32
@Vinnl Vinnl self-assigned this Jan 16, 2025
Copy link

Base automatically changed from undo-reverts to main January 17, 2025 20:26
@rhelmer rhelmer merged commit bc950fc into main Jan 17, 2025
16 checks passed
@rhelmer rhelmer deleted the cirrus-types branch January 17, 2025 21:00
Copy link

Cleanup completed - database 'blurts-server-pr-5508' destroyed, cloud run service 'blurts-server-pr-5508' destroyed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Review: XS Code review time: up to 30min
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants