Skip to content

Commit

Permalink
fix: Removing target options for survey resets its value to remove va…
Browse files Browse the repository at this point in the history 
…lidation error (#27139)
  • Loading branch information
@lucasheriques
lucasheriques authored Jan 9, 2025
1 parent fa86162 commit 4bf728c
Show file tree
Hide file tree
Showing 5 changed files with 41 additions and 26 deletions.
17 changes: 7 additions & 10 deletions cypress/e2e/surveys.cy.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -97,10 +97,8 @@ describe('Surveys', () => {
// select the first property
cy.get('[data-attr="property-select-toggle-0"]').click()
cy.get('[data-attr="prop-filter-person_properties-0"]').click()
cy.get('[data-attr=prop-val]').click({ force: true })
cy.get('[data-attr=prop-val-0]').click({ force: true })

cy.get('[data-attr="rollout-percentage"]').click().type('100')
cy.get('[data-attr=prop-val]').focus().type('true').type('{enter}')
cy.get('[data-attr="rollout-percentage"]').click().clear().type('50')

// save
cy.get('[data-attr="save-survey"]').eq(0).click()
Expand All @@ -109,8 +107,8 @@ describe('Surveys', () => {

// check preview release conditions
cy.contains('Display conditions summary').should('exist')
cy.get('.FeatureConditionCard').should('exist').should('contain.text', 'is_demo equals true')
cy.get('.FeatureConditionCard').should('contain.text', 'Rolled out to 100% of users in this set.')
// cy.get('.FeatureConditionCard').should('exist').should('contain.text', 'is_demo equals true')
cy.get('.FeatureConditionCard').should('contain.text', 'Rolled out to 50% of users in this set.')

// launch survey
cy.get('[data-attr="launch-survey"]').click()
Expand Down Expand Up @@ -200,9 +198,8 @@ describe('Surveys', () => {
cy.contains('Add property targeting').click()
cy.get('[data-attr="property-select-toggle-0"]').click()
cy.get('[data-attr="prop-filter-person_properties-0"]').click()
cy.get('[data-attr=prop-val]').click({ force: true })
cy.get('[data-attr=prop-val-0]').click({ force: true })
cy.get('[data-attr="rollout-percentage"]').click().type('100')
cy.get('[data-attr=prop-val]').focus().type('true').type('{enter}')
cy.get('[data-attr="rollout-percentage"]').click().clear().type('50')

cy.get('[data-attr=save-survey]').first().click()

Expand All @@ -227,7 +224,7 @@ describe('Surveys', () => {
// check if targetting criteria is copied
cy.contains('Display conditions summary').should('exist')
cy.get('.FeatureConditionCard').should('exist').should('contain.text', 'is_demo equals true')
cy.get('.FeatureConditionCard').should('contain.text', 'Rolled out to 100% of users in this set.')
cy.get('.FeatureConditionCard').should('contain.text', 'Rolled out to 50% of users in this set.')

// delete the duplicated survey
cy.get('[data-attr=more-button]').click()
Expand Down
Binary file modified frontend/__snapshots__/scenes-app-surveys--new-survey-targeting-section--light.png
View file
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
24 changes: 17 additions & 7 deletions frontend/src/scenes/feature-flags/FeatureFlagReleaseConditions.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,14 @@ export function FeatureFlagReleaseConditions({
onChange,
hideMatchOptions,
nonEmptyFeatureFlagVariants,
showTrashIconWithOneCondition = false,
removedLastConditionCallback,
}: FeatureFlagReleaseConditionsLogicProps & {
hideMatchOptions?: boolean
isSuper?: boolean
excludeTitle?: boolean
showTrashIconWithOneCondition?: boolean
removedLastConditionCallback?: () => void
}): JSX.Element {
const releaseConditionsLogic = featureFlagReleaseConditionsLogic({
id,
Expand Down Expand Up @@ -137,13 +141,19 @@ export function FeatureFlagReleaseConditions({
noPadding
onClick={() => duplicateConditionSet(index)}
/>
{!isEarlyAccessFeatureCondition(group) && filterGroups.length > 1 && (
<LemonButton
icon={<IconTrash />}
noPadding
onClick={() => removeConditionSet(index)}
/>
)}
{!isEarlyAccessFeatureCondition(group) &&
(filterGroups.length > 1 || showTrashIconWithOneCondition) && (
<LemonButton
icon={<IconTrash />}
noPadding
onClick={() => {
removeConditionSet(index)
if (filterGroups.length === 1) {
removedLastConditionCallback?.()
}
}}
/>
)}
</div>
)}
</div>
Expand Down
19 changes: 13 additions & 6 deletions frontend/src/scenes/surveys/SurveyEdit.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,13 @@ export default function SurveyEdit(): JSX.Element {
setSelectedPageIndex(newIndex)
}

function removeTargetingFlagFilters(): void {
setSurveyValue('targeting_flag_filters', null)
setSurveyValue('targeting_flag', null)
setSurveyValue('remove_targeting_flag', true)
setFlagPropertyErrors(null)
}

return (
<div className="flex flex-row gap-4">
<div className="flex flex-col gap-2 flex-1 SurveyForm">
Expand Down Expand Up @@ -735,7 +742,7 @@ export default function SurveyEdit(): JSX.Element {
groups: [
{
properties: [],
rollout_percentage: undefined,
rollout_percentage: 100,
variant: null,
},
],
Expand All @@ -762,17 +769,17 @@ export default function SurveyEdit(): JSX.Element {
filters
)
}}
showTrashIconWithOneCondition
removedLastConditionCallback={
removeTargetingFlagFilters
}
/>
</div>
<LemonButton
type="secondary"
status="danger"
className="w-max"
onClick={() => {
setSurveyValue('targeting_flag_filters', null)
setSurveyValue('targeting_flag', null)
setSurveyValue('remove_targeting_flag', true)
}}
onClick={removeTargetingFlagFilters}
>
Remove all property targeting
</LemonButton>
Expand Down
7 changes: 4 additions & 3 deletions frontend/src/scenes/surveys/surveyLogic.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -335,7 +335,7 @@ export const surveyLogic = kea<surveyLogicType>([
JSONExtractString(properties, '${getResponseField(questionIndex)}') AS survey_response,
COUNT(survey_response)
FROM events
WHERE event = 'survey sent'
WHERE event = 'survey sent'
AND properties.$survey_id = '${props.id}'
${iterationCondition}
AND timestamp >= '${startDate}'
Expand Down Expand Up @@ -465,7 +465,7 @@ export const surveyLogic = kea<surveyLogicType>([
JSONExtractString(properties, '${getResponseField(questionIndex)}') AS survey_response,
COUNT(survey_response)
FROM events
WHERE event = 'survey sent'
WHERE event = 'survey sent'
AND properties.$survey_id = '${props.id}'
AND timestamp >= '${startDate}'
AND timestamp <= '${endDate}'
Expand Down Expand Up @@ -502,7 +502,7 @@ export const surveyLogic = kea<surveyLogicType>([
const query: HogQLQuery = {
kind: NodeKind.HogQLQuery,
query: `
SELECT
SELECT
count(),
arrayJoin(JSONExtractArrayRaw(properties, '${getResponseField(questionIndex)}')) AS choice
FROM events
Expand Down Expand Up @@ -639,6 +639,7 @@ export const surveyLogic = kea<surveyLogicType>([
iteration_count: NEW_SURVEY.iteration_count,
iteration_frequency_days: NEW_SURVEY.iteration_frequency_days,
})
actions.setFlagPropertyErrors(NEW_SURVEY.targeting_flag_filters)
},
submitSurveyFailure: async () => {
// When errors occur, scroll to the error, but wait for errors to be set in the DOM first
Expand Down

0 comments on commit 4bf728c

Please sign in to comment.