Skip to content

Commit db45783

Browse files
committed
Coderabbit suggestions
- Retain non-managed identities when updating applications - Mutation to patch then update inside the promise so if the patch fails for a single application, it will still update the remaining applications. - `toOptionLike` will allow 0 and empty string values Signed-off-by: Scott J Dickerson <[email protected]>
1 parent 35b2eec commit db45783

File tree

3 files changed

+24
-11
lines changed

3 files changed

+24
-11
lines changed

client/src/app/pages/applications/application-identity-form/application-identity-form.tsx

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ import spacing from "@patternfly/react-styles/css/utilities/Spacing/spacing";
1818

1919
import { Application, Identity, IdentityKind, Ref } from "@app/api/models";
2020
import { DEFAULT_SELECT_MAX_HEIGHT } from "@app/Constants";
21-
import { toOptionLike, toRef } from "@app/utils/model-utils";
21+
import { toOptionLike, toRef, toRefs } from "@app/utils/model-utils";
2222
import {
2323
UpdateAllApplicationsResult,
2424
useBulkPatchApplicationsMutation,
@@ -131,9 +131,23 @@ export const ApplicationIdentityForm: React.FC<
131131
toRef(identitiesByKind.asset?.find((i) => i.id === asset)),
132132
].filter(Boolean);
133133

134+
// Retain identities that aren't managed by the form
135+
const otherIdentities = applications.reduce((acc, application) => {
136+
const otherIdentities = application.direct.identities?.filter(
137+
(i) => !["source", "maven", "asset"].includes(i.kind)
138+
);
139+
if (otherIdentities?.length) {
140+
acc.set(application.id, toRefs(otherIdentities));
141+
}
142+
return acc;
143+
}, new Map<number, Ref[]>());
144+
134145
const patch = (application: Application) => {
135146
return Object.assign({}, application, {
136-
identities: updatedIdentities,
147+
identities: [
148+
...updatedIdentities,
149+
...(otherIdentities.get(application.id) ?? []),
150+
],
137151
});
138152
};
139153

client/src/app/queries/applications.ts

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -130,17 +130,14 @@ const patchAndUpdateApplications = async ({
130130
applications: Application[];
131131
patch: (application: Application) => Application;
132132
}): Promise<UpdateAllApplicationsResult> => {
133-
const patchedApplications = applications.map((application) =>
134-
patch(application)
135-
);
136-
137133
// TODO: Ideally we'd have a single request to update all applications instead of one per application.
138134
const results = await Promise.allSettled(
139-
patchedApplications.map(async (application) => {
135+
applications.map(async (application) => {
140136
try {
141-
const response = await updateApplication(application);
137+
const patchedApplication = patch(application);
138+
const response = await updateApplication(patchedApplication);
142139
return {
143-
success: { application, response },
140+
success: { application: patchedApplication, response },
144141
};
145142
} catch (error) {
146143
return {

client/src/app/utils/model-utils.tsx

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -200,10 +200,12 @@ export const getKindIdByRef = (
200200
};
201201

202202
export const toOptionLike = <T,>(
203-
value: T | undefined,
203+
value: T | undefined | null,
204204
options: OptionWithValue<T>[]
205205
) => {
206-
return value ? options.find((option) => option.value === value) : undefined;
206+
return value === null || value === undefined
207+
? undefined
208+
: options.find((option) => option.value === value);
207209
};
208210

209211
export const IssueManagerOptions: OptionWithValue<IssueManagerKind>[] = [

0 commit comments

Comments
 (0)