feat: ODP REST API for Sending ODP Events #786

mikechu-optimizely · 2022-09-21T13:16:36Z

Summary

Adding module to provide an internal service for sending events to ODP's REST API.

Test plan

Added new unit tests to .spec
All Existing Unit tests pass.
All existing full-stack compatibility tests pass.

Issues

FSSDK-8386 (OASIS-8386 [to pass jira_ticket_reference_check])

coveralls · 2022-09-21T13:19:02Z

Coverage increased (+0.006%) to 93.466% when pulling 6552e7f on mike/odp-rest-api-interface into 267f12b on master.

packages/optimizely-sdk/lib/plugins/odp/graphql_manager.ts

jaeopt

I see missing some error handlings. Also we can consider removing {GraphQLManager, RestApiManager} since most logic is in OdpClient. See my comments for more context.

packages/optimizely-sdk/lib/plugins/odp/graphql_manager.ts

packages/optimizely-sdk/lib/plugins/odp/odp_client.ts

packages/optimizely-sdk/lib/utils/http_request_handler/request_handler_factory.ts

packages/optimizely-sdk/lib/plugins/odp/graphql_manager.ts

packages/optimizely-sdk/lib/plugins/odp/rest_api_manager.ts

packages/optimizely-sdk/lib/plugins/odp/odp_client.ts

packages/optimizely-sdk/lib/plugins/odp/graphql_manager.ts

Co-authored-by: Jae Kim <[email protected]>

This reverts commit f40bd80.

WIP REST API manager test

zashraf1985

Great Work Overall! Requested a few changes.

packages/optimizely-sdk/lib/plugins/odp/graphql_manager.ts

packages/optimizely-sdk/lib/plugins/odp/odp_event.ts

packages/optimizely-sdk/lib/plugins/odp/rest_api_manager.ts

packages/optimizely-sdk/lib/utils/http_request_handler/browser_request_handler.ts

packages/optimizely-sdk/lib/utils/http_request_handler/request_handler_factory.ts

Co-authored-by: Zeeshan Ashraf <[email protected]>

…o mike/odp-rest-api-interface # Conflicts: # packages/optimizely-sdk/lib/plugins/odp/graphql_manager.ts

jaeopt

Looks good! A few more small changes suggested.

packages/optimizely-sdk/lib/plugins/odp/graphql_manager.ts

packages/optimizely-sdk/lib/plugins/odp/odp_event.ts

packages/optimizely-sdk/lib/plugins/odp/rest_api_manager.ts

packages/optimizely-sdk/lib/utils/http_request_handler/request_handler_factory.ts

packages/optimizely-sdk/tests/restApiManager.spec.ts

packages/optimizely-sdk/tests/odpClientSendEvents.spec.ts

zashraf1985

Looks Great!

jaeopt

Changes look good. I found a couple of cases inconsistent.

packages/optimizely-sdk/lib/plugins/odp/graphql_manager.ts

packages/optimizely-sdk/lib/plugins/odp/rest_api_manager.ts

# Conflicts: # packages/optimizely-sdk/lib/utils/enums/index.ts # packages/optimizely-sdk/lib/utils/http_request_handler/request_handler_factory.ts

jaeopt

LGTM

mikechu-optimizely added 20 commits September 2, 2022 15:28

Add ODP event, send event, & refactors

3cb9ec6

WIP testing sendOdpEvents

5d6144d

Unit tests + code ODP Client edits

2890a0d

Refactor apiHost to apiEndpoint

a3463b9

WIP code review changes

9eba320

Separate/refactor send events and query segments unit tests

e34e842

Code review requested changes

5d5f643

Code review resolutions

c18637e

Validation refactoring per code review

fed5e9b

Replace execution context

f40bd80

Ready OdpEvent for EventManager

bbd1685

Remove parameter wraps

ec25064

Update OdpResponseSchema jsdoc

6eb4f18

Correct GraphQL vs REST managers

5134c6e

Update unit tests + refactors

e305858

Bug fix GraphQL-ifying query parameters

1a44aa7

Change return on REST API Manager sendEvents()

0ad9447

WIP REST API Manager's test skeleton

e9b2f38

Refactor GQLMgr's name

d5e5ec1

Unit tests for REST API Manager

b480af4

mikechu-optimizely added the feature label Sep 21, 2022

mikechu-optimizely requested a review from a team as a code owner September 21, 2022 13:16

optimizely-codereview-bot assigned mikechu-optimizely Sep 21, 2022

mikechu-optimizely requested review from zashraf1985 and jaeopt September 21, 2022 13:17

Fix name of REST API class

d170d3d

mikechu-optimizely removed the request for review from a team September 21, 2022 17:39

mikechu-optimizely commented Sep 21, 2022

View reviewed changes

packages/optimizely-sdk/lib/plugins/odp/graphql_manager.ts Outdated Show resolved Hide resolved

jaeopt requested changes Sep 21, 2022

View reviewed changes

mikechu-optimizely and others added 2 commits September 22, 2022 08:56

Update packages/optimizely-sdk/lib/plugins/odp/odp_client.ts

182bf29

Co-authored-by: Jae Kim <[email protected]>

Update packages/optimizely-sdk/lib/plugins/odp/odp_client.ts

97b9f05

Co-authored-by: Jae Kim <[email protected]>

mikechu-optimizely mentioned this pull request Sep 22, 2022

bug: Remove execution context determination #787

Merged

mikechu-optimizely added 4 commits September 22, 2022 09:41

Revert "Replace execution context"

768aea1

This reverts commit f40bd80.

Apply only needed ODP_USER_KEY & ODP_CONFIG_STATE

afae4ce

Code review changes

33114a0

Moved OdpClient logic up to GraphQLManager and...

829c6de

WIP REST API manager test

zashraf1985 suggested changes Sep 22, 2022

View reviewed changes

mikechu-optimizely and others added 4 commits September 23, 2022 10:31

Update packages/optimizely-sdk/lib/plugins/odp/graphql_manager.ts

a9273fa

Co-authored-by: Zeeshan Ashraf <[email protected]>

More code review changes

21a969c

Merge remote-tracking branch 'origin/mike/odp-rest-api-interface' int…

9b3a620

…o mike/odp-rest-api-interface # Conflicts: # packages/optimizely-sdk/lib/plugins/odp/graphql_manager.ts

Fixes after forgetting to pull first

192b90b

mikechu-optimizely requested a review from jaeopt September 23, 2022 17:18

jaeopt requested changes Sep 23, 2022

View reviewed changes

mikechu-optimizely added 3 commits September 23, 2022 14:14

Merge branch 'master' into mike/odp-rest-api-interface

52f3bd9

Refactor constructors

220bed5

More code review changes

7bfd1ca

mikechu-optimizely requested review from jaeopt and zashraf1985 September 26, 2022 13:48

Formatting & whitespace edits

be1767d

zashraf1985 approved these changes Sep 26, 2022

View reviewed changes

jaeopt requested changes Sep 26, 2022

View reviewed changes

packages/optimizely-sdk/lib/plugins/odp/graphql_manager.ts Show resolved Hide resolved

packages/optimizely-sdk/lib/plugins/odp/rest_api_manager.ts Outdated Show resolved Hide resolved

mikechu-optimizely added 3 commits September 26, 2022 13:17

Merge branch 'master' into mike/odp-rest-api-interface

740ae16

# Conflicts: # packages/optimizely-sdk/lib/utils/enums/index.ts # packages/optimizely-sdk/lib/utils/http_request_handler/request_handler_factory.ts

Final clean up of execution context

ef9df47

Code review changes

6552e7f

mikechu-optimizely requested a review from jaeopt September 26, 2022 17:27

jaeopt approved these changes Sep 26, 2022

View reviewed changes

mikechu-optimizely merged commit f6c2232 into master Sep 26, 2022

mikechu-optimizely deleted the mike/odp-rest-api-interface branch September 26, 2022 17:39

feat: ODP REST API for Sending ODP Events #786

feat: ODP REST API for Sending ODP Events #786

Uh oh!

Conversation

mikechu-optimizely commented Sep 21, 2022

Summary

Test plan

Issues

Uh oh!

coveralls commented Sep 21, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

jaeopt left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

zashraf1985 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jaeopt left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

zashraf1985 left a comment

Choose a reason for hiding this comment

Uh oh!

jaeopt left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

jaeopt left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

coveralls commented Sep 21, 2022 •

edited

Loading