Skip to content

refactor: Clean up the Optimizely integration #481

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.

Sign up for GitHub

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

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
+834 −1,252
Open

refactor: Clean up the Optimizely integration #481

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
e1fa89f
refactor: Clean up the Optimizely integration
nchilada Jun 12, 2020
8e556d5
Way more changes
nchilada Jun 13, 2020
e5475be
Finish fixing and refactoring tests
nchilada Jun 13, 2020
099f98b
yarn add lodash --dev
nchilada Jun 17, 2020
ed636c0
Update HISTORY.md
nchilada Jun 18, 2020
d2211d7
Fix lint
nchilada Aug 13, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions integrations/optimizely/HISTORY.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,13 @@

4.0.0 / 2020-06-12
==================

* Don't attempt to interface with Optimizely Classic Web, since Classic is finally dead
* Prepare to support Optimizely Edge, an alternative to Optimizely Web
* Drop all references to customCampaignProperties. It seems to have been documented [here](https://segment.com/docs/connections/destinations/catalog/optimizely-web/#settings) but it couldn't possibly have worked.
Copy link
Author

@nchilada nchilada Jun 18, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Beyond this update to the integration code, Custom Campaign Properties and Custom Experiment Properties should also be removed from the docs and from the Segment app. (I didn't highlight Custom Experiment Properties in HISTORY.md since it was specific to the Optimizely Classic Web product, whose code is being removed entirely.)

Full explanation of why this functionality is surely broken:

  • The docs indicate that Custom Experiment Properties and Custom Campaign Properties could be used to capture experiment- and campaign-specific properties like experiment_color and campaign_name in Classic Web and X Web respectively. This makes sense, kinda.
  • Unfortunately, the integration code indicates that for Classic Web, the Custom Experiment Properties are being read off of the top-level window.optimizely.data object, rather than off of an experiment object
  • And for X Web, the Custom Campaign Properties are being read off of a non-existent window.optimizely.newMockData property. This property is mocked in index.test.js but does not appear in actual X Web snippets.

Copy link

@tysonmote tysonmote Aug 10, 2020

Choose a reason for hiding this comment

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

Thanks for the heads-up! I'll work on getting the documentation updated.

* Generally refactor the code and tests.
* If there is an effective referrer for the current page load, include it when tracking future campaigns and not just when tracking current campaigns.

3.5.0 / 2019-12-28
==================

Expand Down
Loading