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: Move versioned feature history into feature details modal #4499

Merged
merged 10 commits into from
Sep 10, 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
7 changes: 4 additions & 3 deletions frontend/e2e/helpers.cafe.ts
Original file line number Diff line number Diff line change
Expand Up @@ -264,9 +264,8 @@ export const logout = async (t) => {
}

export const goToFeatureVersions = async (featureIndex:number) =>{
await gotoFeatures()
await click(byId(`feature-action-${featureIndex}`))
await click(byId(`feature-history-${featureIndex}`))
await gotoFeature(featureIndex)
await click(byId('change-history'))
}

export const compareVersion = async (
Expand Down Expand Up @@ -294,10 +293,12 @@ export const compareVersion = async (
if(newValue) {
await assertTextContent(byId(`old-value`), `${oldValue}`)
}
await closeModal()
}
export const assertNumberOfVersions = async (index:number, versions:number) =>{
await goToFeatureVersions(index)
await waitForElementVisible(byId(`history-item-${versions-2}-compare`))
await closeModal()
}

export const createRemoteConfig = async (
Expand Down
Original file line number Diff line number Diff line change
@@ -1,50 +1,33 @@
import React, { FC, useState } from 'react'
import FlagSelect from 'components/FlagSelect'
import ConfigProvider from 'common/providers/ConfigProvider'
import { RouterChildContext } from 'react-router'
import Utils from 'common/utils/utils'
import ProjectStore from 'common/stores/project-store'
import { useGetFeatureVersionsQuery } from 'common/services/useFeatureVersion'
import { useGetUsersQuery } from 'common/services/useUser'
import AccountStore from 'common/stores/account-store'
import PanelSearch from 'components/PanelSearch'
import {
Environment,
FeatureVersion as TFeatureVersion,
} from 'common/types/responses'
import PageTitle from 'components/PageTitle'
import Button from 'components/base/forms/Button'
import FeatureVersion from 'components/FeatureVersion'
import InlineModal from 'components/InlineModal'
import TableFilterItem from 'components/tables/TableFilterItem'
import { FeatureVersion as TFeatureVersion } from 'common/types/responses'
import Button from './base/forms/Button'
import FeatureVersion from './FeatureVersion'
import InlineModal from './InlineModal'
import TableFilterItem from './tables/TableFilterItem'
import moment from 'moment'
import { Link } from 'react-router-dom'
import DateList from 'components/DateList'
import DateList from './DateList'
import PlanBasedBanner from './PlanBasedAccess'
import classNames from 'classnames'
import PlanBasedBanner from 'components/PlanBasedAccess'

const widths = [250, 150]
type FeatureHistoryPageType = {
router: RouterChildContext['router']

match: {
params: {
environmentId: string
projectId: string
}
}
environmentId: string
environmentApiKey: string
projectId: string
feature: number
}

const FeatureHistoryPage: FC<FeatureHistoryPageType> = ({ match, router }) => {
const feature = Utils.fromParam(router.route.location.search)?.feature
const FeatureHistory: FC<FeatureHistoryPageType> = ({
environmentApiKey,
environmentId,
feature,
projectId,
}) => {
const [open, setOpen] = useState(false)

const env: Environment | undefined = ProjectStore.getEnvironment(
match.params.environmentId,
) as any
// @ts-ignore
const environmentId = `${env?.id}`
const environmentApiKey = `${env?.api_key}`
const { data: users } = useGetUsersQuery({
organisationId: AccountStore.getOrganisation().id,
})
Expand All @@ -56,57 +39,37 @@ const FeatureHistoryPage: FC<FeatureHistoryPageType> = ({ match, router }) => {
is_live: true,
page,
},
{ skip: !env || !feature },
{ skip: !environmentId || !feature },
)
const [selected, setSelected] = useState<TFeatureVersion | null>(null)
const live = data?.results?.[0]
const [compareToLive, setCompareToLive] = useState(false)
const [diff, setDiff] = useState<null | string>(null)
const versionLimit = 3
return (
<div className='container app-container'>
<PageTitle title={'History'}>
<div>
View and rollback history of feature values, multivariate values and
segment overrides.
</div>
</PageTitle>
<div className='row'>
<div className='col-md-4'>
<div className='flex-row'>
<label className='mb-0'>Feature</label>
<div className='flex-fill ml-2'>
<FlagSelect
placeholder='Select a Feature...'
projectId={match.params.projectId}
onChange={(flagId: string) => {
router.history.replace(
`${document.location.pathname}?feature=${flagId}`,
)
}}
value={feature ? parseInt(feature) : null}
/>
</div>
</div>
</div>
<div>
<h5>Change History</h5>
<div>
View and rollback history of feature values, multivariate values and
segment overrides.
</div>
<div className='mt-4'>
{!!versionLimit && (
<PlanBasedBanner
className='mb-4'
force
feature={'VERSIONING'}
theme={'page'}
/>
)}
{/*{!!versionLimit && (*/}
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any reason to keep this commented?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, because we're adjusting the version limit to be time based and I want to keep this here

Copy link
Member Author

@kyle-ssg kyle-ssg Aug 21, 2024

Choose a reason for hiding this comment

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

It'll be updated next week

edit: #4547

{/* <PlanBasedBanner*/}
{/* className='mb-4'*/}
{/* force*/}
{/* feature={'VERSIONING'}*/}
{/* theme={'page'}*/}
{/* />*/}
{/*)}*/}
<DateList<TFeatureVersion>
items={data}
isLoading={isLoading}
nextPage={() => setPage(page + 1)}
prevPage={() => setPage(page + 1)}
goToPage={setPage}
renderRow={(v: TFeatureVersion, i: number) => {
const isOverLimit = !!versionLimit && i + 1 > versionLimit
const isOverLimit = false
const user = users?.find((user) => v.published_by === user.id)

return (
Expand All @@ -115,7 +78,11 @@ const FeatureHistoryPage: FC<FeatureHistoryPageType> = ({ match, router }) => {
'blur no-pointer': isOverLimit,
})}
>
<div className='flex-fill'>
<div
className={classNames('flex-fill', {
'overflow-hidden': !open,
})}
>
<div className='flex-row flex-fill'>
<div
className='table-column flex-fill'
Expand All @@ -134,8 +101,10 @@ const FeatureHistoryPage: FC<FeatureHistoryPageType> = ({ match, router }) => {
</div>
</div>
<div className='table-column' style={{ width: widths[1] }}>
<Link
to={`/project/${match.params.projectId}/environment/${environmentApiKey}/history/${v.uuid}/`}
<a
href={`/project/${projectId}/environment/${environmentApiKey}/history/${v.uuid}/`}
target='_blank'
rel='noreferrer'
>
<Button
theme='text'
Expand All @@ -144,7 +113,7 @@ const FeatureHistoryPage: FC<FeatureHistoryPageType> = ({ match, router }) => {
>
View Details
</Button>
</Link>
</a>
</div>
<div className='table-column' style={{ width: widths[1] }}>
{i + 1 !== data!.results.length && (
Expand Down Expand Up @@ -229,7 +198,7 @@ const FeatureHistoryPage: FC<FeatureHistoryPageType> = ({ match, router }) => {
</div>
{diff === v.uuid && (
<FeatureVersion
projectId={`${match.params.projectId}`}
projectId={`${projectId}`}
featureId={feature}
environmentId={environmentId}
newUUID={compareToLive ? live!.uuid : v.uuid}
Expand All @@ -250,4 +219,4 @@ const FeatureHistoryPage: FC<FeatureHistoryPageType> = ({ match, router }) => {
)
}

export default ConfigProvider(FeatureHistoryPage)
export default ConfigProvider(FeatureHistory)
4 changes: 1 addition & 3 deletions frontend/web/components/FeatureRow.js
Original file line number Diff line number Diff line change
Expand Up @@ -392,9 +392,7 @@ class TheComponent extends Component {
hideHistory={!environment?.use_v2_feature_versioning}
onShowHistory={() => {
if (disableControls) return
this.context.router.history.push(
`/project/${projectId}/environment/${environmentId}/history?feature=${projectFlag.id}`,
)
this.editFeature(projectFlag, environmentFlags[id], 'history')
}}
onShowAudit={() => {
if (disableControls) return
Expand Down
1 change: 1 addition & 0 deletions frontend/web/components/base/forms/TabItem.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ type TabItemType = {
tabLabelString?: string
tabLabel: ReactNode
children: ReactNode
className?: string
}

const TabItem: FC<TabItemType> = ({ children }) => {
Expand Down
4 changes: 2 additions & 2 deletions frontend/web/components/base/forms/Tabs.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ const Tabs = class extends React.Component {
return (
<div className={`tabs ${this.props.className || ''}`}>
<div
className={`tabs-nav ${this.props.theme}`}
className={`${hideNav ? '' : 'tabs-nav'} ${this.props.theme}`}
style={isMobile ? { flexWrap: 'wrap' } : {}}
>
{!hideNav &&
Expand Down Expand Up @@ -97,7 +97,7 @@ const Tabs = class extends React.Component {
key={`content${i}`}
className={`tab-item ${isSelected ? ' tab-active' : ''} ${
this.props.isRoles && 'p-0'
}`}
} ${child.props.className || ''}`}
>
{child}
</div>
Expand Down
9 changes: 6 additions & 3 deletions frontend/web/components/diff/DiffFeature.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ const DiffFeature: FC<FeatureDiffType> = ({
const hideValue =
!totalChanges && (diff.newValue === null || diff.newValue === undefined)
return (
<div className='p-2'>
<div>
{!feature ? (
<div className='text-center'>
<Loader />
Expand All @@ -94,6 +94,7 @@ const DiffFeature: FC<FeatureDiffType> = ({
value={value}
>
<TabItem
className={'p-0'}
tabLabel={
<div className='d-flex justify-content-center gap-1 align-items-center'>
Value
Expand All @@ -105,8 +106,8 @@ const DiffFeature: FC<FeatureDiffType> = ({
}
>
{!totalChanges && (
<div className='mt-4'>
<InfoMessage>No Changes Found</InfoMessage>
<div className='mb-3 text-center fw-semibold'>
No Changes Found
</div>
)}
{!!valueConflict && (
Expand Down Expand Up @@ -169,6 +170,7 @@ const DiffFeature: FC<FeatureDiffType> = ({
</TabItem>
{!!variationDiffs?.diffs?.length && (
<TabItem
className={'p-0'}
tabLabel={
<div>
Variations{' '}
Expand All @@ -185,6 +187,7 @@ const DiffFeature: FC<FeatureDiffType> = ({
)}
{!!segmentDiffs?.diffs.length && (
<TabItem
className={'p-0'}
tabLabel={
<div className='d-flex justify-content-center gap-1 align-items-center'>
Segment Overrides
Expand Down
4 changes: 4 additions & 0 deletions frontend/web/components/diff/DiffSegments.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ const DiffSegments: FC<DiffSegmentsType> = ({
<Tabs className='mt-4' uncontrolled theme='pill'>
{!!created.length && (
<TabItem
className='p-0'
tabLabel={
<div>
Created <div className='unread'>{created.length}</div>
Expand All @@ -135,6 +136,7 @@ const DiffSegments: FC<DiffSegmentsType> = ({
)}
{!!deleted.length && (
<TabItem
className='p-0'
tabLabel={
<div>
Deleted <div className='unread'>{deleted.length}</div>
Expand All @@ -154,6 +156,7 @@ const DiffSegments: FC<DiffSegmentsType> = ({
)}
{!!modified.length && (
<TabItem
className='p-0'
tabLabel={
<div>
Modified <div className='unread'>{modified.length}</div>
Expand All @@ -173,6 +176,7 @@ const DiffSegments: FC<DiffSegmentsType> = ({
)}
{!!unChanged.length && (
<TabItem
className='p-0'
tabLabel={
<div>
Unchanged <div className='unread'>{unChanged.length}</div>
Expand Down
Loading
Loading