Add dedicated webservice endpoint for `FannieEquity`

[lgedgar]

for sake of adding proper equity transactions

Well despite what I said in https://github.com/CORE-POS/IS4C/pull/1189#issuecomment-1704482742 once I dug in it seemed like the final insertion logic really belonged in CORE. Since it auto-determines next trans_no and who knows what else.

So here is a first attempt at add_equity API method (based on concepts from MemEquityTransferTool). It doesn't do extensive input validation yet, and notably, it only adds 1 record to dtransactions - so it "rings up" the equity but doesn't pay for it, I guess.

In local testing, this much was sufficient for CORE to process these records as equity history, per usual AFAIK. I haven't looked at any reports yet so not sure how those might be affected. Must we add a tender record as well? Or is that just up to the bookkeeper, whether they need it shown on reports?

I feel like I still don't know what I don't know, to really get this "right" all the way around. Also not clear on the employee and register numbers, can I pass in something "made up" or must these be valid?

So am wondering if you think this new API endpoint is a good approach, and if so what it lacks. I don't mind coding but am not sure what it needs from a "native" perspective.

[lgedgar]

FYI, updated this PR to include tender, and use default emp/reg numbers. Probably going live with this soon unless anyone can tell me what's wrong with the approach. And if nothing, then hoping this can live upstream.

Also I've since discovered EquityHistoryImportPage which is similar to what I need, but avoids dtransactions and targets stockpurchases directly. Which probably would have been fine for me too..but if dtransactions is a superior approach then I'm glad to have gone down this road anyway. I still need the API no matter what it's doing under the hood.

[gohanman]

Sorry for the delay; was out of town.

This looks pretty good to me, and is probably a useful thing to have an API for. Taking equity payments through some other means seems like a common use case.

I think you're on the right track w/ adding it to dtransactions. The import was intended as a one-time thing to initialize the system.

A couple possible additions:

• Might consider letting the caller specify trans_no. If the system this data is coming from has a unique transaction/order/purchase ID, this is a decent place to put it so you can keep track of which CORE transaction corresponds to which external transaction
• Alternately, you might have an optional comment field for any kind of reference to the external system that could point to where this transaction came from specifically

[lgedgar]

Thanks, added those. I guess this is ready AFAIK other than I feel like error codes are pretty neglected. Not sure how much those matter in practice. Curious if you have thoughts about them.

[gohanman]

The spec says: https://www.jsonrpc.org/specification#error_object

Several seem like they should be 32603; the others are a bit ambiguous between internal error vs server error. Possibly it'd be good to have a transaction w/ either commit or rollback so either all INSERTs succeed or not.

[lgedgar]

Good call on transaction, added.

I guess I read that spec to say we're free to do whatever the hell we want outside of that range, as much as it is telling us how that range should be used. In which case basically I'm violating it currently by using -32000. So went ahead and changed that to use code 1 for "invalid params" and 2 for "DB error" - pretty simplistic but I don't know if it warrants sophistication at this point.