ach
ach copied to clipboard
moov-io
feat: support reading paymentTypeCode from JSON
It came up that TEL/WEB entry detail records have a "payment type code" field (size 1) that overlaps with the the DiscretionaryData field (size 2). This causes a bit of confusion because the JSON representation doesn't error when you provide paymentTypeCode but also doesn't populate DiscretionaryData.
This PR is an experiment to support paymentTypeCode only in the JSON level.
@atonks2 can I get your thoughts on this experiment?
Put another way, it sounds like DiscretionaryData can be used to contain either a single code, two distinct codes. In the case of WEB/TEL entries, the first position is used to hold the PaymentTypeCode.
This looks like it would give users the ability to include a paymentTypeCode field in their JSON requests even though that isn't a named field in an EntryDetail. I'm concerned this would be a hidden feature, or hidden behavior. I'm also curious about marshaling EntryDetail back into JSON. Is there a reason users can't just use the existing DiscretionaryData field as is, populating the first position with payment type code?
I agree about a hidden feature vs using the renamed field for TEL/WEB. I see both sides having a tradeoff and needing special callouts in the docs (which is done currently).
From a technical perspective, this looks good to me. My concerns are more about predictable behavior, and making the JSON equivalent in both directions. You've got much more experience than me with both ACH in general and this library, so I'm happy to leave that judgement call to you.
I don't think it's proper to introduce phantom values like this. An interesting idea, but not what we should do IMO. Feel free to comment/reply if you think this would be useful.