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

razor view does allow dynamic casting of property values using system.text.json #16233

Closed
giannik opened this issue Jun 4, 2024 · 21 comments · Fixed by #16292
Closed

razor view does allow dynamic casting of property values using system.text.json #16233

giannik opened this issue Jun 4, 2024 · 21 comments · Fixed by #16292
Labels
Milestone

Comments

@giannik
Copy link
Contributor

giannik commented Jun 4, 2024

Using the new system.text.json seems to not cast to dynamic in razor views , so you cannot access dynamically property values like previously .

See in below image when looping in a razor view over some content items i cannot extract dynamically the ContentItemId Property value like before. is there a way to cast to dynamic like before ?

image

image

@Piedone
Copy link
Member

Piedone commented Jun 4, 2024

Did you try foreach (dynamic faq...?

@giannik
Copy link
Contributor Author

giannik commented Jun 4, 2024

@Piedone yes i did , same result .

image

@giannik
Copy link
Contributor Author

giannik commented Jun 4, 2024

the object is there but cannot access dynamically properties using dot notation : @faq.ContentItemId

@Piedone
Copy link
Member

Piedone commented Jun 4, 2024

How should this be approached with STJ, @MikeAlhayek @sebastienros?

@gvkries
Copy link
Contributor

gvkries commented Jun 5, 2024

I would've expected ContentItems to return JsonDynamicObject or JsonDynamicArray instead of JsonObject. What's the definition of ForTutors?

@giannik
Copy link
Contributor Author

giannik commented Jun 5, 2024

@gvkries ForTutors is BagPart named 'ForTutors' with a collection of content items.

@gvkries
Copy link
Contributor

gvkries commented Jun 5, 2024

Okay, I think this happens because JsonDynamicArray only implements IEnumerable<JsonNode>, which breaks dynamic behavior when using foreach:

public class JsonDynamicArray : DynamicObject, IEnumerable<JsonNode?>

@giannik
Copy link
Contributor Author

giannik commented Jun 5, 2024

@gvkries thanks for the fix.
It partially works .It can read now a property but in the following case (where SubscriptionElements is BagPart) when it tries to read again through the .Content property it sends an error
for example

image

image

@gvkries
Copy link
Contributor

gvkries commented Jun 5, 2024

Hmm, I'll have a look...

@gvkries
Copy link
Contributor

gvkries commented Jun 5, 2024

I think it's not a ContentItem anymore at that point, you have to remove the Content part and access SubscriptionElement directly.

@giannik
Copy link
Contributor Author

giannik commented Jun 5, 2024

@gvkries I tried removing the .Content part with no luck :
image

@gvkries
Copy link
Contributor

gvkries commented Jun 5, 2024

You can cast that JsonDynamicValue to string.

Was all of this working before with NewtonSoft.Json?

@giannik
Copy link
Contributor Author

giannik commented Jun 5, 2024

@gvkries yes it was working with newtonsoft.

the issue is that even without .Content it cannot resolve using dot notation.

image

@sebastienros sebastienros added this to the 2.x milestone Jun 6, 2024
Copy link

github-actions bot commented Jun 6, 2024

We triaged this issue and set the milestone according to the priority we think is appropriate (see the docs on how we triage and prioritize issues).

This indicates when the core team may start working on it. However, if you'd like to contribute, we'd warmly welcome you to do that anytime. See our guide on contributions here.

@gvkries
Copy link
Contributor

gvkries commented Jun 7, 2024

In comparison to Newtonsoft JSON, the primary limitation when using dynamics is the lack of direct value comparison. For instance, a straightforward comparison like if (subscriptionElement.SubscriptionElement.TargetUserProfile.Text == "Trainer") no longer works out of the box. Instead, it necessitates casting to the target type, such as if ((string)subscriptionElement.SubscriptionElement.TargetUserProfile.Text == "Trainer").

Regarding the usage of .Content, it seems improbable that this would have worked with Newtonsoft JSON either. ContentItems is essentially a dynamic JSON object and not a ContentElement, which holds true when using Newtonsoft as well.

@giannik
Copy link
Contributor Author

giannik commented Jun 7, 2024

yes , you are right about the string comparison :
You need to do :

  if (subscriptionElement.SubscriptionElement.TargetUserProfile.Text.ToString() == "Trainer")  //this works 

  if (subscriptionElement.SubscriptionElement.TargetUserProfile.Text == "Trainer") //this does'nt work 

But what i am finding is that the previous api that did a 1-1 mapping of the json with api now does'nt exist :

for instance in previous api you could get strongly typed access to fields (because subscriptionElement was a contentitem) as so :

var mapsToSubscriptionPlanField = subscriptionElement.Get<ContentPart>("SubscriptionElement")?.Get<ContentPickerField>
/("MapstoSubscriptionPlan");
 if (mapsToSubscriptionPlanField?.ContentItemIds.Length == 1)
              {
                  subscriptionPriceContentItemId = mapsToSubscriptionPlanField?.ContentItemIds[0];   
              }

Now you cannot have this api access , right ? you would have to use stj helpers like json.selectNode(.....) . . correct ?

Please tell me i am wrong ...

@giannik
Copy link
Contributor Author

giannik commented Jun 7, 2024

@gvkries would it make sence to add an extension method like jsonobject.ToContentItem() to convert to contentitem and then use the same api again ?

@gvkries
Copy link
Contributor

gvkries commented Jun 7, 2024

Yes, I think that is missing and we should add support for it back. Not sure if an extension method is enough though, we may need to support this on the dynamic wrappers too.

@gvkries
Copy link
Contributor

gvkries commented Jun 10, 2024

I've read through this again, and I think the issue might be due to a change in how you're accessing the content items from the bag part. If you get the ContentItem first, you'll end up with the JSON object itself, which isn't a content item and thus can't be used with Get<> or .Content.

Instead, you need to directly access the shapes by using the .Content property of the Model. This property is a ZoneHolding, allowing you to retrieve the content items contained within the bag part.

So, instead of using:

@foreach(var subscriptionElement in Model.ContentItem.Content.SubscriptionElements.ContentItems)

use:

@foreach(var subscriptionElement in Model.Content.SubscriptionElements.ContentItems)

This will give you a ContentItem.

@gvkries
Copy link
Contributor

gvkries commented Jun 10, 2024

Here is an excellent description of the differences I was referring too:
#1448 (comment)

@giannik
Copy link
Contributor Author

giannik commented Jun 10, 2024

@gvkries thanks. the extension method also solves it too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
4 participants