diff --git a/frontend/common/dispatcher/action-constants.js b/frontend/common/dispatcher/action-constants.js index 6b44608b7d8c..44118b4bf115 100644 --- a/frontend/common/dispatcher/action-constants.js +++ b/frontend/common/dispatcher/action-constants.js @@ -1,14 +1,12 @@ import BaseActions from './base/_action-constants' const Actions = Object.assign({}, BaseActions, { 'ACCEPT_INVITE': 'ACCEPT_INVITE', - 'ACTION_CHANGE_REQUEST': 'ACTION_CHANGE_REQUEST', 'CHANGE_USER_FLAG': 'CHANGE_USER_FLAG', 'CONFIRM_TWO_FACTOR': 'CONFIRM_TWO_FACTOR', 'CREATE_ENV': 'CREATE_ENV', 'CREATE_FLAG': 'CREATE_FLAG', 'CREATE_ORGANISATION': 'CREATE_ORGANISATION', 'CREATE_PROJECT': 'CREATE_PROJECT', - 'DELETE_CHANGE_REQUEST': 'DELETE_CHANGE_REQUEST', 'DELETE_ENVIRONMENT': 'DELETE_ENVIRONMENT', 'DELETE_ORGANISATION': 'DELETE_ORGANISATION', 'DELETE_PROJECT': 'DELETE_PROJECT', @@ -24,7 +22,6 @@ const Actions = Object.assign({}, BaseActions, { 'EDIT_PROJECT': 'EDIT_PROJECT', 'EDIT_USER_FLAG': 'EDIT_USER_FLAG', 'ENABLE_TWO_FACTOR': 'ENABLE_TWO_FACTOR', - 'GET_CHANGE_REQUEST': 'GET_CHANGE_REQUEST', 'GET_ENVIRONMENT': 'GET_ENVIRONMENT', 'GET_IDENTITY': 'GET_IDENTITY', 'GET_ORGANISATION': 'GET_ORGANISATION', @@ -37,7 +34,6 @@ const Actions = Object.assign({}, BaseActions, { 'SELECT_ORGANISATION': 'SELECT_ORGANISATION', 'TOGGLE_USER_FLAG': 'TOGGLE_USER_FLAG', 'TWO_FACTOR_LOGIN': 'TWO_FACTOR_LOGIN', - 'UPDATE_CHANGE_REQUEST': 'UPDATE_CHANGE_REQUEST', 'UPDATE_SUBSCRIPTION': 'UPDATE_SUBSCRIPTION', 'UPDATE_USER_ROLE': 'UPDATE_USER_ROLE', }) diff --git a/frontend/common/dispatcher/app-actions.js b/frontend/common/dispatcher/app-actions.js index e2a4ea8c1d34..9e0c4bf6a0ca 100644 --- a/frontend/common/dispatcher/app-actions.js +++ b/frontend/common/dispatcher/app-actions.js @@ -8,14 +8,6 @@ const AppActions = Object.assign({}, BaseAppActions, { id, }) }, - actionChangeRequest(id, action, cb) { - Dispatcher.handleViewAction({ - action, - actionType: Actions.ACTION_CHANGE_REQUEST, - cb, - id, - }) - }, changeUserFlag(identity) { Dispatcher.handleViewAction({ actionType: Actions.CHANGE_USER_FLAG, @@ -59,13 +51,6 @@ const AppActions = Object.assign({}, BaseAppActions, { }) }, - deleteChangeRequest(id, cb) { - Dispatcher.handleViewAction({ - actionType: Actions.DELETE_CHANGE_REQUEST, - cb, - id, - }) - }, deleteEnv(env) { Dispatcher.handleViewAction({ actionType: Actions.DELETE_ENVIRONMENT, @@ -187,14 +172,6 @@ const AppActions = Object.assign({}, BaseAppActions, { actionType: Actions.ENABLE_TWO_FACTOR, }) }, - getChangeRequest(id, projectId, environmentId) { - Dispatcher.handleViewAction({ - actionType: Actions.GET_CHANGE_REQUEST, - environmentId, - id, - projectId, - }) - }, getIdentity(envId, id) { Dispatcher.handleViewAction({ actionType: Actions.GET_IDENTITY, @@ -274,12 +251,6 @@ const AppActions = Object.assign({}, BaseAppActions, { pin, }) }, - updateChangeRequest(changeRequest) { - Dispatcher.handleViewAction({ - actionType: Actions.UPDATE_CHANGE_REQUEST, - changeRequest, - }) - }, updateSubscription(hostedPageId) { Dispatcher.handleViewAction({ actionType: Actions.UPDATE_SUBSCRIPTION, diff --git a/frontend/common/hooks/useChangeRequestDetail.ts b/frontend/common/hooks/useChangeRequestDetail.ts new file mode 100644 index 000000000000..dab773f00fd2 --- /dev/null +++ b/frontend/common/hooks/useChangeRequestDetail.ts @@ -0,0 +1,98 @@ +import { + ChangeRequest, + FeatureState, + ProjectFlag, +} from 'common/types/responses' +import { + useActionChangeRequestMutation, + useDeleteChangeRequestMutation, + useGetChangeRequestQuery, + useUpdateChangeRequestAssigneesMutation, +} from 'common/services/useChangeRequest' +import { isNotFoundError, toErrorMessage } from 'common/utils/requestError' + +export type ChangeRequestAssignees = Pick< + ChangeRequest, + 'approvals' | 'group_assignments' +> + +type ChangeRequestDetailParams = { + environmentId: string + id: string + projectId: string +} + +export type ChangeRequestDetail = { + changeRequest?: ChangeRequest + environmentFlag?: FeatureState + projectFlag?: ProjectFlag + error?: unknown + isUpdating: boolean + notFound: boolean + approve: () => void + commit: (onDone: () => void) => void + remove: (onDone: () => void) => void + updateAssignees: (assignees: ChangeRequestAssignees) => void +} + +export const useChangeRequestDetail = ({ + environmentId, + id, + projectId, +}: ChangeRequestDetailParams): ChangeRequestDetail => { + const { + data, + error: loadError, + isFetching, + } = useGetChangeRequestQuery( + { environmentId, id, projectId }, + // Refetch after an action, not on window focus + { refetchOnFocus: false }, + ) + const [action, actionState] = useActionChangeRequestMutation() + const [updateAssignees, updateState] = + useUpdateChangeRequestAssigneesMutation() + const [deleteChangeRequest, deleteState] = useDeleteChangeRequestMutation() + + const isUpdating = + isFetching || + actionState.isLoading || + updateState.isLoading || + deleteState.isLoading + const error = + actionState.error || updateState.error || deleteState.error || loadError + // Each mutation keeps its last error, so clear them when a new action starts + const clearErrors = () => { + actionState.reset() + updateState.reset() + deleteState.reset() + } + + return { + approve: () => { + clearErrors() + action({ actionType: 'approve', id }) + }, + changeRequest: data?.changeRequest, + commit: async (onDone) => { + clearErrors() + const res = await action({ actionType: 'commit', id }) + if (!('error' in res)) onDone() + }, + environmentFlag: data?.environmentFlag, + error: toErrorMessage(error), + isUpdating, + notFound: !data && isNotFoundError(loadError), + projectFlag: data?.projectFlag, + remove: async (onDone) => { + clearErrors() + const res = await deleteChangeRequest({ id: Number(id) }) + if (!('error' in res)) onDone() + }, + updateAssignees: (assignees) => { + if (isUpdating || !data) return + clearErrors() + updateAssignees({ ...assignees, id }) + }, + } +} diff --git a/frontend/common/services/useChangeRequest.ts b/frontend/common/services/useChangeRequest.ts index 472965928441..de82b5478e47 100644 --- a/frontend/common/services/useChangeRequest.ts +++ b/frontend/common/services/useChangeRequest.ts @@ -2,6 +2,8 @@ import { ChangeRequest, ChangeSet, FeatureState, + PagedResponse, + ProjectFlag, Res, } from 'common/types/responses' import { Req } from 'common/types/requests' @@ -10,11 +12,42 @@ import Utils from 'common/utils/utils' import sortBy from 'lodash/sortBy' import moment from 'moment' import transformCorePaging from 'common/transformCorePaging' +import { sortMultivariateOptions } from 'common/utils/multivariate' +import { isNotFoundError } from 'common/utils/requestError' +import { + addFeatureSegmentsToFeatureStates, + featureStateService, +} from './useFeatureState' export const changeRequestService = service .enhanceEndpoints({ addTagTypes: ['ChangeRequest'] }) .injectEndpoints({ endpoints: (builder) => ({ + actionChangeRequest: builder.mutation< + Res['actionChangeRequest'], + Req['actionChangeRequest'] + >({ + invalidatesTags: (_, error, { id }) => + error + ? [] + : [ + { id, type: 'ChangeRequest' }, + { id: 'LIST', type: 'ChangeRequest' }, + ], + onQueryStarted: async (_, { dispatch, queryFulfilled }) => { + try { + await queryFulfilled + dispatch(featureStateService.util.invalidateTags(['FeatureState'])) + } catch { + // The mutation's own error state reports the failure + } + }, + query: ({ actionType, id }) => ({ + body: {}, + method: 'POST', + url: `features/workflows/change-requests/${id}/${actionType}/`, + }), + }), createEnvironmentChangeRequest: builder.mutation< Res['createEnvironmentChangeRequest'], Req['createEnvironmentChangeRequest'] @@ -57,6 +90,60 @@ export const changeRequestService = service url: `features/workflows/change-requests/${id}/`, }), }), + getChangeRequest: builder.query< + Res['changeRequestDetail'], + Req['getChangeRequest'] + >({ + providesTags: (_, _2, { id }) => [{ id, type: 'ChangeRequest' }], + queryFn: async ( + { environmentId, id, projectId }, + _api, + _extraOptions, + baseQuery, + ) => { + const res = await baseQuery({ + url: `features/workflows/change-requests/${id}/`, + }) + if (res.error) return { error: res.error } + const raw = res.data as ChangeRequest + const changeRequest: ChangeRequest = { + ...raw, + feature_states: await Promise.all( + raw.feature_states.map(addFeatureSegmentsToFeatureStates), + ), + } + const feature = + changeRequest.feature_states[0]?.feature || + changeRequest.change_sets?.[0]?.feature + const [environmentFlags, projectFlagRes] = await Promise.all([ + baseQuery({ + url: `environments/${environmentId}/featurestates/?feature=${feature}`, + }), + baseQuery({ url: `projects/${projectId}/features/${feature}/` }), + ]) + // A 404 means the feature was deleted since, which the page explains + if (isNotFoundError(projectFlagRes.error)) { + return { data: { changeRequest } } + } + if (projectFlagRes.error) return { error: projectFlagRes.error } + if (environmentFlags.error) return { error: environmentFlags.error } + const projectFlag = projectFlagRes.data as ProjectFlag + return { + data: { + changeRequest, + environmentFlag: ( + environmentFlags.data as PagedResponse + ).results[0], + projectFlag: { + ...projectFlag, + multivariate_options: sortMultivariateOptions( + projectFlag.multivariate_options, + ), + }, + }, + } + }, + }), getChangeRequests: builder.query< Res['changeRequests'], Req['getChangeRequests'] @@ -80,6 +167,43 @@ export const changeRequestService = service url: `features/workflows/change-requests/${changeRequest.id}/`, }), }), + updateChangeRequestAssignees: builder.mutation< + Res['updateChangeRequest'], + Req['updateChangeRequestAssignees'] + >({ + invalidatesTags: (_, error, { id }) => + error + ? [] + : [ + { id: `${id}`, type: 'ChangeRequest' }, + { id: 'LIST', type: 'ChangeRequest' }, + ], + // Re-read first so the PUT carries the stored change, not the page's copy + queryFn: async ( + { approvals, group_assignments, id }, + _api, + _extraOptions, + baseQuery, + ) => { + const url = `features/workflows/change-requests/${id}/` + const res = await baseQuery({ url }) + if (res.error) return { error: res.error } + const current = res.data as ChangeRequest + const put = await baseQuery({ + body: { + ...current, + approvals, + environment_feature_versions: + current.environment_feature_versions?.map((v) => v.uuid), + group_assignments, + }, + method: 'PUT', + url, + }) + if (put.error) return { error: put.error } + return { data: put.data as Res['updateChangeRequest'] } + }, + }), // END OF ENDPOINTS }), }) @@ -106,12 +230,26 @@ export async function updateChangeRequest( changeRequestService.endpoints.updateChangeRequest.initiate(data, options), ) } +export async function actionChangeRequest( + store: any, + data: Req['actionChangeRequest'], + options?: Parameters< + typeof changeRequestService.endpoints.actionChangeRequest.initiate + >[1], +) { + return store.dispatch( + changeRequestService.endpoints.actionChangeRequest.initiate(data, options), + ) +} // END OF FUNCTION_EXPORTS export const { + useActionChangeRequestMutation, useCreateEnvironmentChangeRequestMutation, useDeleteChangeRequestMutation, + useGetChangeRequestQuery, useGetChangeRequestsQuery, + useUpdateChangeRequestAssigneesMutation, useUpdateChangeRequestMutation, // END OF EXPORTS } = changeRequestService diff --git a/frontend/common/stores/change-requests-store.js b/frontend/common/stores/change-requests-store.js deleted file mode 100644 index f34e6a436d19..000000000000 --- a/frontend/common/stores/change-requests-store.js +++ /dev/null @@ -1,158 +0,0 @@ -import Dispatcher from 'common/dispatcher/dispatcher' -import { sortMultivariateOptions } from 'common/utils/multivariate' -import BaseStore from './base/_store' -import data from 'common/data/base/_data' -import { addFeatureSegmentsToFeatureStates } from 'common/services/useFeatureState' -import { getStore } from 'common/store' -import { changeRequestService } from 'common/services/useChangeRequest' -import { featureStateService } from 'common/services/useFeatureState' -const transformChangeRequest = async (changeRequest) => { - const feature_states = await Promise.all( - changeRequest.feature_states.map(addFeatureSegmentsToFeatureStates), - ) - - return { - ...changeRequest, - feature_states, - } -} -const controller = { - actionChangeRequest: (id, action, cb) => { - store.loading() - data - .post( - `${Project.api}features/workflows/change-requests/${id}/${action}/`, - {}, - ) - .then(() => { - data - .get(`${Project.api}features/workflows/change-requests/${id}/`) - .then(async (res) => { - store.model[id] = await transformChangeRequest(res) - cb && cb() - store.loaded() - getStore().dispatch( - changeRequestService.util.invalidateTags(['ChangeRequest']), - ) - getStore().dispatch( - featureStateService.util.invalidateTags(['FeatureState']), - ) - }) - }) - .catch((e) => API.ajaxHandler(store, e)) - }, - deleteChangeRequest: (id, cb) => { - store.loading() - data - .delete(`${Project.api}features/workflows/change-requests/${id}/`) - .then(() => { - store.loaded() - getStore().dispatch( - changeRequestService.util.invalidateTags(['ChangeRequest']), - ) - cb() - }) - .catch((e) => API.ajaxHandler(store, e)) - }, - getChangeRequest: (id, projectId, environmentId) => { - store.loading() - data - .get(`${Project.api}features/workflows/change-requests/${id}/`) - .then(async (apiResponse) => { - const res = await transformChangeRequest(apiResponse) - const feature = - res.feature_states[0]?.feature || res.change_sets[0]?.feature - return Promise.all([ - data.get( - `${Project.api}environments/${environmentId}/featurestates/?feature=${feature}`, - ), - data.get(`${Project.api}projects/${projectId}/features/${feature}/`), - ]) - .then(([environmentFlag, projectFlag]) => { - store.flags[id] = { - environmentFlag: environmentFlag.results[0], - // Every other read of a feature sorts these, so without it the - // change request lists the variations in a different order. - projectFlag: { - ...projectFlag, - multivariate_options: - projectFlag.multivariate_options && - sortMultivariateOptions(projectFlag.multivariate_options), - }, - } - }) - .finally(() => { - store.model[id] = res - store.loaded() - }) - }) - .catch((e) => API.ajaxHandler(store, e)) - }, - updateChangeRequest: (changeRequest) => { - store.loading() - data - .get( - `${Project.api}features/workflows/change-requests/${changeRequest.id}/`, - ) - .then((res) => { - data - .put( - `${Project.api}features/workflows/change-requests/${changeRequest.id}/`, - { - ...res, - approvals: changeRequest.approvals, - description: changeRequest.description, - environment_feature_versions: - changeRequest?.environment_feature_versions?.map((v) => v.uuid), - group_assignments: changeRequest.group_assignments, - title: changeRequest.title, - }, - ) - .then(async () => { - const res = await data.get( - `${Project.api}features/workflows/change-requests/${changeRequest.id}/`, - ) - store.model[changeRequest.id] = await transformChangeRequest(res) - getStore().dispatch( - changeRequestService.util.invalidateTags(['ChangeRequest']), - ) - store.loaded() - }) - .catch((e) => API.ajaxHandler(store, e)) - }) - }, -} - -const store = Object.assign({}, BaseStore, { - committed: {}, - flags: {}, - id: 'change-request-store', - model: {}, - scheduled: {}, -}) - -store.dispatcherIndex = Dispatcher.register(store, (payload) => { - const action = payload.action // this is our action from handleViewAction - switch (action.actionType) { - case Actions.GET_CHANGE_REQUEST: - controller.getChangeRequest( - action.id, - parseInt(action.projectId), - action.environmentId, - ) - break - case Actions.UPDATE_CHANGE_REQUEST: - controller.updateChangeRequest(action.changeRequest) - break - case Actions.DELETE_CHANGE_REQUEST: - controller.deleteChangeRequest(action.id, action.cb) - break - case Actions.ACTION_CHANGE_REQUEST: - controller.actionChangeRequest(action.id, action.action, action.cb) - break - default: - break - } -}) -controller.store = store -export default controller.store diff --git a/frontend/common/stores/feature-list-store.ts b/frontend/common/stores/feature-list-store.ts index 56ae9b431d5f..8d75720f1316 100644 --- a/frontend/common/stores/feature-list-store.ts +++ b/frontend/common/stores/feature-list-store.ts @@ -37,6 +37,7 @@ import { getVersionFeatureState } from 'common/services/useVersionFeatureState' import { getFeatureStates } from 'common/services/useFeatureState' import { getSegments } from 'common/services/useSegment' import { + actionChangeRequest, changeRequestService, updateChangeRequest, } from 'common/services/useChangeRequest' @@ -666,7 +667,6 @@ const controller = { : `${Project.api}environments/${environmentId}/create-change-request/` return data[reqType](url, req).then( (updatedChangeRequest: ChangeRequest) => { - let prom = Promise.resolve() const shouldUpdateMv = multivariate_options && updatedChangeRequest.feature_states?.[0] if (shouldUpdateMv) { @@ -693,7 +693,7 @@ const controller = { ) } - prom = ( + void ( shouldUpdateMv || (commit && ignore_conflicts) ? updateChangeRequest( getStore(), @@ -705,33 +705,19 @@ const controller = { : updatedChangeRequest, ) : Promise.resolve() - ).then(() => { + ).then(async () => { if (commit) { - AppActions.actionChangeRequest( - updatedChangeRequest.id, - 'commit', - () => { - AppActions.refreshFeatures(projectId, environmentId) - }, - ) + const res = await actionChangeRequest(getStore(), { + actionType: 'commit', + id: `${updatedChangeRequest.id}`, + }) + if (!('error' in res)) { + AppActions.refreshFeatures(projectId, environmentId) + } } else { getStore().dispatch( changeRequestService.util.invalidateTags(['ChangeRequest']), ) - AppActions.getChangeRequest( - updatedChangeRequest.id, - projectId, - environmentId, - ) - } - }) - prom.then(() => { - if (featureStateId) { - AppActions.getChangeRequest( - changeRequestData.id, - projectId, - environmentId, - ) } }) return updatedChangeRequest diff --git a/frontend/common/types/requests.ts b/frontend/common/types/requests.ts index a99864c3f2e8..fd213742531a 100644 --- a/frontend/common/types/requests.ts +++ b/frontend/common/types/requests.ts @@ -39,6 +39,8 @@ import { } from './responses' import { UtmsType } from './utms' +export type ChangeRequestAction = 'approve' | 'commit' + export type UpdateProjectBody = { name: string hide_disabled_flags?: boolean @@ -701,6 +703,13 @@ export type Req = { committed?: boolean }> updateChangeRequest: ChangeRequest + getChangeRequest: { id: string; environmentId: string; projectId: string } + actionChangeRequest: { id: string; actionType: ChangeRequestAction } + updateChangeRequestAssignees: { + id: string + approvals: ChangeRequest['approvals'] + group_assignments: ChangeRequest['group_assignments'] + } getGroupSummaries: { orgId: number } diff --git a/frontend/common/types/responses.ts b/frontend/common/types/responses.ts index f8dee3a87451..2262cea98e27 100644 --- a/frontend/common/types/responses.ts +++ b/frontend/common/types/responses.ts @@ -1686,6 +1686,12 @@ export type Res = { projectChangeRequests: PagedResponse projectChangeRequest: ProjectChangeRequest actionChangeRequest: {} + changeRequestDetail: { + changeRequest: ChangeRequest + // Missing when the feature has since been deleted + environmentFlag?: FeatureState + projectFlag?: ProjectFlag + } identityTrait: { id: string } identityTraits: IdentityTrait[] conversionEvents: PagedResponse diff --git a/frontend/common/utils/__tests__/requestError.test.ts b/frontend/common/utils/__tests__/requestError.test.ts new file mode 100644 index 000000000000..035101000172 --- /dev/null +++ b/frontend/common/utils/__tests__/requestError.test.ts @@ -0,0 +1,56 @@ +import { isNotFoundError, toErrorMessage } from 'common/utils/requestError' + +describe('isNotFoundError', () => { + it('is true for a 404', () => { + expect(isNotFoundError({ data: {}, status: 404 })).toBe(true) + }) + + it.each([ + ['no error', undefined], + ['a server error', { data: {}, status: 500 }], + ['a permission error', { data: {}, status: 403 }], + ['a network error', { error: 'Failed to fetch', status: 'FETCH_ERROR' }], + ['a serialized error', { message: 'Boom', name: 'Error' }], + ] as const)('is false for %s', (_, error) => { + expect(isNotFoundError(error)).toBe(false) + }) +}) + +describe('toErrorMessage', () => { + it("returns the API's detail message", () => { + // Given + const error = { data: { detail: 'Already committed.' }, status: 400 } + + // When + const message = toErrorMessage(error) + + // Then + expect(message).toBe('Already committed.') + }) + + it('returns field errors as they are', () => { + // Given + const error = { data: { title: ['This field is required.'] }, status: 400 } + + // When + const message = toErrorMessage(error) + + // Then + expect(message).toBe(error) + }) + + it('returns errors without a body as they are', () => { + // Given + const error = { message: 'Boom', name: 'Error' } + + // When + const message = toErrorMessage(error) + + // Then + expect(message).toBe(error) + }) + + it('returns nothing when there is no error', () => { + expect(toErrorMessage(undefined)).toBeUndefined() + }) +}) diff --git a/frontend/common/utils/requestError.ts b/frontend/common/utils/requestError.ts new file mode 100644 index 000000000000..20181007c429 --- /dev/null +++ b/frontend/common/utils/requestError.ts @@ -0,0 +1,21 @@ +import type { FetchBaseQueryError } from '@reduxjs/toolkit/query' +import type { SerializedError } from '@reduxjs/toolkit' + +export type RequestError = FetchBaseQueryError | SerializedError | undefined + +export const isNotFoundError = (error: RequestError) => + !!error && 'status' in error && error.status === 404 + +const hasDetail = (data: unknown): data is { detail: string } => + typeof data === 'object' && + data !== null && + 'detail' in data && + typeof data.detail === 'string' + +// Show the API's detail message on its own, not the whole error object +export const toErrorMessage = (error: RequestError) => { + if (error && 'data' in error && hasDetail(error.data)) { + return error.data.detail + } + return error +} diff --git a/frontend/web/components/pages/ChangeRequestDetailPage.tsx b/frontend/web/components/pages/ChangeRequestDetailPage.tsx index 781e2f59f40d..ec10cac0a3fe 100644 --- a/frontend/web/components/pages/ChangeRequestDetailPage.tsx +++ b/frontend/web/components/pages/ChangeRequestDetailPage.tsx @@ -1,7 +1,6 @@ import { FC, ReactNode, useEffect, useMemo, useState } from 'react' import Icon from 'components/icons/Icon' import OrganisationStore from 'common/stores/organisation-store' -import ChangeRequestStore from 'common/stores/change-requests-store' import FeatureListStore from 'common/stores/feature-list-store' import { useGetMyGroupsQuery } from 'common/services/useMyGroup' import CreateFeatureModal from 'components/modals/create-feature' @@ -44,6 +43,7 @@ import { useHistory } from 'react-router-dom' import { openPublishChangeRequestConfirm } from 'components/PublishChangeRequestModal' import { getChangeRequestLiveDate } from 'common/utils/getChangeRequestLiveDate' import { EnvironmentPermission } from 'common/types/permissions.types' +import { useChangeRequestDetail } from 'common/hooks/useChangeRequestDetail' type ChangeRequestPageType = { match: { @@ -60,21 +60,8 @@ const ChangeRequestDetailPage: FC = ({ match }) => { const { environmentId, id, projectId } = match.params const [_, setUpdate] = useState(Date.now()) const [updateChangeRequest] = useUpdateChangeRequestMutation() - const error = ChangeRequestStore.error - const changeRequest = ( - ChangeRequestStore.model as Record | undefined - )?.[id] - const flags = ( - ChangeRequestStore.flags as Record< - string, - { - environmentFlag: FeatureState - projectFlag: ProjectFlag - } - > - )[id] - const environmentFlag = flags?.environmentFlag - const projectFlag = flags && flags.projectFlag + const detail = useChangeRequestDetail({ environmentId, id, projectId }) + const { changeRequest, environmentFlag, error, projectFlag } = detail const approvePermission = useHasPermission({ id: environmentId, level: 'environment', @@ -93,54 +80,40 @@ const ChangeRequestDetailPage: FC = ({ match }) => { permission: EnvironmentPermission.ADMIN, }) - useEffect(() => { - AppActions.getChangeRequest(id, projectId, environmentId) - }, [id, projectId, environmentId]) - useEffect(() => { const forceUpdate = () => { setUpdate(Date.now()) } - ChangeRequestStore.on('change', forceUpdate) FeatureListStore.on('change', forceUpdate) OrganisationStore.on('change', forceUpdate) return () => { - ChangeRequestStore.off('change', forceUpdate) FeatureListStore.off('change', forceUpdate) OrganisationStore.off('change', forceUpdate) } }, []) const addOwner = (id: number, isUser = true) => { - if (ChangeRequestStore.isLoading || !changeRequest) return - AppActions.updateChangeRequest({ + if (!changeRequest) return + detail.updateAssignees({ approvals: isUser ? changeRequest.approvals.concat([{ user: id }]) : changeRequest.approvals, - description: changeRequest.description, - feature_states: changeRequest.feature_states, group_assignments: isUser ? changeRequest.group_assignments : changeRequest.group_assignments.concat([{ group: id }]), - id: changeRequest.id, - title: changeRequest.title, }) } const removeOwner = (id: number, isUser = true) => { - if (ChangeRequestStore.isLoading || !changeRequest) return - AppActions.updateChangeRequest({ + if (!changeRequest) return + detail.updateAssignees({ approvals: isUser ? changeRequest.approvals.filter((v) => v.user !== id) : changeRequest.approvals, - description: changeRequest.description, - feature_states: changeRequest.feature_states, group_assignments: isUser ? changeRequest.group_assignments : changeRequest.group_assignments.filter((v) => v.group !== id), - id: changeRequest.id, - title: changeRequest.title, }) } @@ -154,7 +127,7 @@ const ChangeRequestDetailPage: FC = ({ match }) => { ), destructive: true, onYes: () => { - AppActions.deleteChangeRequest(id, () => { + detail.remove(() => { history.replace( `/project/${projectId}/environment/${environmentId}/change-requests`, ) @@ -232,7 +205,7 @@ const ChangeRequestDetailPage: FC = ({ match }) => { } const approveChangeRequest = () => { - AppActions.actionChangeRequest(id, 'approve') + detail.approve() } const publishChangeRequest = () => { @@ -258,7 +231,7 @@ const ChangeRequestDetailPage: FC = ({ match }) => { isScheduled, onYes: (ignore_conflicts) => { const commitChangeRequest = () => { - AppActions.actionChangeRequest(id, 'commit', () => { + detail.commit(() => { AppActions.refreshFeatures(projectId, environmentId) }) } @@ -281,7 +254,7 @@ const ChangeRequestDetailPage: FC = ({ match }) => { }) } - if (error && !changeRequest) { + if (detail.notFound) { return (
= ({ match }) => {
) } + if (!changeRequest && error) { + return ( +
+ +
+ ) + } if (!changeRequest || OrganisationStore.isLoading) { return (
= ({ match }) => { return (
= ({ match }) => { scheduledDate={getChangeRequestLiveDate(changeRequest)} deleteChangeRequest={deleteChangeRequest} editChangeRequest={ - !changeRequest?.committed_at + !changeRequest?.committed_at && projectFlag && environmentFlag ? () => editChangeRequest(projectFlag, environmentFlag) : undefined } diff --git a/frontend/web/components/pages/SegmentPage.tsx b/frontend/web/components/pages/SegmentPage.tsx index 6f0dc80e166a..0adb06cf573c 100644 --- a/frontend/web/components/pages/SegmentPage.tsx +++ b/frontend/web/components/pages/SegmentPage.tsx @@ -5,7 +5,6 @@ import { useHistory, useRouteMatch } from 'react-router-dom' import CreateSegment from 'components/modals/CreateSegment' import ProjectStore from 'common/stores/project-store' import { Environment } from 'common/types/responses' -import ChangeRequestStore from 'common/stores/change-requests-store' import Breadcrumb from 'components/Breadcrumb' import { useHasPermission } from 'common/providers/Permission' import Icon from 'components/icons/Icon' @@ -41,7 +40,6 @@ const SegmentPage: FC = ({}) => { } return (