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

feat: ODP Datafile Parsing #775

Merged

Conversation

opti-jnguyen
Copy link
Contributor

@opti-jnguyen opti-jnguyen commented Aug 5, 2022

Summary

  • Parses new datafile odp values (“publicKey” and “host”) from the “integrations” section. These values are used to send requests to the ODP server.
  • Parses datafile with and without the “integrations” section or empty section as well.

Technical Changes

  • Added new integrations field to the project_config_schema composed of array items which are objects with the keys of key, host, and publicKey.
  • Added new Integrations
  • Added new fields to ProjectConfig interface to prepare catching this new incoming integrations field:
    • integrations
    • integrationKeyMap (Optional)
    • publicKeyForOdp (Optional)
    • hostForOdp (Optional)
    • allSegments
  • Exports new getAudienceSegments function from project_config. This function is used for recursively evaluating through audience conditions qualified matches, returning a concatenated array of all condition values where the condition match is equal to the value qualified .

Test plan

  • Tests cover project config creation with new test datafiles and check for new allSegments, publicKeyForOdp, and hostForOdp variables.
  • Tests check that evaluation of datafiles with and without ODP integrations work.
  • Tests check that evaluation of datafiles with and without segments work.
  • Tests check that getAudienceSegments works as a standalone function.

Issues

  • OASIS-8390

Note: Prettier caused many whitespace changes to be applied. To see only the related changes, click the gear icon and select "Hide whitespace changes" in the diff.

@opti-jnguyen opti-jnguyen marked this pull request as ready for review August 5, 2022 15:28
@opti-jnguyen opti-jnguyen requested a review from a team as a code owner August 5, 2022 15:28
Copy link
Contributor

@jaeopt jaeopt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! A couple of changes suggested.

packages/optimizely-sdk/lib/core/project_config/index.ts Outdated Show resolved Hide resolved
packages/optimizely-sdk/lib/tests/test_data.js Outdated Show resolved Hide resolved
Copy link
Contributor

@zashraf1985 zashraf1985 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks Great! Suggested one change.

packages/optimizely-sdk/lib/core/project_config/index.ts Outdated Show resolved Hide resolved
packages/optimizely-sdk/lib/core/project_config/index.ts Outdated Show resolved Hide resolved
@opti-jnguyen
Copy link
Contributor Author

@jaeopt , @zashraf1985 - updated to resolve your comments above. Let me know if the solution looks good to you or if I missed anything... thanks!

Copy link
Contributor

@jaeopt jaeopt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

@zashraf1985 zashraf1985 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks Great!

@opti-jnguyen opti-jnguyen merged commit 550af26 into zeeshan/ats-audience-targeting Aug 10, 2022
@opti-jnguyen opti-jnguyen deleted the jnguyen/odp-datafile-parsing branch August 10, 2022 20:47
@opti-jnguyen opti-jnguyen changed the title ODP Datafile Parsing feat: ODP Datafile Parsing Aug 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants