reference modif actions - #869
Conversation
Signed-off-by: SOUISSI Maissa (Externe) <souissimai@gm0winl878.bureau.si.interne>
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthrough
ChangesContainer resolution
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
Warning Your free Security trial is over. An organization admin can activate billing to continue. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Signed-off-by: SOUISSI Maissa (Externe) <souissimai@gm0winl878.bureau.si.interne>
Signed-off-by: SOUISSI Maissa (Externe) <souissimai@gm0winl878.bureau.si.interne>
|
Warning Your free Security trial is over. An organization admin can activate billing to continue. |
This comment was marked as resolved.
This comment was marked as resolved.
| * @param modificationUuid uuid of the modification-reference itself | ||
| * @param referenceId uuid of the referenced shared composite in directory-server | ||
| * @param containerId uuid of the composite containing the reference, null if the modification-reference is at the root level | ||
| * @author RTE |
| /** | ||
| * A COMPOSITE container may be designated through a modification reference: in that case the actual | ||
| * container is the shared composite the reference points to. | ||
| */ | ||
| private UUID resolveContainerId(ModificationContainerInfos containerInfos) { | ||
| if (ModificationContainerType.COMPOSITE.equals(containerInfos.type())) { | ||
| return modificationRepository.findById(containerInfos.id()) | ||
| .filter(ModificationReferenceEntity.class::isInstance) | ||
| .map(entity -> ((ModificationReferenceEntity) entity).getReferenceId()) | ||
| .orElse(containerInfos.id()); | ||
| } | ||
| return containerInfos.id(); | ||
| } |
There was a problem hiding this comment.
I don't think that this function is useful.
containerInfos.id() should never be a reference modification uuid, it should be the uuid of the real container, that is the uuid of the composite toward which the reference is pointing.
Therefore .filter(ModificationReferenceEntity.class::isInstance) should always return false. Or Something strange is in the container table.
There was a problem hiding this comment.
Ok I misunderstood : resolveContainerId allows to get the composite uuid container from the reference modification uuid before geting it from the container repository :
AbstractModificationContainerEntity containerEntity = modificationContainerRepository.findById(containerId)
So the reference is never a container in the modificationContainerRepository but it can be one in the dto.
This is a bit hard to follow. Maybe we could use different terms, but given that this is going to be refactored, we may improve this later.
There was a problem hiding this comment.
Just to make it even clearer :
| /** | |
| * A COMPOSITE container may be designated through a modification reference: in that case the actual | |
| * container is the shared composite the reference points to. | |
| */ | |
| private UUID resolveContainerId(ModificationContainerInfos containerInfos) { | |
| if (ModificationContainerType.COMPOSITE.equals(containerInfos.type())) { | |
| return modificationRepository.findById(containerInfos.id()) | |
| .filter(ModificationReferenceEntity.class::isInstance) | |
| .map(entity -> ((ModificationReferenceEntity) entity).getReferenceId()) | |
| .orElse(containerInfos.id()); | |
| } | |
| return containerInfos.id(); | |
| } | |
| /** | |
| * A COMPOSITE container may be designated through a modification reference, in the ModificationContainerInfos dto: | |
| * in that case the actual container is the shared composite the reference points to. | |
| */ | |
| private UUID resolveContainerId(ModificationContainerInfos containerInfos) { | |
| if (ModificationContainerType.COMPOSITE.equals(containerInfos.type())) { | |
| return modificationRepository.findById(containerInfos.id()) | |
| .filter(ModificationReferenceEntity.class::isInstance) | |
| .map(entity -> ((ModificationReferenceEntity) entity).getReferenceId()) | |
| .orElse(containerInfos.id()); | |
| } | |
| return containerInfos.id(); | |
| } |
But I really think that we should add a "TODO" about this because we are forcing one more request for all the composites when it is not really needed : the container may be typed as REFERENCE in the dto, or the actual shared composite container may be determined in the front if we add the data in the front...
There was a problem hiding this comment.
right, this should be simplified later
Signed-off-by: SOUISSI Maissa (Externe) <souissimai@gm0winl878.bureau.si.interne>
|
2 participants
PR Summary