Potential fix for Issue 93

[rkuchler]

Update HasValue logic for Filefield to accomodate images inserted from Content Hub

[rkuchler]

Thanks for the reply, Ben. Arg, I forgot to update the tests!

Here are a couple of screenshots of the debugger showing the value of the InnerField.Value property for each type of image:

Media Library Image:

Content Hub Image:

You'll see that media library images just have mediaid attribute with an id to the image item in the media library whereas Content Hub images have a src attribute specifying the URL of that image in the Content Hub.

If I can provide any other info please let me know.

Thanks again, Ryan

On Tue, May 25, 2021 at 10:39 AM Ben Lipson @.***> wrote:

@.**** commented on this pull request.

In Source/Synthesis/FieldTypes/FileField.cs https://github.com/blipson89/Synthesis/pull/94#discussion_r638918535:

@@ -93,8 +93,8 @@ public override bool HasValue { get {

• if (InnerField == null) return false;
• return !(MediaItemId == (ID)null || MediaItemId.IsNull);
• if (InnerField == null) return false;
• return !(string.IsNullOrEmpty(InnerField.Value));

Hi @rkuchler https://github.com/rkuchler - this change looks like it'd make sense to me. What sort of value would ContentHub be putting in this field? Is it a URL?

I want to add an example to my testing suite.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/blipson89/Synthesis/pull/94#pullrequestreview-667992649, or unsubscribe https://github.com/notifications/unsubscribe-auth/AANZJJQPASUXVQCRHQHHBMTTPPAD7ANCNFSM45J32JMA .

[rkuchler]

@blipson89 just checking in to see if you had everything you needed from me. Our clients are anxious to get this fix in so I wanted to see if you had a timeline for releasing a new nuget package. If it looks like it will still be awhile out I can create a custom build for them. Thanks!

[blipson89]

Apologies for the delay! The screenshots actually didn't come through. The change seems simple enough though. I was going to batch it up with a larger release but I don't have a timeline for that.

Given how small of a change this is, I can separate this from the larger release and very likely get a release out next week.

[rkuchler]

Sorry about that. I updated the previous comment to reload the screen shots. I really appreciate you considering pulling this out into a smaller release. If there's anything I can do to help please let me know.

[blipson89]

Hey @rkuchler - I don't actually have access to a ContentHub instance to test against. I'm going to make a few changes to better support CH and ensure safe backwards compatibility for non-CH.

Would you be willing to test the changes for me on the ContentHub side?

Also - is your solution using Leprechaun or Synthesis for code generation?

[blipson89]

@rkuchler I moved your branch over to Synthesis' repo and pushed up some changes. Can you try pulling down bugfix/issue-93 from the main Synthesis repo and giving it a whirl?

Basically, I created a new field type called ContentHubImageField and brought your HasValue changes down into it, along with a few other helpers.

There's a new config patch file in the Standard Config Files called Synthesis.ContentHub.config.disabled. You'll have to un-disable that and drop it into your project.

If you're using Leprechaun, you're going to want to tweak the Synthesis.csx file, line 153 to be ContentHubImageField.

If you wouldn't mind testing it out, can you please confirm the following:

1. It generates models properly...
   • if using Leprechaun, please see my note above about the csx file
   • if using Synthesis, please see my note above the config patch
2. The field works appropriately in whatever situation you were using it
3. The new attributes I added (ContentId, ThumbnailSrc, and ContentType) populate correctly

---

as a side note:

While Synthesis supports Content Hub, I can't offer full support since I don't have access to a CH instance myself. So if an issue is reported involving Content Hub, I will still work with the reporter to get the issue resolved. I just can't pro-actively ensure or guarantee compatibility.

[rkuchler]

Thanks so much, Ben. We are using Synthesis for code gen. I'll pull down this branch and test out the changes. I'll follow up the results later today. I appreciate your assistance in getting this resolved for us.

On Sat, Jun 5, 2021 at 2:05 PM Ben Lipson @.***> wrote:

@rkuchler https://github.com/rkuchler I moved your branch over to Synthesis' repo and pushed up some changes. Can you try pulling down bugfix/issue-93 from the main Synthesis repo and giving it a whirl?

Basically, I created a new field type called ContentHubImageField and brought your HasValue changes down into it, along with a few other helpers.

There's a new config patch file in the Standard Config Files called Synthesis.ContentHub.config.disabled https://github.com/blipson89/Synthesis/blob/bugfix/issue-93/Source/Synthesis/Standard%20Config%20Files/Synthesis.ContentHub.config.disabled. You'll have to un-disable that and drop it into your project.

If you're using Leprechaun, you're going to want to tweak the Synthesis.csx file, line 153 https://github.com/blipson89/Leprechaun/blob/1.1.1/src/Leprechaun.CodeGen.Roslyn/Scripts/Synthesis.csx#L153 to be ContentHubImageField.

If you wouldn't mind testing it out, can you please confirm the following:

1. It generates models properly...
   • if using Leprechaun, please see my note above about the csx file
   • if using Synthesis, please see my note above the config patch
2. The field works appropriately in whatever situation you were using it
3. The new attributes I added (ContentId, ThumbnailSrc, and ContentType) populate correctly

---

as a side note:

While Synthesis supports Content Hub, I can't offer full support since I don't have access to a CH instance myself. So if an issue is reported involving Content Hub, I will still work with the reporter to get the issue resolved. I just can't pro-actively ensure or guarantee compatibility.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/blipson89/Synthesis/pull/94#issuecomment-855282454, or unsubscribe https://github.com/notifications/unsubscribe-auth/AANZJJVEHM7ODEA5N522WSLTRJYOTANCNFSM45J32JMA .

[rkuchler]

@blipson89 I did a little testing with the Synthesis.ContentHub.config file enabled but it looks like the breakpoints in ContentHubImageField.cs aren't being hit. All images (whether from CH or media library) arestill only hitting the breakpoints in FileField.cs. I tried moving the map for ContentHubFileField above FileField in configuration but that didn't seem to help. I guess I'm a little unclear on how Synthsis determines which file type to use.

[blipson89]

Hmm. So when you regenerate the models, what is the property type of the field it generates? Is it an ImageField (i.e. public ImageField MyImage { get; set; })?

Synthesis determines the field type based off the fieldMappings element. If you go to https://<YOURDOMAIN>/sitecore/admin/showconfig.aspx and search for fieldMappings, does the mapping for Image correctly show the ContentHub field?

[rkuchler]

@blipson89 - that was my fault - I neglected to rebuild the models with the updated dll. My apologies. After rebuilding I was able to see images from both CH and media library load. One last thing, can we add a ContentHubImageFieldTests class to Synthesis.Tests project?

[blipson89]

@rkuchler well, to be fair, I gave a whole bunch of detailed instructions but forgot to mention you'd need to rebuild the models - so it's somewhat on me too :)

regarding tests: I was going to add it after I checked with you to make sure it actually worked. If you'd like to add it yourself, please be my guest. Since you've confirmed everything works, I'll plan on releasing this tomorrow.

[rkuchler]

Thanks so much. I'll add in the tests and submit another PR. Our client will be extremely happy to get the new nuget package this week!

On Tue, Jun 8, 2021 at 11:59 AM Ben Lipson @.***> wrote:

@rkuchler https://github.com/rkuchler well, to be fair, I gave a whole bunch of detailed instructions but forgot to mention you'd need to rebuild the models.

regarding tests: I was going to add it after I checked with you to make sure it actually worked. If you'd like to add it yourself, please be my guest. Since you've confirmed everything works, I'll plan on releasing this tomorrow.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/blipson89/Synthesis/pull/94#issuecomment-856937412, or unsubscribe https://github.com/notifications/unsubscribe-auth/AANZJJRY5WPE6SFB5MWMZA3TRZEAZANCNFSM45J32JMA .

[blipson89]

@rkuchler I cherry picked your changes in due to a slight branching issue I was having on my end. I'm going to close this PR. If we need to discuss further or continue troubleshooting, let's discuss it within issue #93