DavidRouyer / pipedrive-dotnet

Pipedrive.net is an async .NET Standard client for pipedrive.com
MIT License
38 stars 46 forks source link

order_nr is null for OrganisationFields #141

Open dombarnes opened 1 year ago

dombarnes commented 1 year ago

Describe the bug Just today I started getting some errors on using OrganisationFields.GetAll()

One or more errors occurred. (Error converting value {null} to type 'System.Int64'. Path 'data[22].order_nr', line 1, position 12274.) Error converting value {null} to type 'System.Int64'. Path 'data[22].order_nr', line 1, position 12274. Null object cannot be converted to a value type. Using PostMan to inspect the API, this API call pulls all our custom fields but also includes a bunch of standard items now. E.g.

{
            "id": null,
            "key": "address_formatted_address",
            "name": "Full/combined address",
            "order_nr": null,
            "field_type": "varchar",
            "json_column_flag": false,
            "add_time": null,
            "update_time": null,
            "last_updated_by_user_id": null,
            "edit_flag": false,
            "details_visible_flag": null,
            "add_visible_flag": null,
            "important_flag": null,
            "bulk_edit_allowed": null,
            "filtering_allowed": null,
            "sortable_flag": null,
            "mandatory_flag": true,
            "parent_id": 4021,
            "id_suffix": "formatted_address",
            "active_flag": true,
            "is_subfield": true
        },

Stack Trace:

System.AggregateException: One or more errors occurred. (Error converting value {null} to type 'System.Int64'. Path 'data[22].order_nr', line 1, position 12274.)
 ---> Newtonsoft.Json.JsonSerializationException: Error converting value {null} to type 'System.Int64'. Path 'data[22].order_nr', line 1, position 12274.
 ---> System.InvalidCastException: Null object cannot be converted to a value type.
   at System.Convert.ChangeType(Object value, Type conversionType, IFormatProvider provider)
   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.EnsureType(JsonReader reader, Object value, CultureInfo culture, JsonContract contract, Type targetType)
   --- End of inner exception stack trace ---
   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.EnsureType(JsonReader reader, Object value, CultureInfo culture, JsonContract contract, Type targetType)
   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.CreateValueInternal(JsonReader reader, Type objectType, JsonContract contract, JsonProperty member, JsonContainerContract containerContract, JsonProperty containerMember, Object existingValue)
   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.SetPropertyValue(JsonProperty property, JsonConverter propertyConverter, JsonContainerContract containerContract, JsonProperty containerProperty, JsonReader reader, Object target)
   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.PopulateObject(Object newObject, JsonReader reader, JsonObjectContract contract, JsonProperty member, String id)
   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.CreateObject(JsonReader reader, Type objectType, JsonContract contract, JsonProperty member, JsonContainerContract containerContract, JsonProperty containerMember, Object existingValue)
   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.CreateValueInternal(JsonReader reader, Type objectType, JsonContract contract, JsonProperty member, JsonContainerContract containerContract, JsonProperty containerMember, Object existingValue)
   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.PopulateList(IList list, JsonReader reader, JsonArrayContract contract, JsonProperty containerProperty, String id)
   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.CreateList(JsonReader reader, Type objectType, JsonContract contract, JsonProperty member, Object existingValue, String id)
   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.CreateValueInternal(JsonReader reader, Type objectType, JsonContract contract, JsonProperty member, JsonContainerContract containerContract, JsonProperty containerMember, Object existingValue)
   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.SetPropertyValue(JsonProperty property, JsonConverter propertyConverter, JsonContainerContract containerContract, JsonProperty containerProperty, JsonReader reader, Object target)
   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.PopulateObject(Object newObject, JsonReader reader, JsonObjectContract contract, JsonProperty member, String id)
   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.CreateObject(JsonReader reader, Type objectType, JsonContract contract, JsonProperty member, JsonContainerContract containerContract, JsonProperty containerMember, Object existingValue)
   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.CreateValueInternal(JsonReader reader, Type objectType, JsonContract contract, JsonProperty member, JsonContainerContract containerContract, JsonProperty containerMember, Object existingValue)
   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.Deserialize(JsonReader reader, Type objectType, Boolean checkAdditionalContent)
   at Newtonsoft.Json.JsonSerializer.DeserializeInternal(JsonReader reader, Type objectType)
   at Newtonsoft.Json.JsonConvert.DeserializeObject(String value, Type type, JsonSerializerSettings settings)
   at Newtonsoft.Json.JsonConvert.DeserializeObject[T](String value, JsonSerializerSettings settings)
   at Pipedrive.Internal.JsonHttpPipeline.DeserializeResponse[T](IResponse response)
   at Pipedrive.Connection.Run[T](IRequest request, CancellationToken cancellationToken)
   at Pipedrive.ApiConnection.GetPage[TU](Uri uri, IDictionary`2 parameters, String accepts, ApiOptions options)
   at Pipedrive.ApiConnection.<>c__DisplayClass16_0`1.<<GetAll>b__0>d.MoveNext()
--- End of stack trace from previous location ---
   at Pipedrive.ApiPagination.GetAllPages[T](Func`1 getFirstPage, Uri uri)
   --- End of inner exception stack trace ---
   at System.Threading.Tasks.Task.ThrowIfExceptional(Boolean includeTaskCanceledExceptions)
   at System.Threading.Tasks.Task`1.GetResultCore(Boolean waitCompletionNotification)
.....My Code

I'm not aware anything was changed on our Pipedrive account.

To Reproduce Call OrganisationFields.GetAll()

Expected behavior Valid response object

System Information .NET SDK (reflecting any global.json): Version: 6.0.202 Commit: f8a55617d2

Runtime Environment: OS Name: Mac OS X OS Version: 13.1 OS Platform: Darwin RID: osx-x64 Base Path: /usr/local/share/dotnet/sdk/6.0.202/

Pipedrive Client Package v0.5.19

gregarican commented 1 year ago

Same here for issuing the following call.

var fields = client.PersonField.GetAll().Result;

The error I get back indicates --- System.AggregateException: One or more errors occurred. (Error converting value {null} to type 'System.Int64'. Path 'data[5].order_nr', line 1, position 2646.).

I'm wondering if Pipedrive made potentially breaking changes to their API? Looking back through my e-mail I do see mention of breaking changes to the Users API endpoints. That doesn't take effect until next year, however. Browsing through their changelog I don't see anything either.

NChampion commented 1 year ago

The same applies to deals once those have been customized. That's where I started receiving errors:

https://github.com/DavidRouyer/pipedrive-dotnet/pull/139

gregarican commented 1 year ago

I have reached out to Pipedrive API support, but they are at the initial stages of asking if the API response itself offers an HTTP status code indicating an error. When the issue is the API response body now offering a null instead of a 0 value for these custom fields.

I would assume that this particular project could handle the difference by making these fields nullable. Like this below. It's not just the order_nr field, lots of others are now being returned as null values in the JSON response body. Even DateTime and Boolean.

PersonField cs

vmandic commented 1 year ago

+1, I have the same confirmation of the same behavior since since Dec 24th.

vmandic commented 1 year ago

This can be bypassed (until author releases a fix) by reusing the library integration code eg. (I needed only the two Name and Key fields, but you can create your own model to deserialize more, note that a lot of booleans are now nullable):

#nullable enable

private sealed class OrganizationFieldNameKeyOnly
{
    public string Key { get; set; } = null!;

    public string Name { get; set; } = null!;
}

var client = new PipedriveClient(...);
var apiConnection = new ApiConnection(client!.Connection);
var organizationFields = await apiConnection.GetAll<OrganizationFieldNameKeyOnly>(
    ApiUrls.OrganizationFields());
gregarican commented 1 year ago

I believe that strings in and of themselves are inherently nullable anyway, correct? There are a lot of DateTime and Boolean fields that are now nullable in the Pipedrive API response. These fields would all need to be wrapped similar to your code above. Not to mention that there are a total of 6 field classes that would require this for the workaround to be complete.

vmandic commented 1 year ago

@gregarican yes, aside booleans DateTimes also as you speak of, in my case name and key are always needed but strings can be null yes.

gregarican commented 1 year ago

@DavidRouyer when you get a chance, can you please review my pull request? https://github.com/DavidRouyer/pipedrive-dotnet/pull/143

This should fix the issue, as the Pipedrive API response now introduces a breaking change into any of these six field classes being parsed correctly. Thanks!

gregarican commented 1 year ago

I did raise this new issue to Pipedrive Support. Here was the most recent response from them. FYI.

Hi Greg

Hope you’re doing well today and that you had a nice year!

As promised, I’m coming back to you to let you know that our Engineering Team has updated me mentioning that our Developers believe to have found the cause of the issue and are currently working on fixing it.

I'll let you know as soon as there are any other developments

Thanks and have a great week, Miguel

vmandic commented 1 year ago

Update from support:

Hello and good morning Vedran, I apologise for the delay in getting back to you. Our engineering and developers team had a backlog after the holidays. As the issue with sub-fields returning null was a bit of a complicated issue, they needed some additional time. After their initial investigation, a new bug was found in the system, and it has been reported to the respective teams, and they are further investigating the issue. Once they have found the root cause of the issue, they look for potential solutions. As I understand, you have created a workaround for the issue, but we apologise for the inconvenience caused here, and we will keep you updated when the fix is deployed. And if you have any questions or concerns, kindly let us know.

Seems in progress.

gregarican commented 1 year ago

Yep sounds like they are on it. I got a similar response. The workaround of defining these various models in this project so that their elements are nullable would help eliminate this fail point in the future.

Good morning Greg

Hope you’re doing well today!

I’m coming back to you as our Engineering Team has let me know they've run some tests on our end, and this unfortunately seems to be due to a bug on our system.

I have opened up a bug report for our developers and the team will be working on resolving this as soon as possible. We appreciate your patience while we wait for an update. My apologies for any inconvenience this may have caused, and I will let you know as soon as the issue has been resolved.

Thanks and have a great day, Miguel

vmandic commented 1 year ago

Update 10min ago:

Hello, We want to inform you that our development team has finished their investigation about the issue you reported in this chat and it should now be corrected. Please feel free to do some testing in your account and, if the issue persists, please reopen this conversation and let us know if it hasn't been resolved for you. Thank you for your patience! Have a good day!

I have not retested.

dombarnes commented 1 year ago

Not from here.

{
            "id": null,
            "key": "address_street_number",
            "name": "House number",
            "order_nr": null,
            "field_type": "varchar",
            "json_column_flag": false,
            "add_time": null,
            "update_time": null,
            "last_updated_by_user_id": null,
            "edit_flag": false,
            "details_visible_flag": null,
            "add_visible_flag": null,
            "important_flag": null,
            "bulk_edit_allowed": null,
            "filtering_allowed": null,
            "sortable_flag": null,
            "mandatory_flag": true,
            "parent_id": 4021,
            "id_suffix": "street_number",
            "active_flag": true,
            "is_subfield": true
        },
gregarican commented 1 year ago

@vmandic , I received a similar e-mail from Pipedrive Support this morning. Stating that it should be resolved. Apparently not. That's why the one Pull Request I have out there should address the matter from this project's end of things. As long as those fields are defined as nullable then this quirk shouldn't break anything.

UPDATE: When I just ran a query to retrieve all PersonField values, I see that the order_nr element is no longer being passed back as null. And the various Booleans aren't being passed back as null either. The only element I see that can be passed back as null are some of the id element values. Although I honestly don't recall if this was the case last month or what.

dombarnes commented 1 year ago

Looks like I was too quick, or hit some caching thing. I tested again about an hour later and its resolved for me now.