Skip to content

Commit f598a54

Browse files
authored
error handling when renaming engagement. Remove edit message for read… (#607)
* error handling when renaming engagement. Remove edit message for read only user * fix new engagement exception. Cleanup bits * move to node 14
1 parent f3aafa0 commit f598a54

File tree

8 files changed

+88
-78
lines changed

8 files changed

+88
-78
lines changed

.github/workflows/pull-request-checks.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ jobs:
3535
runs-on: ubuntu-latest
3636
strategy:
3737
matrix:
38-
node-version: [12.x]
38+
node-version: [14.x]
3939
steps:
4040
- name: Download build artifact
4141
uses: actions/download-artifact@v2

src/components/engagement_data_cards/__tests__/__snapshots__/data_cards.spec.tsx.snap

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -856,11 +856,7 @@ Object {
856856
</h5>
857857
<div
858858
class="pf-c-empty-state__body"
859-
>
860-
<p>
861-
Click the 'Edit' button, to begin adding point of contacts
862-
</p>
863-
</div>
859+
/>
864860
</div>
865861
</div>
866862
</div>
@@ -938,11 +934,7 @@ Object {
938934
</h5>
939935
<div
940936
class="pf-c-empty-state__body"
941-
>
942-
<p>
943-
Click the 'Edit' button, to begin adding point of contacts
944-
</p>
945-
</div>
937+
/>
946938
</div>
947939
</div>
948940
</div>

src/components/engagement_data_cards/engagement_summary_card/engagement_summary_card.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,7 @@ export function EngagementSummaryCard() {
150150
<TitledDataPoint title="Use Cases" dataCy={'use_cases'}>
151151
<>
152152
{currentEngagement?.use_cases?.map?.(useCase => (
153-
<p key={useCase.id ?? useCase.description}>
153+
<p key={useCase.uuid ?? useCase.description}>
154154
{useCase.description}
155155
</p>
156156
))}

src/components/engagement_data_cards/point_of_contact_card/point_of_contact_card.tsx

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,9 @@ export function PointOfContactCard() {
101101
No Points of Contact Added
102102
</Title>
103103
<EmptyStateBody>
104-
<p>Click the 'Edit' button, to begin adding point of contacts</p>
104+
<Feature name="writer">
105+
<p>Click the 'Edit' button, to begin adding point of contacts</p>
106+
</Feature>
105107
</EmptyStateBody>
106108
</EmptyState>
107109
)}

src/context/engagement_context/engagement_context.tsx

Lines changed: 4 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import { EngagementFormConfig } from '../../schemas/engagement_config';
1111
import {
1212
AlreadyExistsError,
1313
AlreadyLaunchedError,
14+
NamingError,
1415
NotFoundError,
1516
} from '../../services/engagement_service/engagement_service_errors';
1617
import {
@@ -204,7 +205,7 @@ export const EngagementProvider = ({
204205
const hasUpdates = await _checkHasUpdateRef.current();
205206
if (hasUpdates) {
206207
feedbackContext.showAlert(
207-
'Another user edited this engagement. In order to continue, you must refresh the page. By refreshing, your unsaved changes will be overwritten."',
208+
'Another user edited this engagement. In order to continue, you must refresh the page. By refreshing, your unsaved changes will be overwritten.',
208209
AlertType.error,
209210
false,
210211
[
@@ -412,18 +413,8 @@ export const EngagementProvider = ({
412413
feedbackContext.hideLoader();
413414
let errorMessage =
414415
'There was an issue with saving your changes. Please follow up with an administrator if this continues.';
415-
if (e instanceof AlreadyExistsError) {
416-
// If there is no mongo id associated with the engagement, then it is being committed to the backend for the first time. It is a net new engagement.
417-
if (!!data.mongo_id) {
418-
errorMessage =
419-
'The path that you input is already taken. Please update and try saving again.';
420-
421-
// Otherwise, we are saving an existing engagement.
422-
} else {
423-
_handleErrors(e);
424-
errorMessage =
425-
'Your changes could not be saved. Another user has modified this data. Please refresh your data in order to make changes.';
426-
}
416+
if (e instanceof AlreadyExistsError || e instanceof NamingError) {
417+
errorMessage = e.message;
427418
}
428419
feedbackContext.showAlert(errorMessage, AlertType.error);
429420
}
@@ -432,7 +423,6 @@ export const EngagementProvider = ({
432423
feedbackContext,
433424
currentEngagementChanges,
434425
engagementService,
435-
_handleErrors,
436426
changedGroups,
437427
setCurrentEngagement,
438428
]

src/packages/api_v1_sdk/apiv1_engagement_service.ts

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import {
44
} from '../../services/engagement_service/engagement_service';
55
import { Engagement } from '../../schemas/engagement';
66
import { EngagementFormConfig } from '../../schemas/engagement_config';
7-
import { AlreadyExistsError } from '../../services/engagement_service/engagement_service_errors';
7+
import { AlreadyExistsError, NamingError } from '../../services/engagement_service/engagement_service_errors';
88
import { EngagementJsonSerializer } from '../../serializers/engagement/engagement_json_serializer';
99
import { Logger } from '../../utilities/logger';
1010
import { handleAxiosResponseErrors } from './http_error_handlers';
@@ -17,13 +17,15 @@ export class Apiv1EngagementService implements EngagementService {
1717
this.axios = getApiV1HttpClient();
1818
this.getEngagementById = this.getEngagementById.bind(this);
1919
}
20+
2021
async checkSubdomainUniqueness(subdomain: string): Promise<boolean> {
2122
return this.axios
2223
.head(`/engagements/subdomain/${subdomain}`)
2324
.then(() => true)
2425
.catch(() => false);
2526
}
2627
private static engagementSerializer = new EngagementJsonSerializer();
28+
2729
private buildQueryStringFromParameters(
2830
parameters: EngagementSearchParameters
2931
): string {
@@ -115,7 +117,7 @@ export class Apiv1EngagementService implements EngagementService {
115117
Logger.instance.error(e);
116118
if (e.response.status === 409) {
117119
throw new AlreadyExistsError(
118-
'A project with this customer name and project name already exists'
120+
'A project with this customer name and project name already exists 👀'
119121
);
120122
}
121123
if (e.isAxiosError) {
@@ -142,6 +144,24 @@ export class Apiv1EngagementService implements EngagementService {
142144
return Apiv1EngagementService.engagementSerializer.deserialize(data);
143145
} catch (e) {
144146
if (e.isAxiosError) {
147+
if(e.response.status === 400 && e.response.data["parameter_violations"]) {
148+
const errorMessage = e.response.data["parameter_violations"][0];
149+
let field = "Parameter";
150+
if(errorMessage.path === 'put.engagement.customerName') {
151+
field = "Client name";
152+
} else if(errorMessage.path === 'put.engagement.name') {
153+
field = 'Engagement name';
154+
}
155+
throw new NamingError(
156+
`${field} value ${errorMessage.value} is invalid. 😔 ${errorMessage.message}`
157+
);
158+
}
159+
160+
if (e.response.status === 409) {
161+
throw new AlreadyExistsError(
162+
'A project with this customer name and project name already exists 👀'
163+
);
164+
}
145165
handleAxiosResponseErrors(e);
146166
} else {
147167
throw e;

src/routes/create_new_engagement/create_new_engagement.tsx

Lines changed: 54 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -78,59 +78,64 @@ export function CreateNewEngagementForm() {
7878
}, [selectedProjectNameToFind, engagements]);
7979

8080
const submitNewEngagement = async () => {
81-
if (customerName && projectName) {
82-
if (selectedProjectNameToFind) {
83-
const result = await createEngagement({
84-
customer_name: customerName,
85-
project_name: projectName,
86-
engagement_region: region,
87-
engagement_type:
88-
engagementType ??
89-
engagementFormConfig?.basic_information?.engagement_types?.options?.find?.(
90-
e => e.default
91-
)?.value,
92-
engagement_categories: copiedEngagement?.engagement_categories,
93-
hosting_environments: copiedEngagement?.hosting_environments.map(
94-
hosting_env => {
95-
return {
96-
ocp_cloud_provider_name: hosting_env.ocp_cloud_provider_name,
97-
ocp_cloud_provider_region:
98-
hosting_env.ocp_cloud_provider_region,
99-
ocp_cluster_size: hosting_env.ocp_cluster_size,
100-
ocp_persistent_storage_size:
101-
hosting_env.ocp_persistent_storage_size,
102-
ocp_version: hosting_env.ocp_version,
103-
};
104-
}
105-
),
81+
try {
82+
if (customerName && projectName) {
83+
if (selectedProjectNameToFind) {
84+
const result = await createEngagement({
85+
customer_name: customerName,
86+
project_name: projectName,
87+
engagement_region: region,
88+
engagement_type:
89+
engagementType ??
90+
engagementFormConfig?.basic_information?.engagement_types?.options?.find?.(
91+
e => e.default
92+
)?.value,
93+
engagement_categories: copiedEngagement?.engagement_categories,
94+
hosting_environments: copiedEngagement?.hosting_environments.map(
95+
hosting_env => {
96+
return {
97+
ocp_cloud_provider_name: hosting_env.ocp_cloud_provider_name,
98+
ocp_cloud_provider_region:
99+
hosting_env.ocp_cloud_provider_region,
100+
ocp_cluster_size: hosting_env.ocp_cluster_size,
101+
ocp_persistent_storage_size:
102+
hosting_env.ocp_persistent_storage_size,
103+
ocp_version: hosting_env.ocp_version,
104+
};
105+
}
106+
),
107+
});
108+
uuid = result.uuid;
109+
} else {
110+
const result = await createEngagement({
111+
customer_name: customerName,
112+
project_name: projectName,
113+
engagement_region: region,
114+
engagement_type:
115+
engagementType ??
116+
engagementFormConfig?.basic_information?.engagement_types?.options?.find?.(
117+
e => e.default
118+
)?.value,
119+
timezone: getTimeZone(),
120+
});
121+
uuid = result.uuid;
122+
}
123+
// want to inject uuid from the post return here
124+
logEvent({
125+
action: 'Create New Engagement',
126+
category: AnalyticsCategory.engagements,
106127
});
107-
uuid = result.uuid;
108-
} else {
109-
const result = await createEngagement({
110-
customer_name: customerName,
111-
project_name: projectName,
112-
engagement_region: region,
113-
engagement_type:
114-
engagementType ??
115-
engagementFormConfig?.basic_information?.engagement_types?.options?.find?.(
116-
e => e.default
117-
)?.value,
118-
timezone: getTimeZone(),
128+
129+
history.push({
130+
pathname: `/app/engagements/${uuid}`,
119131
});
120-
uuid = result.uuid;
132+
} else {
133+
history.push('/app/engagements');
121134
}
122-
// want to inject uuid from the post return here
123-
logEvent({
124-
action: 'Create New Engagement',
125-
category: AnalyticsCategory.engagements,
126-
});
127-
128-
history.push({
129-
pathname: `/app/engagements/${uuid}`,
130-
});
131-
} else {
132-
history.push('/app/engagements');
135+
} catch(e) {
136+
console.log('something went wrong while creating the engagement', e);
133137
}
138+
134139
};
135140

136141
const [hasFetchedEngagements, setHasFetchedEngagements] = useState<boolean>(
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
11
export class AlreadyExistsError extends Error {}
2+
export class NamingError extends Error {}
23
export class AlreadyLaunchedError extends Error {}
34
export class NotFoundError extends Error {}

0 commit comments

Comments
 (0)