-
Notifications
You must be signed in to change notification settings - Fork 6
Handle the modification applicability per root network tag #1281
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
79738ea
b89e72d
29f58d8
851d88c
3449f87
9dac1c8
a0cf91c
9d43664
a3794c7
135df14
7695d8d
325898c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -12,51 +12,36 @@ import { updateModificationStatusByRootNetwork } from '../../../services'; | |||||
| import { useSnackMessage } from '../../../hooks'; | ||||||
| import { | ||||||
| ComposedModificationMetadata, | ||||||
| ExcludedNetworkModifications, | ||||||
| ModificationType, | ||||||
| NetworkModificationApplicabilities, | ||||||
| RootNetworkRowInfo, | ||||||
| snackWithFallback, | ||||||
| } from '../../../utils'; | ||||||
|
|
||||||
| function getUpdatedExcludedModifications( | ||||||
| prev: ExcludedNetworkModifications[], | ||||||
| rootNetworkUuid: UUID, | ||||||
| modificationUuid: UUID | ||||||
| ): { nextExcluded: ExcludedNetworkModifications[]; newStatus: boolean } { | ||||||
| const exists = prev.some((item) => item.rootNetworkUuid === rootNetworkUuid); | ||||||
|
|
||||||
| if (exists) { | ||||||
| let newStatus = false; | ||||||
| const nextExcluded = prev.map((modif) => { | ||||||
| if (modif.rootNetworkUuid !== rootNetworkUuid) { | ||||||
| return modif; | ||||||
| } | ||||||
|
|
||||||
| const isExcluded = modif.modificationUuidsToExclude.includes(modificationUuid); | ||||||
| const newModificationUuidsToExclude = isExcluded | ||||||
| ? modif.modificationUuidsToExclude.filter((id) => id !== modificationUuid) | ||||||
| : [...modif.modificationUuidsToExclude, modificationUuid]; | ||||||
|
|
||||||
| // If previously excluded, now it is activated (true), else deactivated (false) | ||||||
| newStatus = isExcluded; | ||||||
|
|
||||||
| return { | ||||||
| ...modif, | ||||||
| modificationUuidsToExclude: newModificationUuidsToExclude, | ||||||
| }; | ||||||
| }); | ||||||
| /** | ||||||
| * A modification is applicable on a root network unless its applicability for it is explicitly false: | ||||||
| * a root network without an entry is applicable. | ||||||
| */ | ||||||
| function isApplicableOn( | ||||||
| applicabilities: NetworkModificationApplicabilities, | ||||||
| modificationUuid: UUID, | ||||||
| rootNetworkUuid: UUID | ||||||
| ) { | ||||||
| return applicabilities[modificationUuid]?.[rootNetworkUuid] ?? true; | ||||||
| } | ||||||
|
|
||||||
| return { nextExcluded, newStatus }; | ||||||
| } | ||||||
| function withApplicability( | ||||||
| applicabilities: NetworkModificationApplicabilities, | ||||||
| modificationUuid: UUID, | ||||||
| rootNetworkUuid: UUID, | ||||||
| applicable: boolean | ||||||
| ): NetworkModificationApplicabilities { | ||||||
| return { | ||||||
| nextExcluded: [ | ||||||
| ...prev, | ||||||
| { | ||||||
| rootNetworkUuid, | ||||||
| modificationUuidsToExclude: [modificationUuid], | ||||||
| }, | ||||||
| ], | ||||||
| newStatus: false, | ||||||
| ...applicabilities, | ||||||
| [modificationUuid]: { | ||||||
| ...applicabilities[modificationUuid], | ||||||
| [rootNetworkUuid]: applicable, | ||||||
| }, | ||||||
| }; | ||||||
| } | ||||||
|
|
||||||
|
|
@@ -65,8 +50,8 @@ export interface RootNetworkChipCellProps { | |||||
| studyUuid: UUID | null; | ||||||
| currentNodeId?: UUID; | ||||||
| rootNetwork: RootNetworkRowInfo; | ||||||
| modificationsToExclude: ExcludedNetworkModifications[]; | ||||||
| setModificationsToExclude: React.Dispatch<SetStateAction<ExcludedNetworkModifications[]>>; | ||||||
| applicabilities: NetworkModificationApplicabilities; | ||||||
| setApplicabilities: React.Dispatch<SetStateAction<NetworkModificationApplicabilities>>; | ||||||
| isDisabled?: boolean; | ||||||
| } | ||||||
|
|
||||||
|
|
@@ -76,8 +61,8 @@ export function RootNetworkChipCell(props: RootNetworkChipCellProps) { | |||||
| studyUuid, | ||||||
| currentNodeId, | ||||||
| rootNetwork, | ||||||
| modificationsToExclude, | ||||||
| setModificationsToExclude, | ||||||
| applicabilities, | ||||||
| setApplicabilities, | ||||||
| isDisabled = false, | ||||||
| } = props; | ||||||
| const [isLoading, setIsLoading] = useState(false); | ||||||
|
|
@@ -88,17 +73,8 @@ export function RootNetworkChipCell(props: RootNetworkChipCellProps) { | |||||
| data.type === ModificationType.MODIFICATION_REFERENCE || data.childFromShared; | ||||||
|
|
||||||
| const isModificationActivated = useMemo(() => { | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this should be called "applicable" or "applied", instead of activated : to avoid any confusion with the
Suggested change
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, I did not know which name to choose...
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @coderabbitai Any suggestion ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
const isModificationApplicable = useMemo(() => {
return isApplicableOn(applicabilities, modificationUuid, rootNetwork.rootNetworkUuid);
}, [modificationUuid, applicabilities, rootNetwork.rootNetworkUuid]);
You are interacting with an AI system. |
||||||
| if (rootNetwork.isCreating) { | ||||||
| return true; | ||||||
| } | ||||||
|
|
||||||
| const excludedSet = new Set( | ||||||
| modificationsToExclude.find((item) => item.rootNetworkUuid === rootNetwork.rootNetworkUuid) | ||||||
| ?.modificationUuidsToExclude || [] | ||||||
| ); | ||||||
|
|
||||||
| return !excludedSet.has(modificationUuid); | ||||||
| }, [modificationUuid, modificationsToExclude, rootNetwork.rootNetworkUuid, rootNetwork.isCreating]); | ||||||
| return isApplicableOn(applicabilities, modificationUuid, rootNetwork.rootNetworkUuid); | ||||||
| }, [modificationUuid, applicabilities, rootNetwork.rootNetworkUuid]); | ||||||
|
|
||||||
| const handleModificationActivationByRootNetwork = useCallback(() => { | ||||||
| if (!studyUuid || !currentNodeId) { | ||||||
|
|
@@ -107,30 +83,26 @@ export function RootNetworkChipCell(props: RootNetworkChipCellProps) { | |||||
|
|
||||||
| setIsLoading(true); | ||||||
|
|
||||||
| // Compute next state (pure, no side effects) | ||||||
| const { nextExcluded, newStatus } = getUpdatedExcludedModifications( | ||||||
| modificationsToExclude, | ||||||
| rootNetwork.rootNetworkUuid, | ||||||
| modificationUuid | ||||||
| ); | ||||||
| // toggle the current applicability | ||||||
| const newApplicability = !isModificationActivated; | ||||||
|
|
||||||
| // Apply optimistic update | ||||||
| setModificationsToExclude(nextExcluded); | ||||||
| setApplicabilities((prev) => | ||||||
| withApplicability(prev, modificationUuid, rootNetwork.rootNetworkUuid, newApplicability) | ||||||
| ); | ||||||
|
|
||||||
| // Perform backend call | ||||||
| updateModificationStatusByRootNetwork( | ||||||
| studyUuid, | ||||||
| currentNodeId, | ||||||
| rootNetwork.rootNetworkUuid, | ||||||
| modificationUuid, | ||||||
| newStatus | ||||||
| newApplicability | ||||||
| ) | ||||||
| .catch((error) => { | ||||||
| // Rollback on failure by toggling back | ||||||
| setModificationsToExclude( | ||||||
| (prev) => | ||||||
| getUpdatedExcludedModifications(prev, rootNetwork.rootNetworkUuid, modificationUuid) | ||||||
| .nextExcluded | ||||||
| // Rollback on failure to the value shown when the user clicked | ||||||
| setApplicabilities((prev) => | ||||||
| withApplicability(prev, modificationUuid, rootNetwork.rootNetworkUuid, isModificationActivated) | ||||||
| ); | ||||||
| snackWithFallback(snackError, error, { headerId: 'modificationActivationByRootNetworkError' }); | ||||||
| }) | ||||||
|
|
@@ -141,9 +113,9 @@ export function RootNetworkChipCell(props: RootNetworkChipCellProps) { | |||||
| modificationUuid, | ||||||
| studyUuid, | ||||||
| currentNodeId, | ||||||
| modificationsToExclude, | ||||||
| isModificationActivated, | ||||||
| rootNetwork.rootNetworkUuid, | ||||||
| setModificationsToExclude, | ||||||
| setApplicabilities, | ||||||
| snackError, | ||||||
| ]); | ||||||
|
|
||||||
|
|
@@ -152,7 +124,7 @@ export function RootNetworkChipCell(props: RootNetworkChipCellProps) { | |||||
| label={rootNetwork.tag} | ||||||
| tooltipMessage={rootNetwork.name} | ||||||
| isActivated={isModificationActivated} | ||||||
| isDisabled={isLoading || isDisabled || isReferenceModificationOrInsideOne} | ||||||
| isDisabled={isLoading || isDisabled || isReferenceModificationOrInsideOne || rootNetwork.isCreating} | ||||||
|
Mathieu-Deharbe marked this conversation as resolved.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think that this disable when A loading mode/icon for chip will probably be needed. But it could be in a separate ticket. |
||||||
| onClick={handleModificationActivationByRootNetwork} | ||||||
| /> | ||||||
| ); | ||||||
|
|
||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function is fine but I am a bit confused by the naming. Why 'with' ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about
setApplicability?