Skip to content

Closes #2788: Dataset metadata conditional rendering#2807

Merged
rscipien merged 8 commits into
developfrom
feature/rscipien_2788_dynamic_field
Jun 4, 2025
Merged

Closes #2788: Dataset metadata conditional rendering#2807
rscipien merged 8 commits into
developfrom
feature/rscipien_2788_dynamic_field

Conversation

@rscipien

Copy link
Copy Markdown
Contributor

No description provided.

@rscipien rscipien requested a review from lbownik May 30, 2025 05:39
@rscipien rscipien requested a review from diasf May 30, 2025 07:00
@rscipien rscipien assigned diasf and unassigned lbownik May 30, 2025
@diasf diasf assigned rscipien and unassigned diasf May 30, 2025
}

public boolean hasChangeListener(DatasetField datasetField) {
return this.conditionalRendering != null && this.conditionalRendering.controlledBy(datasetField);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that we have inputRenderersByFieldType, couldn't we detect listeners automatically?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok will change it


for (DatasetField sibling : siblingsFields) {
InputFieldRenderer renderer = inputRenderersByFieldType.get(sibling.getDatasetFieldType());
if (renderer.getConditionalRendering().isDefined()) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess all siblings should have a renderer, but there being different filters in edit/create modes, I would still check for non-null.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok

@rscipien rscipien assigned diasf and unassigned rscipien Jun 4, 2025

@diasf diasf left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good 👍 But please check that hasChangeListener is correct when taking the first best sibling before merging.


public boolean hasChangeListener(DatasetField vocabDatasetField, Map<DatasetFieldType, InputFieldRenderer> inputRenderersByFieldType) {
Optional<DatasetField> siblingField = vocabDatasetField.getDatasetFieldParent()
.getOrElseThrow(() -> new NullPointerException("datasetfield with type: " + vocabDatasetField.getTypeName()

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess we could just return false in this case.

.getDatasetFieldsChildren()
.stream()
.filter(df -> !df.getDatasetFieldType().getName().equals(vocabDatasetField.getDatasetFieldType().getName()))
.findFirst();

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why only the first sibling? Couldn't it be any of the siblings? I think you could also combine it into one chain. Something like:

    public boolean hasChangeListener(DatasetField vocabDatasetField, Map<DatasetFieldType, InputFieldRenderer> inputRenderersByFieldType) {
        String typeName = vocabDatasetField.getDatasetFieldType().getName();
        return vocabDatasetField
                .getDatasetFieldParent()
                .map(parent ->
                        parent.getDatasetFieldsChildren().stream()
                                .filter(df -> !df.getDatasetFieldType().getName().equals(typeName))
                                .map(df -> inputRenderersByFieldType.get(df.getDatasetFieldType()))
                                .anyMatch(renderer -> hasConditionalRenderingFor(typeName, renderer)))
                .getOrElse(false);
    }

    private boolean hasConditionalRenderingFor(String fieldTypeName, InputFieldRenderer renderer) {
        return Option.of(renderer)
                .flatMap(InputFieldRenderer::getConditionalRendering)
                .map(cr -> cr.getDatasetFieldName().equals(fieldTypeName))
                .getOrElse(false);
    }

Just for inspiration, don't assume correctness 😅

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks better :)

@diasf diasf assigned rscipien and unassigned diasf Jun 4, 2025
@rscipien rscipien merged commit 58a8d49 into develop Jun 4, 2025
1 check passed
@rscipien rscipien deleted the feature/rscipien_2788_dynamic_field branch June 4, 2025 07:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants