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

Add GetRawValue() to return raw JSON in JsonSerializedField. #9

Merged

Conversation

jbreuer
Copy link
Contributor

@jbreuer jbreuer commented Aug 16, 2024

Description

This pull request introduces an GetRawValue() method in the JsonSerializedField class to directly return the raw JSON string. This enhancement provides a more straightforward and efficient way to access the underlying JSON without the need for additional deserialization.

Motivation

The primary motivation for this change is to simplify the process of working with JSON data, particularly in scenarios where the JSON is deserialized, modified, and then re-serialized. In my case, I needed to deserialize JSON into a SitecoreLayoutResponseContent object, apply modifications, and then serialize it back to JSON. Previously, this required a custom JsonConverter that accessed the _json field via reflection. With this improvement, such workarounds are no longer necessary, making the code cleaner and more maintainable.

@sc-ivanlieckens sc-ivanlieckens self-assigned this Aug 16, 2024
@sc-ivanlieckens sc-ivanlieckens added the enhancement New feature or request label Aug 16, 2024
@sc-ivanlieckens sc-ivanlieckens added this to the Beta Release milestone Aug 16, 2024
@jbreuer jbreuer changed the title Add ToString to return raw JSON in JsonSerializedField. Add GetRawValue to return raw JSON in JsonSerializedField. Aug 17, 2024
@jbreuer jbreuer changed the title Add GetRawValue to return raw JSON in JsonSerializedField. Add GetRawValue() to return raw JSON in JsonSerializedField. Aug 17, 2024
@sc-ivanlieckens sc-ivanlieckens dismissed their stale review August 22, 2024 19:45

Unit Test missing

@sc-ivanlieckens
Copy link
Collaborator

@jbreuer I was a bit too quick in accepting, still missing a Unit Test to cover this :)

@jbreuer
Copy link
Contributor Author

jbreuer commented Aug 23, 2024

@jbreuer I was a bit too quick in accepting, still missing a Unit Test to cover this :)

The unit test has been added.

@sc-ivanlieckens
Copy link
Collaborator

@jbreuer I believe CodeQL on your fork is disabled causing it not to run for this PR and it's blocking merging. Could you have a look?

@jbreuer
Copy link
Contributor Author

jbreuer commented Aug 28, 2024

@jbreuer I believe CodeQL on your fork is disabled causing it not to run for this PR and it's blocking merging. Could you have a look?

I've enabled CodeQL on my fork.

@sc-ivanlieckens sc-ivanlieckens merged commit a79266c into Sitecore:main Aug 29, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants