kontent-ai / model-generator-net

Kontent.ai .NET model generator.
https://www.nuget.org/packages/Kontent.Ai.ModelGenerator
MIT License
17 stars 18 forks source link

Add the option to use KontentElementCodename instead of KontentElementId for MAPI model output #167

Closed kentico-michaelb closed 1 year ago

kentico-michaelb commented 1 year ago

Motivation

In a multi-environment project, developers will want to upsert content type changes in non-production environments via the Management API, test the newly added elements, then run the same script against production (in the event they do not want to use, or cannot use, the environment swap feature -- which is the case for some projects that cannot engage in a long content freeze or have policies against sensitive content existing in downstream environments).

The issue is that upserting these changes in content types in the different environments results in a different content type element ID for the elements, which means the developer cannot use the same models generated for the downstream (development environments) in the production environment. They need to re-run the model generator for production to ensure the KontentElementId values as shown here: https://github.com/kontent-ai/model-generator-net#management-api-example-output are valid.

Proposed solution

If there was an option to use the codename instead of the internal ID, then you could run the generator against development, and re-use the outputs in other environments, rather than having to re-run the generator per environment.

Additional context

This is in reference to a customer inquiry and I can bridge communication with them if necessary.

Sevitas commented 1 year ago

Hi, thanks for the issue, I will look into it.

Simply007 commented 1 year ago

Please sync about the actual use case.

If decided to implement the feature - try to be consistent with JS as much as possible https://github.com/kontent-ai/model-generator-js#readme

Sevitas commented 1 year ago

Hi, we have introduced KontentElementId property in the https://github.com/kontent-ai/model-generator-net/issues/112 because it can be used while working with variants language variants. Currently the codename is set to the [JsonProperty("{codename}")] attribute of the generated model, so they currently can use the codenames. We will add them a possibility to exclude id attribute.

Proposed solution

  1. include option for choosing desired identification option a. Use string values separated by ',' so we are able to use multiple options. Do not use bool flag. b. option will be used with param '-e' and '--elementreference' and have possible values 'id;codename;externalid'. Default value will be 'id;codename'. ExternalId is optional in this issue and might be implemented separately.
  2. Generate model using specified options
using System;
using Kentico.Kontent.Management.Models.Assets;
using Kentico.Kontent.Management.Models.Items;
using Kentico.Kontent.Management.Models.Items.Elements;
using Kentico.Kontent.Management.Modules.ModelBuilders;
using Newtonsoft.Json;

namespace KenticoKontentModels
{
    public partial class MyContentType
    {
        [JsonProperty("text")]
        [KontentElementId("77108990-3c30-5ffb-8dcd-8eb85fc52cb1")]
        public TextElement Text { get; set; }
    }
}
  1. Include codename in the https://github.com/kontent-ai/management-sdk-net/blob/a94d1601fc2baa7aa43b5dcffc1bf835085bf01a/Kontent.Ai.Management/Modules/Extensions/PropertyInfoExtensions.cs
  2. Edit working with variants language variants snippet

What do you think about this @kentico-michaelb?

kentico-michaelb commented 1 year ago

I missed the part where JsonProperty is referring to the codename of the element, not the codename of the element data type (which is what I assumed from the names used in the README examples):

"JsonProperty's attribute value is being generated from element codename (not from the type) and KontentElementId attribute value is element's ID."

This being said, I believe all parts of the proposal fit the use case and would be a good solution.

Sevitas commented 1 year ago

We found some issues with proposed solution. MAPI SDK maps elements on element id https://github.com/kontent-ai/management-sdk-net/blob/a94d1601fc2baa7aa43b5dcffc1bf835085bf01a/Kontent.Ai.Management/Modules/ModelBuilders/ElementModelProvider.cs#L21 We need to change this to include codename and external id as well before we release this feature.

MAPI SDK issue https://github.com/kontent-ai/management-sdk-net/issues/210

Sevitas commented 1 year ago

After further discussion, we have decided not to continue with this effort. For the complete reasoning please see https://github.com/kontent-ai/management-sdk-net/issues/210.