jsgoupil / quickbooks-sync

Sync Quickbooks Desktop
MIT License
89 stars 40 forks source link

Allow deserialization events to be passed #48

Closed dteske25 closed 4 years ago

dteske25 commented 4 years ago

In relation to #47, this was all that I was able to come up with to help resolve my issues while not impacting the overall schema validation that's already in place.

By allowing the deserialization events to be passed, consumers of the package can easily implement their own error handling for issues that occur during deserialization, rather than quietly assigning null.

https://docs.microsoft.com/en-us/dotnet/api/system.xml.serialization.xmldeserializationevents?view=netframework-4.7.2

jsgoupil commented 4 years ago

I see that could definitely help out with your case, however I think I see just a few problems

dteske25 commented 4 years ago

Really appreciate the quick feedback!

jsgoupil commented 4 years ago

I'll have another set of eyes looking at this.

However, I am not understanding your test; with or without the events, the UseTimeDataToCreatePaychecks and UseTimeDataToCreatePaychecksSpecified are the same values. -> NotSet

image

dteske25 commented 4 years ago

The test was simply showing that while some data is still coming through, the OnUnknownElement event handler is being called. I've added specific handling for my scenario, I just figured that wasn't something that belonged in the test.

jsgoupil commented 4 years ago

I will be merging today, but I will revert the WebConnector part. It doesn't work, the entry point comes from QbManager.cs. People can still be overriding the ReceiveXMLAsync if they want I guess.

jsgoupil commented 4 years ago

Merged and pushed. I did some cleanup as well. Version of QbXml is 2.3.0, will be on Nuget shortly.

dteske25 commented 4 years ago

Sounds good, really appreciate all the feedback and the quick responses!

jsgoupil commented 4 years ago

@dteske25 Thank you for contributing! Good luck with your project!