CORE-POS / IS4C

Cooperative Operational Retail Environment
http://www.core-pos.com
GNU General Public License v2.0
63 stars 44 forks source link

Allow API access to TRANS_DB #1189

Closed lgedgar closed 11 months ago

lgedgar commented 1 year ago

for models declaring that preference, e.g. Stockpurchases

Having to sync equity payments between two systems and first step is just to read them from CORE. I believe this is a sane change but not real sure of any implications beyond the obvious, i.e. it does grant access to "trans" DB, which is the intent. But there may be security/performance or other concerns I'm not aware of yet.

lgedgar commented 1 year ago

I see now that this PR so far would only let me read Stockpurchases records, but not e.g. create new ones. Due to the logic in FannieEntity->canIncrementInsert()

Not sure yet what should happen next. Need to discuss more on our end I'm sure. You have a preference between improving the API vs. working around it with direct SQL? I'd prefer the API, all else being equal, but perhaps not if it's a heavy lift. E.g. the "problem" here really is more about the Stockpurchases schema which may be a whole rabbit trail..?

gohanman commented 1 year ago

At a glance, it doesn't seem like an issue to add an incremented PK to the table. The only potential gotcha is if an external transaction overlaps with an internal one - meaning matches tdate, trans_num, card_no, dept - then the internal one won't import, but that seems fairly unlikely.

Depending what you're aiming for, I wonder if it'd make more sense to create an equity transactions in dtransactions, probably w/ a different store_id, and let that populate stockpurchases.

lgedgar commented 1 year ago

I wondered about that too, writing a "normal" transaction and letting CORE process it "normally" etc. Hadn't looked into it yet though. Would you see that happening via the API too? So, read from stockpurchases and write to dtransactions both via the API.

Glancing at dtransactions it does seem that store_row_id is autoincrement PK so probably the API should work. Any pointers to what all info is required for these records is appreciated. E.g. my assumption would be that more than one record might be required for a "complete" transaction. But I haven't dug in yet, answers may be obvious.

gohanman commented 1 year ago

It's autoincrement but not a PK. It might be possible to have a separate category for insert-only tables. There's a possible ambiguity if you send a request and don't get a response, but that might or might not be a practical problem depending how network distant the sync process is.

I'd think a complete transaction would be one or more open department ring records for equity (potentially more than one if there are different classes of equity), and then one tender record for however payment was taken.

lgedgar commented 12 months ago

Finally tested a basic POST to the API for DTransactionsModel but it didn't work. I couldn't figure out why the checks for isUnique() and canIncrementInsert() were both failing. I even temporarily defined store_row_id with 'primary_key'=>true but it didn't help - so am a bit confused.

The need for equity sync still exists, but am starting to wonder if the API is worth the trouble here. Even if we clear that hurdle, it is table-agnostic and hence doesn't really add any special logic that couldn't be accomplished with direct SQL. When I think of a "create transaction" API conceptually, I picture it requiring all (items/tenders) data up-front, which it validates etc. before saving and perhaps processing even further.

So curious what you think about it. At the very least I can be testing with direct SQL in the meantime, but will be tempted to just go live with that. I'm game for helping craft an API if you do think it's worth it, but may not tackle on my own.

As for this PR, it still just exposes the trans DB to normal API calls. If you think that much is safe and desirable, by all means merge it.

lgedgar commented 11 months ago

1199 superseded this for me, and for now anyway probably not worth changing the main FannieEntity API.

Anecdotally, I integrate with another POS using their API, but only for writes. I use direct SQL to read for them too. I think it's just not practical to craft a "complete" API, better to just meet real needs and leave the reading to SQL maybe.