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

Fix actions loading #710

Merged
merged 4 commits into from
Dec 2, 2022
Merged

Fix actions loading #710

merged 4 commits into from
Dec 2, 2022

Conversation

chrisradek
Copy link
Contributor

@chrisradek chrisradek commented Dec 2, 2022

This PR fixes an issue that occurs when explicitly enabling/disabling action destinations.

A very small handful of action destinations have a different plugin name than their creation name. Amplitude is one of these.
Our current remote loader implementation only takes into account the plugin's name and not the creation name when deciding whether to load the destination.

That means there are 2 scenarios where loading Amplitude would not do the expected thing.

  1. analytics.load(writeKey, { integrations: { All: false, "Actions Amplitude": true }})
    With this scenario, all destinations are disabled by default and only if one is explicitly enabled will it be loaded. In this specific example, the plugin name is Amplitude (Actions) despite the integration (creationName) being Actions Amplitude. This mismatch caused the plugin to fail to load.

  2. analytics.load(writeKey, { integrations: { "Actions Amplitude": false }})
    With this scenario, the destination is explicitly disabled. However without these changes users would have had to specify Amplitude (Actions) instead.

[x] I've included a changeset (psst. run yarn changeset. Read about changesets here).

@changeset-bot
Copy link

changeset-bot bot commented Dec 2, 2022

🦋 Changeset detected

Latest commit: bfa66ff

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@segment/analytics-next Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link
Contributor

@pooyaj pooyaj left a comment

Choose a reason for hiding this comment

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

💯

@chrisradek chrisradek merged commit ef5cd39 into master Dec 2, 2022
@chrisradek chrisradek deleted the fix-actions-loading branch December 2, 2022 23:34
@github-actions github-actions bot mentioned this pull request Dec 2, 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.

2 participants