Skip to content

Commit

Permalink
[MI-2613]: Fixed resetting error message on Link modal, showing descr… (
Browse files Browse the repository at this point in the history 
#26)

* [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 <[email protected]>
  • Loading branch information
@avas27JTG
avas27JTG and avas27JTG authored Jan 19, 2023
1 parent 4daf4e0 commit 72424ce
Show file tree
Hide file tree
Showing 9 changed files with 51 additions and 16 deletions.
3 changes: 2 additions & 1 deletion server/plugin/utils.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
12 changes: 9 additions & 3 deletions webapp/src/app.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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);
Expand All @@ -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
Expand Down
5 changes: 4 additions & 1 deletion webapp/src/containers/modals/LinkModal/index.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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
Expand All @@ -94,7 +97,7 @@ const LinkModal = () => {
confirmDisabled={isLoading}
loading={isLoading}
showFooter={!showResultPanel}
error={Utils.getErrorMessage(isError, 'LinkProjectModal', error as ApiErrorResponse)}
error={errorMessage}
>
<>
{
Expand Down
4 changes: 3 additions & 1 deletion webapp/src/containers/modals/SubscribeModal/filters/boards.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ type BoardsFilterProps = {
selectedAreaPath: string
handleSelectAreaPath: (value: string, name?: string) => void
setIsFiltersError: (value: boolean) => void
isModalOpen: boolean
}

const BoardsFilter = ({
Expand All @@ -25,6 +26,7 @@ const BoardsFilter = ({
selectedAreaPath,
handleSelectAreaPath,
setIsFiltersError,
isModalOpen,
}: BoardsFilterProps) => {
const {subscriptionFiltersNameForBoards, subscriptionFiltersForBoards} = pluginConstants.form;

Expand All @@ -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,
Expand Down
4 changes: 3 additions & 1 deletion webapp/src/containers/modals/SubscribeModal/filters/pipelines.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -51,6 +52,7 @@ const PipelinesFilter = ({
projectId,
eventType,
selectedBuildPipeline,
isModalOpen,
handleSelectBuildPipeline,
setIsFiltersError,
selectedBuildStatus,
Expand Down Expand Up @@ -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,
Expand Down
4 changes: 3 additions & 1 deletion webapp/src/containers/modals/SubscribeModal/filters/repos.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -36,6 +37,7 @@ const ReposFilter = ({
projectId,
eventType,
selectedRepo,
isModalOpen,
handleSelectRepo,
selectedTargetBranch,
handleSelectTargetBranch,
Expand Down Expand Up @@ -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,
Expand Down
20 changes: 15 additions & 5 deletions webapp/src/containers/modals/SubscribeModal/index.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -219,19 +219,26 @@ 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: <span><i className={`icon ${channel.type === mm_constants.PRIVATE_CHANNEL ? 'icon-lock-outline' : 'icon-globe'} dropdown-option-icon`}/>{channel.display_name}</span>,
value: channel.id,
})));
setChannelOptions(channelList?.map((channel) => {
if (currentChannelId === channel.id) {
isCurrentChannelIdPresentInChannelList = true;
}

return ({
label: <span><i className={`icon ${channel.type === mm_constants.PRIVATE_CHANNEL ? 'icon-lock-outline' : 'icon-globe'} dropdown-option-icon`}/>{channel.display_name}</span>,
value: channel.id,
});
}));
}

// Pre-select the dropdown value in case of single option
if (isOrganizationAndProjectListSuccess && !showResultPanel) {
const autoSelectedValues: Pick<Record<FormFieldNames, string>, 'organization' | 'project' | 'channelID'> = {
organization: organization ?? '',
project: project ?? '',
channelID: currentChannelId ?? '',
channelID: isCurrentChannelIdPresentInChannelList && currentChannelId ? currentChannelId : '',
};

if (!organization && organizationList.length === 1) {
Expand Down Expand Up @@ -453,6 +460,7 @@ const SubscribeModal = () => {
{
formFields.serviceType === pluginConstants.common.boards && formFields.eventType && Object.keys(eventTypeBoards).includes(formFields.eventType) && (
<BoardsFilter
isModalOpen={visibility}
organization={formFields.organization as string}
projectId={selectedProjectId || projectID as string}
eventType={formFields.eventType || ''}
Expand All @@ -465,6 +473,7 @@ const SubscribeModal = () => {
{
formFields.serviceType === pluginConstants.common.repos && formFields.eventType && Object.keys(eventTypeRepos).includes(formFields.eventType) && (
<ReposFilter
isModalOpen={visibility}
organization={formFields.organization as string}
projectId={selectedProjectId || projectID as string}
eventType={formFields.eventType || ''}
Expand All @@ -489,6 +498,7 @@ const SubscribeModal = () => {
{
formFields.serviceType === pluginConstants.common.pipelines && formFields.eventType && Object.keys(eventTypePipelines).includes(formFields.eventType) && (
<PipelinesFilter
isModalOpen={visibility}
organization={organization as string}
projectId={projectID as string}
eventType={formFields.eventType || ''}
Expand Down
3 changes: 3 additions & 0 deletions webapp/src/pluginConstants/messages.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,12 @@ export const error = {
accessDenied: 'Access Denied',
subscriptionNotFound: 'Requested subscription does not exist',
adminAccessError: 'Looks like you do not have access to add/delete a subscription for this project. Please make sure you are a project or team administrator for this project',
failedToGetSubscriptions: 'Failed to get the subscription filter values',

// Link
notAccessibleError: 'Looks like this project/organization does not exist or you do not have permissions to access it',
adminAccessErrorForUnlinking: 'You do not have sufficient permissions to delete subscriptions for this project but you can still unlink the project',
projectAlreadyLinkedError: 'This project is already linked.',
errorExpectedForOAuthNotEnabled: 'failed to link Project: status: 401 Unauthorized',
errorMessageOAuthNotEnabled: 'Looks like "third-party application access via OAuth" setting is not enabled for the organization or you do not have sufficient permissions to access it',
};
12 changes: 9 additions & 3 deletions webapp/src/utils/errorHandling.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,19 @@ const getErrorMessage = (
if (errorState?.status === 403 && errorState?.data.Error.includes(pluginConstants.messages.error.accessDenied)) {
return pluginConstants.messages.error.adminAccessError;
}
return pluginConstants.messages.error.generic;
if (errorState?.status === 500 && errorState?.data.Error.includes(pluginConstants.messages.error.failedToGetSubscriptions)) {
return pluginConstants.messages.error.failedToGetSubscriptions;
}
return errorState?.data.Error ?? pluginConstants.messages.error.generic;

case 'LinkProjectModal':
if (errorState?.status === 404 || errorState?.status === 401) {
return pluginConstants.messages.error.notAccessibleError;
}
return pluginConstants.messages.error.generic;
if (errorState?.status === 500 && errorState?.data.Error.includes(pluginConstants.messages.error.errorExpectedForOAuthNotEnabled)) {
return pluginConstants.messages.error.errorMessageOAuthNotEnabled;
}
return errorState?.data.Error ?? pluginConstants.messages.error.generic;

case 'ConfirmationModal':
if (errorState?.status === 403 && errorState?.data.Error.includes(pluginConstants.messages.error.accessDenied)) {
Expand All @@ -35,7 +41,7 @@ const getErrorMessage = (
return pluginConstants.messages.error.generic;

default:
return pluginConstants.messages.error.generic;
return errorState?.data.Error ?? pluginConstants.messages.error.generic;
}
};

Expand Down

0 comments on commit 72424ce

Please sign in to comment.