Fix: enum serialization

[Kindest13]

Fixes microsoft/kiota-typescript#1276

[baywet]

Thanks for the contribution!
In the issue you were mentioning that you're having issue implementing a unit test.
Can you please also include the draft unit test in this pull request?

[Kindest13]

Thanks for the contribution! In the issue you were mentioning that you're having issue implementing a unit test. Can you please also include the draft unit test in this pull request?

Hey @baywet
I was debugging existing test and wanted to update its assertion (to remove spread operator)
line 1104 WritesConstructorWithEnumValueAsync in CodeFunctionWriterTests.cs
But seems like we pass enum but it's not recognized as one

[baywet]

@Kindest13 I've just pushed an example unit test. I think the changes are getting us to the right direction but are not enough yet.
There are effectively 3 different enum cases:

• single value => we already have what we need in place end to end.
• flaggable => I think this is why we originally introduced the spread operator. What I don't know is how anybody would set those properties/if we need to correct the generation for those kinds of properties. I think we'd need to default to an array which will be serialized as CSV since there's no flag or enum set notion in TypeScript
• collection of enum values => here we're most likely missing a method in the kiota-typescript SerializationWriter and ParseNode interfaces. If you want you can go ahead and start a pull request over there as well.

Regardless, all methods need to ALSO accept the serialization object so we can map enum members to their serialization representations, which is not implemented today.

Let us know if you have any additional comments or questions.

I'd also like @andrueastman opinion on this one.

[andrueastman]

flaggable => I think this is why we originally introduced the spread operator. What I don't know is how anybody would set those properties/if we need to correct the generation for those kinds of properties. I think we'd need to default to an array which will be serialized as CSV since there's no flag or enum set notion in TypeScript

I think this also what we were trying to solve with the x-ms-enums-flags extension with the idea of having also specify the serialization format.
This way a language like TS could specify the enum in both case 2 and 3 as a collection of enum values then the serializer would do the job knowing what to write (csv string or collection of strings) using the type specified.

I agree with making the property default as a collection for now. That requires us to pass extra information to the serializer to differentiate the case 2 and 3. But that is solved with the separate methods for now (once we add the method for collection of enum values).

Similar to GO we could also generate the string conversion function for the enum but that would lead to a bigger footprint in generation.

[baywet]

Thank you for the additional information @andrueastman
The extension itself is for future work, and likely a source breaking change to coordinate across all languages.

If we focused on "this version of the generation across all languages" I believe we could "align" TypeScript with the changes I've described in my previous reply. Anything I might have missed? Or issues? In your opinion.

[baywet]

gentle reminder on the last question @andrueastman

[Kindest13]

@baywet Do we still deciding on the approach?

[baywet]

Thank you for your patience, I'll try to recap the situation the best I can so the next steps are clear.

Here is what I believe we should have at the generation level:

• enum, single value => writeEnumValue, no spread operator
• enum, array => probably missing a method in kiota typescript
• enum, flaggable => writeEnumValue, spread operator (we had that, but this PR removes it)

At the serialization libraries level, during serialization:

• enum, single value: writeEnumValue => this ultimately ends up doing string interpolation This should work since we're ultimately passing the constant at runtime, but we should validate this.
• enum, array => I believe we're missing a method here
• enum, flaggable: writeEnumValue with spread operator => should work today (assuming the first one works)

At the serialization libraries level, during deserialization:

• enum, single value: getEnumValue => should work as is since we're passing the mapping object
• enum, array: getEnumCollectionValue => supports my case about a method missing for serialization
• enum, flaggable => I believe we're missing a method, or at least the implementation of the collection method should split strings.

With all that in mind, here is how I suggest we proceed:

1. Add missing methods in the serialization libraries, and test cases that demonstrate the different scenarios.
2. Update the generation to match the different cases based on the updated in the serialization library.

Let us know if you have any additional comments or questions. And thanks for the nudge here.

[andrueastman]

This looks good from my end @baywet.

enum, flaggable => I believe we're missing a method, or at least the implementation of the collection method should split strings.

I think for this one we can simply have the implementation split the strings first. Adding another method would probably have something like getCollectionOfFlaggedEnums which would give the notion that we would need something similar on the serializer side. Also, I'd argue this is okay as we would not expect collections of flagged enums so this would be only happening at the first layer/dimension.

In summary we should have something like this.

	Enum - No Flags	Enum - With Flags	Enum Collection - No Flags	Enum Collection - With Flags
Serialization	writeEnumValue	writeEnumValue	writeCollectionOfEnumValue (TODO as @baywet highlighted)	N/A(case doesn't make sense more info here)
Deserialization	getEnumValue	getCollectionOfEnumValue (TODO update the implementation to split the node before getting underlying enums)	getCollectionOfEnumValue	N/A(case doesn't make sense more info here)

[baywet]

Thanks for chiming in @andrueastman
@Kindest13 do you want to start sending PRs on kiota typescript?
Let us know if you have any additional comments or questions.