trbngr / infusio

Functional Infusionsoft API DSL for .NET
MIT License
4 stars 6 forks source link

Yay! More bad swaggerdoc enum issues :) #17

Open dan-i-am opened 6 years ago

dan-i-am commented 6 years ago

Hey Chris:

So I encountered yet another set of Field enums not being communicated down through the swaggerdoc (PR coming to fill those in).

I also encountered something that I'm not sure if you have a code-generation work-around for... I've found that the email_status field is NOT returning the value the swagger doc contract is expressing.

So for instance, your code generation is creating EmailStatus.OptOutAdmin = "Opt-Out: Admin". This is correct based on the swaggerdoc's specification and the API documentation.

But the value that's actually coming back from the API call is simply "Admin" (so FullContact fails to de-serialize).

I've discovered this through testing the various scenarios around email opt-in/out. I could add them to the EmailStatus enum, but given there's an enum already presented for these statuses (but are returning the wrong value from the API), maybe some other solution should be considered.

EDIT: next comment organizes my ramblings a little more regarding a possible solution. I did submit a PR for the missing Field enums. This is the quickest fix for email/phone/fax/address, but I found the issue for some of the missing enum values being related to a bug in the code generation parser. (ie... this problem will also emerge for the "Type" enum,)

dan-i-am commented 6 years ago

Regarding the Field enums:

I looked into this further, and it does appear that all of the Field enums are in the swagger doc. However, they are spread out through doc with relevant values sitting together (i.e. the Email structure has Field enums EMAIL1, EMAIL2, EMAIL3.. The Phone structure has PHONE1,2,3,4,5). The Address structure appears first (BILLING, SHIPPING, OTHER).

So I speculate what might be happening here is the code generator is ignoring later versions of an enum (vs appending values to it)?

Regarding the Email Status: This is a harder problem. The swaggerdoc is indeed stale to the API output. I would suggest the ability to override that property in code generation to be a string until it's sorted out. There's a companion property to that ( bool EmailOptedIn ) which summarizes in general what needs to be determined by the email status property in most cases. But as it is right now, that status enum will break de-serialization if a contact is allowed to unsubscribe from a campaign, or an admin manipulates that field in any way other than SingleOptIn.

UPDATE: It looks like the enum properties have a concept of a parent class, which implies that the "Field" and "Type" enums should probably generated as individual "sub-classes" within it's parent class vs. existing as single high-level enum types. Unfortunately, that fix wouldn't be backward compatible to anyone who's already consumed your library.

If backward compatibility is a major concern, I've written some procedural code to normalize the properties and definition (i.e. all "Field" enum values are searched and get dumped into a single master enum). I haven't had time to convert it to the functional Language-Ext (that's relatively new to me), but would be happy to share it here as an example of what needs to happen if anyone would like to take a crack at fitting it into the current functional pattern of the library.

trbngr commented 6 years ago

Thanks for digging into this, Dan.

I'm hoping to have some time this week to try and tackle it.

dan-i-am commented 6 years ago

One other thing I noticed as well is.. When adding an EmailAddress object to the contact, "Field.BILLING" is the field property setting expected, or the call fails. This isn't even a valid value for the email object. (should only allow EMAIL1, EMAIL2, or EMAIL3).

I'm wondering if the API allows for and is receiving the ordinal value of the enum (vs. the name value), in such a way that it believes ordinal 0 is actually referring to EMAIL1. (i.e. if EMAIL1, EMAIL2, EMAIL3 were isolated to a new enum like EmailAddress.Field/EmailAddressField, EMAIL1 would have ordinal 0, the same as BILLING is now in the global Field enum).

Stranger yet, when it de-serializes, it picks the right enum value (EmailAddress objects come back with Field.EMAIL1 as the "field" property created with BILLING). My guess is because it's coming back as text, which is the only way it can be parsed into resulting object.

I haven't tested this extensively, but if my speculation above is true, there's a good chance the request needs to accommodate the global enum by it's name value, or global enums are not the best solution.

dan-i-am commented 6 years ago

bump

Just wondering if this is still in view, and if there's anything I can do to assist here.

trbngr commented 6 years ago

Hey Dan.

I'm still wanting to fix this stuff. Just been slammed at work lately.

dan-i-am commented 6 years ago

No worries! I totally understand busy. Just let me know if you have a direction in mind from what I discovered and where I can assist (if I can).

Thanks!

dan-i-am commented 6 years ago

bump. nudge. etc.

trbngr commented 6 years ago

whoa. It's been awhile, huh? Sorry about that. I'm gonna try for some time this weekend.

daiplusplus commented 6 years ago

Any update @trbngr ?

Lann094 commented 5 years ago

Starting to get hit pretty hard with this issue. I'm using this in production, so if fixes aren't being worked on anymore, some handover etc would be great.

trbngr commented 5 years ago

I'm so sorry for going missing on this.

I pretty much got the finger from infusiosoft on getting a Dev account. And I no longer have a need to integrate with infusionsoft.

If you guys want to take over, you're more than welcome.

Just let me know what you need.