OwnerID doesn't support "0" value for custom fields

[tofer]

Multiple request, response, and ret types use property OwnerID, and are of type GUIDTYPE or GUIDTYPE[], but this does not support the allowed value of "0" to retrieve or set custom fields from QuickBooks. Limiting to GUIDs seems to limit compatibility with private data only.

Note: I did try new GUIDTYPE("0") but that gets formatted to {00000000-0000-0000-0000-000000000000}

I found the following quickbooks documentation in chapter 11 (page 137) for more info on the difference between custom fields and private data, and page 146 for an example query with the "0" value: https://developer-static.intuit.com/qbSDK-current/doc/PDF/QBSDK_ProGuide.pdf

I did some very limited testing, and changing OwnerID to string or string[] I was able to get custom fields successfully via the DataExtRet.

[jsgoupil]

That is the magic code. The OwnerID of zero is reserved for custom fields. If you wanted to make it private data, you would use a GUID here, like this:

Yay for magic! Everybody loves magic. The types are controlled by the XML definition. Obviously, intuit likes to not follow it. I have changed it in the past, that's where we should start. I'll see how easy this one is.

[tofer]

Leave it to Intuit, lol.

Would it be possible to add custom serialization to those properties instead, so it can serialize Guid.Empty to "0" instead of {00000000-0000-0000-0000-000000000000}?

[tofer]

OK, I did another hack that didn't require changing the property type on the XSD generated types. I only did simple testing again, but your unit tests do pass, by modifying the GUIDTYPE class with some "magic zero" handling:

public partial class GUIDTYPE : ITypeWrapper, IComparable<GUIDTYPE>, IXmlSerializable
{
	protected Guid value;

	// Support for Intuit's "magic code" for OwnerID of "0"
	private bool _magicZero;

	public GUIDTYPE()
	{
		this.value = Guid.Empty;
	}

	public GUIDTYPE(string value)
	{
		this.value = Parse(value);
		if (value == "0") _magicZero = true;
	}

	public GUIDTYPE(Guid value)
	{
		this.value = value;
	}

	public override string ToString()
	{
		if (_magicZero && value == Guid.Empty)
		{
			return "0";
		}

		return value.ToString("B", CultureInfo.InvariantCulture);
	}

	public Guid ToGuid()
	{
		return value;
	}
	
	//...
	
	public void ReadXml(System.Xml.XmlReader reader)
	{
		reader.MoveToContent();
		var isEmptyElement = reader.IsEmptyElement;
		reader.ReadStartElement();
		if (!isEmptyElement)
		{
			var str = reader.ReadContentAsString();
			_magicZero = str == "0";
			value = Parse(str);

			reader.ReadEndElement();
		}
	}
	
	//...
}

With simple usage request.OwnerID = new [] { new GUIDTYPE("0") };

In theory, this would then still allow the same functionality in all other purposes, even still allowing {00000000-0000-0000-0000-000000000000} to be parsed without then outputting "0", but I'm obviously not as familiar with the rest of your library to know what ramifications this might have.

[jsgoupil]

I think that's a good alternative, do you think you can submit a pull request with some unit tests?

[tofer]

This could arguably be simpler by always returning 0 for Guid.Empty. Do you have any insight on if that would cause issues for other uses of GUIDTYPE?

[jsgoupil]

I looked at all the usage of OwnerId and ... it's hard to tell how it should behave. Their documentation here: https://developer-static.intuit.com/qbSDK-current/Common/newOSR/index.html says

A GUIDTYPE value can be zero (0), or it can take the form {XXXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX} where X is a hexadecimal digit. For example: {6B063959-81B0-4622-85D6-F548C8CCB517}

Now technically, 00000000-0000-0000-0000-000000000000 is a valid guid. So I think we should allow it. Especially if people started to use it, then we would break them.
So your magicZero idea code is a good idea.

[tofer]

OK sounds good. I'll write up some tests and submit a pull request. Thanks for the fast responses.

[jsgoupil]

Make sure to not drop curling braces to keep consistency :)

[tofer]

Pull request #27 made

[jsgoupil]

Merged.
Thanks!