Skip to content
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

Custom Content Resolver Support #14

Merged
merged 3 commits into from
Sep 5, 2024

Conversation

sc-ivanlieckens
Copy link
Collaborator

Description / Motivation

This modifies the FieldParser to support Custom Content Resolvers returning modified JSON.
This fixes #8

Added missing CONTRIBUTING file.

Removed AddSystemTextJson which should not be used or needed as it pollutes the DI.

Testing

Added integration tests both for layout client and rendering engine binding leveraging the "Navigation" component from Headless SXA.

  • The Unit & Intergration tests are passing.
  • I have added the necesary tests to cover my changes.

Terms

@sc-ivanlieckens sc-ivanlieckens added bug Something isn't working enhancement New feature or request labels Sep 3, 2024
@sc-ivanlieckens sc-ivanlieckens added this to the Symposium Release milestone Sep 3, 2024
@sc-ivanlieckens sc-ivanlieckens self-assigned this Sep 3, 2024
@jbreuer
Copy link
Contributor

jbreuer commented Sep 4, 2024

Does the new Custom Content Resolver Support also address the issue raised in PR #12? Specifically, will it prevent the FieldConverter from breaking when encountering a field with an empty string?

@jbreuer
Copy link
Contributor

jbreuer commented Sep 5, 2024

I tested the Custom Content Resolver branch, but the empty string issue from PR #12 is still present. This happens because the FieldConverter is executed before changes in the FieldParser. Since the field is an empty string, the JsonValueKind is JsonValueKind.String, which causes the FieldConverter to throw an exception instead of handling the empty string properly.

@sc-ivanlieckens sc-ivanlieckens merged commit e0a4b12 into main Sep 5, 2024
3 checks passed
@sc-ivanlieckens sc-ivanlieckens deleted the feat/custom-content-resolver branch September 5, 2024 13:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Support for Placeholder Fields property to be an Array
4 participants