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

Tool fails to correctly build models #66

Open
ethan-schofer opened this issue Jan 14, 2025 · 5 comments
Open

Tool fails to correctly build models #66

ethan-schofer opened this issue Jan 14, 2025 · 5 comments
Labels
bug Something isn't working

Comments

@ethan-schofer
Copy link

Describe the bug
The Optimizely Graph Client Tools, which should produce C# classes representing my models in my Graph instance is unable to handle some situations when generating models.

  1. Blocks or Pages that inherit subclasses (Other than PageData or BlockData) create models that inherit from two classes. This is not valid C# code. For example, any page model inherits from 'Content' and inherits from my 'BasePageData'. It should ONLY inherit 'BasePageData'.
public partial class BannerBlock:Content,BasePageBanner
{
   ...
  All of my properties
  ...
}
  1. Some properties are added with an exclamation point at the end of the property type name. I don't have a clear understanding of why this happens. It is invalid C#.
public partial class ImageBlock:Content
 {
     ...
    ... some properties     
     public ContentModelReference! Image { get; set; }
     ... some other properties
 }
  1. Built in property types are added to blocks or pages but they are invalid property types.
 public partial class BasePageData:Content
{
    public IEnumerable<ContentAreaItemModel> PageBanner { get; set; }
    [Searchable]
    public string PageTitle { get; set; }
    public string BodyText { get; set; }
    public LinkItemNode BodyTextCta { get; set; }
    public string BodyTextCtaAccessibilityText { get; set; }
    public IEnumerable<ContentAreaItemModel> MainContent { get; set; }
    public ContentModelReference ListingImage { get; set; }

For example, ContentAreaItemModel, LinkItemNode and ContentModelReference are all invalid property types, or at least they are not getting generated so my code base doesnt know what they are.

To Reproduce
Follow instructions from documentation: https://github.com/episerver/graph-net-sdk?tab=readme-ov-file#optimizely-graph-client-tool

  1. Create manifest: dotnet new tool-manifest
  2. Install tool: dotnet tool install Optimizely.Graph.Client.Tools --local
  3. Create models: dotnet ogschema src/MyProject/appsettings.Development.json src/MyProject/CodeGeneration

Expected behavior
Creates models for interacting with the graph from the command line.

Package (please complete the following information):

  • Optimizely.ContentGraph.Cms 3.14.3
  • Optimizely.Graph.Client 1.4.0
@ethan-schofer ethan-schofer added the bug Something isn't working label Jan 14, 2025
@ManhOptimizely
Copy link
Collaborator

ManhOptimizely commented Jan 15, 2025

Hi @ethan-schofer. Thanks for posting. For your issues:

  1. Blocks or Pages that inherit subclasses (Other than PageData or BlockData) create models that inherit from two classes => To make all content types be searchable, after your data was indexed to Graph all of your content types should inherit from Content. That's why you see your classes alway inherit from Content as first parent class. Your BasePageData is an abstract class and it not present on Graph schema, i guess.

  2. Some properties are added with an exclamation point at the end of the property type name => This may be you just enabled a flag named MarkRequiredPropertiesToGraph in Graph indexing job (Optimizely.ContentGraph.Cms 3.14.3) which is a breaking change for the tool. We will release a new version for fixing this but you should disable MarkRequiredPropertiesToGraph or degrade the Optimizely.ContentGraph.Cms package to lower version if applicable.

  3. Built in property types are added to blocks or pages but they are invalid property types => Because Graph is using Content Delivery so that your original class name was "transformed" to a different name. Example ContentAreaItemModel, LinkItemNode and ContentModelReference represent correct models on Graph schema corresponding to ContentAreaItem, LinkItem, and ContentReference on CMS. When you query ContentAreaItem on GraphiQL, for example, you will see the error that Graph did not recognize ContentAreaItem model but ContentAreaItemModel is correct. It's the same when you build query from Optimizely.Graph.Client, you should use ContentAreaItemModel for querying not ContentAreaItem.

Lets us know your if you have any further question.

@ethan-schofer
Copy link
Author

  1. Im not sure I understand your answer here. Are you saying the expectation is that I will need to manually edit the generated classes every time I generate them?
  2. This is disappointing, but I understand.
  3. Are you saying that I should not be using Optimized for delivery? I guess Im wondering why this is?

@ManhOptimizely
Copy link
Collaborator

Hi @ethan-schofer,

  1. I mean it was correct and you don't need to edit anything manually. Your Blocks or Pages should inhereit from Content class.
  2. Sorry for this, if you still use Optimizely.ContentGraph.Cms 3.14.3 you can disbale MarkRequiredPropertiesToGraph or remove exclamation mannually.
  3. Optimizely Graph was used Optimizely Content Delivery (CD) for headless CMS so that some classes in CMS were changed the model name. You can rely on generated models to build your queries, your original models not match 100% with Graph models (because of CD) but generated models do.

@ethan-schofer
Copy link
Author

Hi @ManhOptimizely

  1. What I am saying is that I DO need to change something because the generated models are incorrect. They inherit from two classes which will not compile.

  2. So Optimized for Delivery is for headless sites. If I am using the .Net client, then I do not need to Optimize for delivery?

Thanks

@ManhOptimizely
Copy link
Collaborator

Hi @ethan-schofer,

  1. Base on your example
    public partial class BannerBlock:Content,BasePageBanner { ... All of my properties ... }
    Do you have [ContentType] attribute in your BasePageBanner class or setting type convention for BannerBlock? If then please remove them and the tool should generate correct models. I admit that we should fix the tool in the next release.
  2. I understand but you can not disable Content Delivery in Optimizely.ContentGraph.Cms, it was used internally.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants