Adarshvk98 commented on code in PR #3155:
URL:
https://github.com/apache/incubator-kie-tools/pull/3155#discussion_r2106686711
##########
packages/import-java-classes-component/src/components/ImportJavaClasses/model/JavaClass.ts:
##########
@@ -26,11 +26,14 @@ export class JavaClass {
public fields: JavaField[];
/** It indicates if the fields has been loaded, in order to support empty
fields Java Classes */
public fieldsLoaded: boolean;
+ /** It indicates if there is an external conflict */
+ public isExternalConflict: boolean;
Review Comment:
I think we shouldn't add a new variable type called `isExternalConflict`
here. May be, we could create a new type that extends `JavaClass` ? This is
because this variable is only used to check for conflicts. So, instead of
modifying the existing type, let’s create a new type for conflicts and
non-conflicts, which should extend from `JavaClass`. WDYT?
##########
packages/dmn-editor/src/dataTypes/useImportJavaClasses.ts:
##########
@@ -188,21 +188,35 @@ const useImportJavaClasses = () => {
// Pre-allocate arrays to avoid resizing
const conflicts: JavaClass[] = [];
const nonConflicts: JavaClass[] = [];
+ if (!dataTypesTree || !externalModelsByNamespace) {
+ console.error("Data types or external models are undefined.");
+ return { conflicts: [], nonConflicts: [] };
+ }
// Use a traditional for loop for better performance
for (let i = 0, len = updatedJavaClasses?.length; i < len; i++) {
const javaClass = updatedJavaClasses?.[i];
const fullClassName = javaClass.name;
- if (dataTypeNames.has(fullClassName)) conflicts.push(javaClass);
- else nonConflicts.push(javaClass);
+ const isExternalConflict = dataTypesTree.some((dataType) => {
+ return (
+ dataType.namespace &&
externalModelsByNamespace[dataType.namespace] && dataType.feelName ===
fullClassName
Review Comment:
this check is being executed in every iteration. We could potentially avoid
it by moving this inside the if condition `if
(dataTypeNames.has(fullClassName))`. This way, once the class name matches,
then only need to check for conflicts with external types ? WDYT ?
##########
packages/dmn-editor/src/dataTypes/ImportJavaClasses.tsx:
##########
@@ -131,21 +136,48 @@ const ImportJavaClassNameConflictsModal = ({
variant={ModalVariant.small}
position="top"
actions={[
- <Button key="import-java-classes-conflict-btn" variant="primary"
onClick={handleActionButtonClick}>
+ <Button
+ key="import-java-classes-conflict-btn"
+ isDisabled={externalConflicts.length > 0 && action ===
JavaClassConflictOptions.REPLACE}
+ variant="primary"
+ onClick={handleActionButtonClick}
+ >
Import
</Button>,
]}
>
<TextContent>
{classNames?.length === 1 ? (
<Text component={TextVariants.p}>
- An existing DMN type named <b>{classNames?.join()}</b> has been
detected. This type is currently in use
- within the system. How would you like to proceed?
+ An existing DMN type named{" "}
+ {internalConflicts.length > 0 ?
<b>{internalConflicts[0]?.name}</b> : externalConflicts[0]?.name} has been
Review Comment:
I could see that the condition `internalConflicts.length > 0` and
`externalConflicts.length > 0` has been used multiple times. Perhaps for better
understanding, you could store this value in a variable and reuse it wherever
needed?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
