From 72424cefb1140def0d0e4a43b36193adc24c4e88 Mon Sep 17 00:00:00 2001 From: Abhishek Verma <72438220+avas27JTG@users.noreply.github.com> Date: Thu, 19 Jan 2023 16:10:26 +0530 Subject: [PATCH] =?UTF-8?q?[MI-2613]:=20Fixed=20resetting=20error=20messag?= =?UTF-8?q?e=20on=20Link=20modal,=20showing=20descr=E2=80=A6=20(#26)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * [MI-2611]: Fixed modals not opening on multiple clients in case of cloud servers * [MI-2613]: Fixed resetting error message on Link modal, showing descriptive error messages on UI and handling modals if account is not connected * [MI-2613]: Fixed channel validation for DMs * [MI-2613]: Added checks for filter API calls and user friendly error message * [MI-2613]: Added logic to check if a subscription is not found on the Azure DevOps portal then delete it from the Mattermost's KV store * [MI-2613]: Review fixes * [MI-2613]: Review fixes Co-authored-by: Abhishek Verma --- server/plugin/utils.go | 3 ++- webapp/src/app.tsx | 12 ++++++++--- .../src/containers/modals/LinkModal/index.tsx | 5 ++++- .../modals/SubscribeModal/filters/boards.tsx | 4 +++- .../SubscribeModal/filters/pipelines.tsx | 4 +++- .../modals/SubscribeModal/filters/repos.tsx | 4 +++- .../modals/SubscribeModal/index.tsx | 20 ++++++++++++++----- webapp/src/pluginConstants/messages.ts | 3 +++ webapp/src/utils/errorHandling.ts | 12 ++++++++--- 9 files changed, 51 insertions(+), 16 deletions(-) diff --git a/server/plugin/utils.go b/server/plugin/utils.go index 41f03c5c..7fd948a8 100644 --- a/server/plugin/utils.go +++ b/server/plugin/utils.go @@ -454,7 +454,8 @@ func (p *Plugin) UpdatePipelineRunApprovalPost(approvalSteps []*serializers.Appr } func (p *Plugin) deleteSubscription(subscription *serializers.SubscriptionDetails, mattermostUserID string) (int, error) { - if statusCode, err := p.Client.DeleteSubscription(subscription.OrganizationName, subscription.SubscriptionID, mattermostUserID); err != nil { + // On deletion, if a subscription is not found on the Azure DevOps portal then delete it from Mattermost's KV store + if statusCode, err := p.Client.DeleteSubscription(subscription.OrganizationName, subscription.SubscriptionID, mattermostUserID); statusCode != http.StatusNotFound && err != nil { return statusCode, err } diff --git a/webapp/src/app.tsx b/webapp/src/app.tsx index 8cc3e1f4..90e21179 100644 --- a/webapp/src/app.tsx +++ b/webapp/src/app.tsx @@ -10,6 +10,7 @@ import {toggleShowTaskModal} from 'reducers/taskModal'; import {getGlobalModalState, getLinkModalState, getSubscribeModalState, getCreateTaskModalState, getRhsState, getWebsocketEventState} from 'selectors'; import usePluginApi from 'hooks/usePluginApi'; +import useApiRequestCompletionState from 'hooks/useApiRequestCompletionState'; // Global styles import 'styles/main.scss'; @@ -18,7 +19,7 @@ import 'styles/main.scss'; * This is a central component for adding account connection validation on all the modals registered in the root component */ const App = (): JSX.Element => { - const {state, makeApiRequest, makeApiRequestWithCompletionStatus} = usePluginApi(); + const {state, makeApiRequestWithCompletionStatus} = usePluginApi(); const dispatch = useDispatch(); const {isConnected} = getWebsocketEventState(state); @@ -28,13 +29,18 @@ const App = (): JSX.Element => { const {visibility: createTaskModalVisibility} = getCreateTaskModalState(state); const {visibility: subscribeModalVisibility} = getSubscribeModalState(state); - // Check if user is connected on page reload + // Check if user is connected useEffect(() => { if (!isConnected) { - makeApiRequest(pluginConstants.pluginApiServiceConfigs.getUserDetails.apiServiceName); + makeApiRequestWithCompletionStatus(pluginConstants.pluginApiServiceConfigs.getUserDetails.apiServiceName); } }, [isSidebarOpen, modalId]); + useApiRequestCompletionState({ + serviceName: pluginConstants.pluginApiServiceConfigs.getUserDetails.apiServiceName, + handleError: () => dispatch(resetGlobalModalState()), + }); + /** * When a command is issued on the Mattermost to open any modal * then here we first check if the user's account is connected or not diff --git a/webapp/src/containers/modals/LinkModal/index.tsx b/webapp/src/containers/modals/LinkModal/index.tsx index be8b8915..5aebbf1f 100644 --- a/webapp/src/containers/modals/LinkModal/index.tsx +++ b/webapp/src/containers/modals/LinkModal/index.tsx @@ -35,10 +35,12 @@ const LinkModal = () => { const {visibility, organization, project} = getLinkModalState(state); const [showResultPanel, setShowResultPanel] = useState(false); const [resultPanelHeader, setResultPanelHeader] = useState(pluginConstants.common.projectLinkedSuccessfullyMessage); + const [errorMessage, setErrorMessage] = useState(''); // Function to hide the modal and reset all the states. const resetModalState = () => { dispatch(toggleShowLinkModal({isVisible: false, commandArgs: []})); + setErrorMessage(''); resetFormFields(); setShowResultPanel(false); setResultPanelHeader(pluginConstants.common.projectLinkedSuccessfullyMessage); @@ -71,6 +73,7 @@ const LinkModal = () => { setShowResultPanel(true); dispatch(toggleShowLinkModal({isVisible: true, commandArgs: [], isActionDone: true})); }, + handleError: () => setErrorMessage(Utils.getErrorMessage(isError, 'LinkProjectModal', error as ApiErrorResponse)), }); // Set modal field values @@ -94,7 +97,7 @@ const LinkModal = () => { confirmDisabled={isLoading} loading={isLoading} showFooter={!showResultPanel} - error={Utils.getErrorMessage(isError, 'LinkProjectModal', error as ApiErrorResponse)} + error={errorMessage} > <> { diff --git a/webapp/src/containers/modals/SubscribeModal/filters/boards.tsx b/webapp/src/containers/modals/SubscribeModal/filters/boards.tsx index 3215d948..3bf25253 100644 --- a/webapp/src/containers/modals/SubscribeModal/filters/boards.tsx +++ b/webapp/src/containers/modals/SubscribeModal/filters/boards.tsx @@ -16,6 +16,7 @@ type BoardsFilterProps = { selectedAreaPath: string handleSelectAreaPath: (value: string, name?: string) => void setIsFiltersError: (value: boolean) => void + isModalOpen: boolean } const BoardsFilter = ({ @@ -25,6 +26,7 @@ const BoardsFilter = ({ selectedAreaPath, handleSelectAreaPath, setIsFiltersError, + isModalOpen, }: BoardsFilterProps) => { const {subscriptionFiltersNameForBoards, subscriptionFiltersForBoards} = pluginConstants.form; @@ -41,7 +43,7 @@ const BoardsFilter = ({ }), [organization, projectId, eventType, subscriptionFiltersForBoards]); useEffect(() => { - if (organization && projectId && eventType) { + if (isModalOpen && organization && projectId && eventType) { makeApiRequestWithCompletionStatus( pluginConstants.pluginApiServiceConfigs.getSubscriptionFilters.apiServiceName, getSubscriptionFiltersRequest, diff --git a/webapp/src/containers/modals/SubscribeModal/filters/pipelines.tsx b/webapp/src/containers/modals/SubscribeModal/filters/pipelines.tsx index 50f8f76f..c6376437 100644 --- a/webapp/src/containers/modals/SubscribeModal/filters/pipelines.tsx +++ b/webapp/src/containers/modals/SubscribeModal/filters/pipelines.tsx @@ -14,6 +14,7 @@ type PipelinesFilterProps = { projectId: string eventType: string selectedBuildPipeline: string + isModalOpen: boolean handleSelectBuildPipeline: (value: string, name?: string) => void setIsFiltersError: (value: boolean) => void selectedBuildStatus: string @@ -51,6 +52,7 @@ const PipelinesFilter = ({ projectId, eventType, selectedBuildPipeline, + isModalOpen, handleSelectBuildPipeline, setIsFiltersError, selectedBuildStatus, @@ -96,7 +98,7 @@ const PipelinesFilter = ({ }), [organization, projectId, eventType, subscriptionFiltersForPipelines, selectedBuildPipeline, selectedReleasePipeline, selectedRunPipeline]); useEffect(() => { - if (eventType) { + if (isModalOpen && organization && projectId && eventType) { makeApiRequestWithCompletionStatus( pluginConstants.pluginApiServiceConfigs.getSubscriptionFilters.apiServiceName, getSubscriptionFiltersRequest, diff --git a/webapp/src/containers/modals/SubscribeModal/filters/repos.tsx b/webapp/src/containers/modals/SubscribeModal/filters/repos.tsx index 71754973..e165b3b5 100644 --- a/webapp/src/containers/modals/SubscribeModal/filters/repos.tsx +++ b/webapp/src/containers/modals/SubscribeModal/filters/repos.tsx @@ -14,6 +14,7 @@ type ReposFilterProps = { projectId: string eventType: string selectedRepo: string + isModalOpen: boolean handleSelectRepo: (value: string, name?: string) => void selectedTargetBranch: string handleSelectTargetBranch: (value: string, name?: string) => void @@ -36,6 +37,7 @@ const ReposFilter = ({ projectId, eventType, selectedRepo, + isModalOpen, handleSelectRepo, selectedTargetBranch, handleSelectTargetBranch, @@ -67,7 +69,7 @@ const ReposFilter = ({ }), [organization, projectId, eventType, subscriptionFiltersForRepos, selectedRepo]); useEffect(() => { - if (organization && projectId && eventType) { + if (isModalOpen && organization && projectId && eventType) { makeApiRequestWithCompletionStatus( pluginConstants.pluginApiServiceConfigs.getSubscriptionFilters.apiServiceName, getSubscriptionFiltersRequest, diff --git a/webapp/src/containers/modals/SubscribeModal/index.tsx b/webapp/src/containers/modals/SubscribeModal/index.tsx index a06ba65c..d0a0530f 100644 --- a/webapp/src/containers/modals/SubscribeModal/index.tsx +++ b/webapp/src/containers/modals/SubscribeModal/index.tsx @@ -219,11 +219,18 @@ const SubscribeModal = () => { // Set organization, project and channel list values useEffect(() => { + let isCurrentChannelIdPresentInChannelList = false; // Check if the current channel ID is the ID of a public or private channel and not the ID of a DM or group channel if (isChannelListSuccess && !showResultPanel) { - setChannelOptions(channelList?.map((channel) => ({ - label: {channel.display_name}, - value: channel.id, - }))); + setChannelOptions(channelList?.map((channel) => { + if (currentChannelId === channel.id) { + isCurrentChannelIdPresentInChannelList = true; + } + + return ({ + label: {channel.display_name}, + value: channel.id, + }); + })); } // Pre-select the dropdown value in case of single option @@ -231,7 +238,7 @@ const SubscribeModal = () => { const autoSelectedValues: Pick, 'organization' | 'project' | 'channelID'> = { organization: organization ?? '', project: project ?? '', - channelID: currentChannelId ?? '', + channelID: isCurrentChannelIdPresentInChannelList && currentChannelId ? currentChannelId : '', }; if (!organization && organizationList.length === 1) { @@ -453,6 +460,7 @@ const SubscribeModal = () => { { formFields.serviceType === pluginConstants.common.boards && formFields.eventType && Object.keys(eventTypeBoards).includes(formFields.eventType) && ( { { formFields.serviceType === pluginConstants.common.repos && formFields.eventType && Object.keys(eventTypeRepos).includes(formFields.eventType) && ( { { formFields.serviceType === pluginConstants.common.pipelines && formFields.eventType && Object.keys(eventTypePipelines).includes(formFields.eventType) && (