Skip to content

feat: Add ability to create a change request ignoring conflicts #6236

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

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
+27 −3
Draft

feat: Add ability to create a change request ignoring conflicts #6236

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
72512b7
Add ability to create a change request ignoring conflicts
kyle-ssg Nov 3, 2025
587245c
Unused log
kyle-ssg Nov 3, 2025
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
3 changes: 2 additions & 1 deletion frontend/common/stores/feature-list-store.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -549,7 +549,8 @@ const controller = {
if (
!feature_states_to_create.length &&
!feature_states_to_update.length &&
!segment_ids_to_delete_overrides.length
!segment_ids_to_delete_overrides.length &&
!changeRequest.ignore_conflicts
) {
throw new Error('Change request contains no changes')
}
Expand Down
1 change: 1 addition & 0 deletions frontend/common/types/requests.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -620,6 +620,7 @@ export type Req = {
description: string
multivariate_options: MultivariateOption[]
title: string
ignore_conflicts?: boolean
}
getFeatureStates: {
environment?: number
Expand Down
23 changes: 21 additions & 2 deletions frontend/web/components/modals/ChangeRequestModal.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, { useState, useEffect, FC, useMemo } from 'react'
import React, { FC, useEffect, useMemo, useState } from 'react'
import UserSelect from 'components/UserSelect'
import OrganisationProvider from 'common/providers/OrganisationProvider'
import Button from 'components/base/forms/Button'
Expand All @@ -15,13 +15,17 @@ import Utils from 'common/utils/utils'
import { Approval, ChangeRequest, User } from 'common/types/responses'
import { Req } from 'common/types/requests'
import getUserDisplayName from 'common/utils/getUserDisplayName'
import Checkbox from 'components/base/forms/Checkbox'

interface ChangeRequestModalProps {
changeRequest?: ChangeRequest
onSave: (
data: Omit<Req['createChangeRequest'], 'multivariate_options'>,
data: Omit<Req['createChangeRequest'], 'multivariate_options'> & {
ignore_conflicts?: boolean
},
) => void
isScheduledChange?: boolean
showIgnoreConflicts?: boolean
showAssignees?: boolean
}

Expand All @@ -30,6 +34,7 @@ const ChangeRequestModal: FC<ChangeRequestModalProps> = ({
isScheduledChange,
onSave,
showAssignees,
showIgnoreConflicts,
}) => {
const [approvals, setApprovals] = useState<Approval[]>([
...(changeRequest?.approvals ?? []),
Expand All @@ -46,6 +51,7 @@ const ChangeRequestModal: FC<ChangeRequestModalProps> = ({
const [showUsers, setShowUsers] = useState(false)
const [showGroups, setShowGroups] = useState(false)
const [currDate, setCurrDate] = useState(new Date())
const [ignoreConflicts, setIgnoreConflicts] = useState(false)

const { data: groups } = useGetMyGroupsQuery({
orgId: AccountStore.getOrganisation().id,
Expand Down Expand Up @@ -89,6 +95,7 @@ const ChangeRequestModal: FC<ChangeRequestModalProps> = ({
onSave({
approvals,
description,
ignore_conflicts: ignoreConflicts,
live_from: liveFrom || undefined,
title,
})
Expand Down Expand Up @@ -282,6 +289,18 @@ const ChangeRequestModal: FC<ChangeRequestModalProps> = ({
size='-sm'
/>
)}
{!changeRequest && showIgnoreConflicts && (
<InputGroup
title='Ignore Conflicts'
component={
<Checkbox
label='Create this change request even if there is an existing one for the same feature'
Copy link
Contributor

@matthewelwell matthewelwell Nov 3, 2025

Choose a reason for hiding this comment

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

This isn't really related to being able to create the change request, but more about what happens when it goes live at the scheduled date.

Without this boolean enabled, a scheduled change will fail, and send a notification to the author, if another change has been made to the flag between creation of the change request, and the time it is due to go live. With this boolean enabled, it will ignore the conflict and update the flag with the information it had at the point of creation.

Based on this, we should (a) only show this if the change request is scheduled for the future, and (b) update the wording here. Perhaps something like:

"Ignore any conflicting changes when this change goes live. If disabled, and another change is made to this flag before the live date, this change will fail and a notification will be sent to the author."

Copy link
Member Author

@kyle-ssg kyle-ssg Nov 3, 2025

Choose a reason for hiding this comment

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

I could imagine a scenario where people line up these change requests without scheduling and the process is just manual - in a case where they don't know exactly when the time frame is they just know its coming.

For example this could be for maintenance but they do not know exactly when they want to turn the feature back on.

Copy link
Contributor

@matthewelwell matthewelwell Nov 3, 2025

Choose a reason for hiding this comment

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

Sorry, yeah, it looks like the logic also applies there, for CRs that stay open for a while, and other changes happen in between, this would be the case too. In that scenario though, we'd probably want to have this checkbox on the 'publish' action, rather than when you create the change request itself.

The FE technically could handle that itself by checking if there are conflicts on an open CR, showing the checkbox and then sending 2 requests when the user hits publish - one to manually update the CR to set "ignore_conflicts": true, and the second to actually publish the CR.

What we'd probably want to do though is to add this same ignore_conflicts attribute to the payload on the publish endpoint.

Copy link
Member Author

@kyle-ssg kyle-ssg Nov 3, 2025

Choose a reason for hiding this comment

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

Right, so this isn't needed really at all when creating change requests? One thing I noticed was we check for when the change request appears unchanged, it seems this has raised a usecase where submitting an unchanged change request is valid. I think I should just remove this check all-together and move the ignore_conflicts to the publish step?

Copy link
Contributor

@matthewelwell matthewelwell Nov 3, 2025

Choose a reason for hiding this comment

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

Not quite - there are 2 use cases:

  1. Publishing a change request
  2. Creating a scheduled change request

For the scheduled change, we do need this on creation of the change request.

Agreed that we should remove the hard fail on if there are no changes, but we should perhaps just show a warning message instead?

checked={ignoreConflicts}
onChange={setIgnoreConflicts}
/>
}
/>
)}
<FormGroup className='text-right mt-2'>
<Button
id='confirm-cancel-plan'
Expand Down
3 changes: 3 additions & 0 deletions frontend/web/components/modals/CreateFlag.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -931,12 +931,14 @@ const CreateFlag = class extends Component {
? 'Update Change Request'
: 'New Change Request',
<ChangeRequestModal
showIgnoreConflicts={isVersioned}
showAssignees={is4Eyes}
isScheduledChange={schedule}
changeRequest={this.props.changeRequest}
onSave={({
approvals,
description,
ignore_conflicts,
live_from,
title,
}) => {
Expand Down Expand Up @@ -966,6 +968,7 @@ const CreateFlag = class extends Component {
id:
this.props.changeRequest &&
this.props.changeRequest.id,
ignore_conflicts,
live_from,
multivariate_options: this.props
.multivariate_options
Expand Down
Loading