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 decide toggle #1857

Merged
merged 3 commits into from
Oct 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 4 additions & 2 deletions packages/sui-pde/src/adapters/optimizely/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -125,15 +125,17 @@ export default class OptimizelyAdapter {
* @param {object} [params.attributes]
* @returns {object} decision
*/
decide({name, attributes}) {
decide({name, attributes, isEventDisabled}) {
const user = this._optimizely.createUserContext(this._userId, {
...this._applicationAttributes,
...attributes
})

return user.decide(
name,
!this._hasUserConsents ? [optimizelySDK.OptimizelyDecideOption.DISABLE_DECISION_EVENT] : undefined
!this._hasUserConsents || isEventDisabled
? [optimizelySDK.OptimizelyDecideOption.DISABLE_DECISION_EVENT]
: undefined
)
}

Expand Down
8 changes: 4 additions & 4 deletions packages/sui-pde/src/hooks/common/platformStrategies.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ const getServerStrategy = () => ({
getVariation: ({pde, experimentName, attributes, adapterId}) => {
return pde.getVariation({pde, name: experimentName, attributes, adapterId})
},
decide: ({pde, name, attributes, adapterId}) => {
return pde.decide({pde, name, attributes, adapterId})
decide: ({pde, name, attributes, adapterId, isEventDisabled}) => {
return pde.decide({pde, name, attributes, adapterId, isEventDisabled})
},
trackExperiment: () => {},
getForcedValue: ({key, queryString}) => {
Expand All @@ -30,8 +30,8 @@ const getBrowserStrategy = ({customTrackExperimentViewed, cache}) => ({

return variationName
},
decide: ({pde, name, attributes, adapterId}) => {
return pde.decide({pde, name, attributes, adapterId})
decide: ({pde, name, attributes, adapterId, isEventDisabled}) => {
return pde.decide({pde, name, attributes, adapterId, isEventDisabled})
},
trackExperiment: ({variationName, experimentName}) => {
if (customTrackExperimentViewed) {
Expand Down
60 changes: 9 additions & 51 deletions packages/sui-pde/src/hooks/useDecision.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import {useContext, useMemo} from 'react'
import {useMemo} from 'react'

import PdeContext from '../contexts/PdeContext.js'
import {getPlatformStrategy} from './common/platformStrategies.js'
import useDecisionCallback from './useDecisionCallback.js'

/**
* Hook to use a feature test
Expand All @@ -13,56 +12,15 @@ import {getPlatformStrategy} from './common/platformStrategies.js'
* @param {string} param.adapterId Adapter id to be executed
* @return {object}
*/
export default function useDecision(name, {attributes, trackExperimentViewed, queryString, adapterId} = {}) {
const {pde} = useContext(PdeContext)

if (pde === null) {
throw new Error('[sui-pde: useDecision] sui-pde provider is required to work')
}
export default function useDecision(
name,
{attributes, trackExperimentViewed, isEventDisabled, queryString, adapterId} = {}
) {
const {decide} = useDecisionCallback()

const data = useMemo(() => {
try {
const strategy = getPlatformStrategy({
customTrackExperimentViewed: trackExperimentViewed
})

const forced = strategy.getForcedValue({
key: name,
queryString
})

if (forced) {
if (['on', 'off'].includes(forced)) {
return {enabled: forced === 'on', flagKey: name}
}

return {enabled: true, flagKey: name, variationKey: forced}
}

const notificationId = pde.addDecideListener({
onDecide: ({type, decisionInfo: decision}) => {
const {ruleKey, variationKey, decisionEventDispatched} = decision

if (type === 'flag' && decisionEventDispatched) {
strategy.trackExperiment({variationName: variationKey, experimentName: ruleKey})
}
}
})

const data = strategy.decide({
pde,
name,
attributes,
adapterId
})

pde.removeNotificationListener({notificationId})

return data
} catch (error) {
return {enabled: false, flagKey: name}
}
}, [trackExperimentViewed, name, queryString, pde, attributes, adapterId])
return decide(name, {attributes, trackExperimentViewed, isEventDisabled, queryString, adapterId})
}, [name, attributes, trackExperimentViewed, isEventDisabled, queryString, adapterId])

return data
}
66 changes: 66 additions & 0 deletions packages/sui-pde/src/hooks/useDecisionCallback.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
import {useCallback, useContext} from 'react'

import PdeContext from '../contexts/PdeContext.js'
import {getPlatformStrategy} from './common/platformStrategies.js'

/**
* Hook to call decide
* @return {function}
*/
export default function useDecisionCallback() {
Copy link
Member Author

@andresz1 andresz1 Oct 30, 2024

Choose a reason for hiding this comment

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

This allows the decide function to be called programmatically, which is useful for very specific use cases. 99% of cases useDecision will be just fine

const {pde} = useContext(PdeContext)

if (pde === null) {
throw new Error('[sui-pde: useDecision] sui-pde provider is required to work')
}

const decide = useCallback(
(name, {attributes, trackExperimentViewed, isEventDisabled, queryString, adapterId} = {}) => {
try {
const strategy = getPlatformStrategy({
customTrackExperimentViewed: trackExperimentViewed
})

const forced = strategy.getForcedValue({
key: name,
queryString
})

if (forced) {
if (['on', 'off'].includes(forced)) {
return {enabled: forced === 'on', flagKey: name}
}

return {enabled: true, flagKey: name, variationKey: forced}
}

const notificationId = pde.addDecideListener({
onDecide: ({type, decisionInfo: decision}) => {
const {ruleKey, variationKey, decisionEventDispatched} = decision

if (type === 'flag' && decisionEventDispatched) {
strategy.trackExperiment({variationName: variationKey, experimentName: ruleKey})
}
}
})

const data = strategy.decide({
pde,
name,
attributes,
adapterId,
isEventDisabled
})

pde.removeNotificationListener({notificationId})

return data
} catch (error) {
return {enabled: false, flagKey: name}
}
},
[]
)

return {decide}
}
1 change: 1 addition & 0 deletions packages/sui-pde/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ export {default as useFeature} from './hooks/useFeature.js'
export {default as PdeContext} from './contexts/PdeContext.js'
export {default as useExperiment} from './hooks/useExperiment.js'
export {default as useDecision} from './hooks/useDecision.js'
export {default as useDecisionCallback} from './hooks/useDecisionCallback.js'
export {default as Experiment} from './components/experiment.js'
export {default as Feature} from './components/feature.js'
export {default as Decision} from './components/decision.js'
53 changes: 42 additions & 11 deletions packages/sui-pde/test/common/useDecisionSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,19 +28,17 @@ describe('useDecision hook', () => {
describe('when pde context is set', () => {
let wrapper
let decide
const decision = {
variationKey: 'variation',
enabled: true,
variables: {},
ruleKey: 'rule',
flagKey: 'flag',
userContext: {},
reasons: []
}

before(() => {
const decision = {
variationKey: 'variation',
enabled: true,
variables: {},
ruleKey: 'rule',
flagKey: 'flag',
userContext: {},
reasons: []
}
decide = sinon.stub().returns(decision)

const addDecideListener = ({onDecide}) =>
onDecide({type: 'flag', decisionInfo: {...decision, decisionEventDispatched: true}})
const removeNotificationListener = sinon.stub()
Expand All @@ -53,6 +51,10 @@ describe('useDecision hook', () => {
)
})

beforeEach(() => {
decide = sinon.stub().returns(decision)
})

describe.client('and the hook is executed by the browser', () => {
describe('and window.analytics.track exists', () => {
beforeEach(() => {
Expand Down Expand Up @@ -200,6 +202,35 @@ describe('useDecision hook', () => {
sinon.assert.callCount(customTrack, 1)
})
})

describe('and event is disabled', () => {
before(() => {
window.analytics = {
ready: cb => cb(),
track: sinon.spy()
}
sinon.spy(console, 'error')
})

after(() => {
console.error.restore()
})

it('should not send event', () => {
renderHook(
() =>
useDecision('test_experiment_id', {
isEventDisabled: true
}),
{
wrapper
}
)

sinon.assert.callCount(decide, 1)
sinon.assert.calledWith(decide, sinon.match.has('isEventDisabled', true))
})
})
})

describe.server('and the hook is executed by the server', () => {
Expand Down