proseLA / authorizenet_cim

CIM Module for authorizenet for zencart
3 stars 2 forks source link

sql needs update for so payments #1

Closed proseLA closed 4 years ago

proseLA commented 4 years ago

if someone has already installed so and so payments the sql should be modified to alter the table as opposed to dumping it

lat9 commented 4 years ago

I'd suggest that, rather than having a .SQL file, that you include the base tables' creation/modification as part of the payment-method's installation function.

Why, too, are you using the so_payments table rather than one specific to the payment module?

proseLA commented 4 years ago

hi cindy, first of all thanks for looking at the code! much appreciated!!

as this is my first take at something so comprehensive, i find myself learning a bunch as i go.

i will certainly modify the install function to create those base tables. seems like the way to go.

with regards to the payments table, i suppose this is a bit of a bigger question... at least for me. this project evolved from my clients needs (isn't all of ZC done that way) to make use of a card on file option. at the point i started using CIM, the so_payments table was already there (due to having installed the super orders mod) and i decided to make use of that table. now, that can be easily changed i suppose.... but let's think about this logically. someone places an order, they pay for the order. the payment for an order is truly related to the order NOT to the type of payment. the customer gets the order, they return part of it. the store owner refunds part of the order. perhaps the store owner send them a check as opposed to refunding them on their credit card. again a refund is part of the order, not really part of the payment type. don't you think a payments table is not really part of a payments class? but more related to the order class?

is this a discussion/fight worth having over on the ZC github project? i don't know. but i would love to have your input.

and while i have your attention (hopefully), one of the big changes i made for my clients was charging credit cards AFTER receipt of the order, and the store owner doing it on the admin side. i'm doing a bunch of work on reverting that back to the more traditional model. i think one of the issues as to why ZC does NOT have a payments table is because the card gets charged before there is an order id. now perhaps there should be a payment class, and then things might get easier, or i need to do a 'hacky' way to save the payment record without an order number, and update it after order creation. again, if you have any thoughts i'm all ears.

thanks again!

drbyte commented 4 years ago

Note: I'm not commenting on code here. Just responding to the discussion as requested.

I agree that it'd be more ideal if the order were saved (temporarily at least) before payment was processed. But that will NOT happen in the 1.5.7 release. Probably not in any 1.5.x release unless we can find a way to not break all existing mods that rely on that old osCommerce workflow.

There are multiple things going on here:

The first, taking payment, is already easily handled, if it's not related to using stored cards or storing new cards.

Storing cards is more complicated, and probably should use a pivot table to track the customer's customer_id, payment subscriber_id and preferred_card_id identifier (whether that's last-4, or another card-identifier), perhaps named authnet_customer_tokens

Adding an order-status-workflow is something that v2 did a huge architectural overhaul for. Orders were all based on line-items, whether that was add/remove products, add/remove discounts and credits, add/remove calculated subtotals such as taxes, add/remove payments/refunds/credits ... so an order's data was more like event-sourcing where you could look at its state at any point in time, regenerating things to different stages as needed. It completely broke all prior order history though, as the old storage methods didn't contain enough data to fully migrate forward. A big hurdle we haven't finished yet is how to support both sets of data for storeowners' historical lookup and reporting purposes, while still focusing primarily on only the new structure for taking new orders. And of course "editing" any old orders would be near impossible (er, well, without Edit Orders/SuperOrders mods, etc). The benefit of the new as well is that all the editing would be possible from the Admin too, er, well, when it's done of course. The point in saying all that is merely to acknowledge the pain point you mention as real and that it's beyond the scope of this module.

I see no problem with this module detecting that SO is installed and store information in SO's associated table/s. Not sure what to do if SO isn't installed. Granted, without SO there'd be almost no admin-side editing relevant to this mod anyway, right? So maybe it's moot as long as SO is an enforced dependency: if SO exists, features are available, if no SO, then those features are disabled. ?

That said, if we can avoid making alterations to SO tables that SO doesn't really need I'm sure maintainers would be happier. Or is there an opportunity to collaborate here?

If certain data being added doesn't really suit SO then where else can/should it be stored?

proseLA commented 4 years ago

great! thanks so much for the feedback...

my clients sites have so much modification... between super orders and edit orders... i hate them both! (no offense, cindy, i think you do a GREAT job). given that this is a numinix thing, i will create separate payment tables and move on. no big deal there.

with regards to storing credit cards for future use, this part is done and getting ready. one can see from the screen shot.... there is still a lot of work to do. but i appreciate all the feedback and i will incorporate all suggestions.
payments_work

lat9 commented 4 years ago

There are now a plethora of notifications within the order's processing, so it should be "fairly easy" for any store that uses super-orders (or has an so_payments table) to record an order's id post-placement. Granted, making use of the notify/observe mechanisms tends to make debug a tad more difficult (since the processing is distributed among multiple modules).

lat9 commented 4 years ago

What's a numinix thing?

lat9 commented 4 years ago

FWIW, I just invited you to my oldy-moldy version of a.net/cim (based on the old API version). I don't know if it will help (the client for whom I created the payment method handled the card maintenance on the a.net website).

proseLA commented 4 years ago

this is why i get confused by the edit orders v super orders mods. when i go here: https://www.zen-cart.com/downloads.php?do=file&id=155&styleid=2 it says last updated by numinix, which is why i said a numinix thing.
i'll take a look at the a.net/cim thing. thanks. this is pretty close for me to get some additional eyes on for testing. assuming i get more time into it soon. right now it still needs more work... but i am getting closer. and i am DEFINITELY looking at using some observers on the admin side. modifying any core code is the LAST thing i want to do. thanks again for the help.

drbyte commented 4 years ago

when i go here: https://www.zen-cart.com/downloads.php?do=file&id=155&styleid=2

Aside: do you regularly browse the forum using the vBulletin default template instead of the Zen Cart template? ie: &styleid=2

proseLA commented 4 years ago

ALERT THREAD HIJACK!!

yes, i do. why?

drbyte commented 4 years ago

ALERT THREAD HIJACK!!

yes, i do. why?

Just curious: why? :)

proseLA commented 4 years ago

at the time, i was playing around and liked it more. now that i am looking at it again, it does not seem like a big diff between the two. the mobile version is a bit useless for me, as it removes links that i use, specifically the newest forums posts link... i like to tinker and figure some things out... except for git submodules. screw that!

drbyte commented 4 years ago

except for git submodules. screw that!

Ya, I try to avoid that whenever possible!

proseLA commented 4 years ago

when i go here: https://www.zen-cart.com/downloads.php?do=file&id=155&styleid=2

Aside: do you regularly browse the forum using the vBulletin default template instead of the Zen Cart template? ie: &styleid=2

ALERT THREAD HIJACK!! yes, i do. why?

Just curious: why? :)

@drbyte are you aware that the 2 templates do not have the same functionality? at all.

drbyte commented 4 years ago

I guess I'm not. I know we've never maintained the default template, so I'm sure it's missing tons of things that are in the branded template.

What functionality are you referring to? and desktop or mobile?

proseLA commented 4 years ago

always desktop. i never use the mobile part of it.

the issues discussed here:

https://www.zen-cart.com/showthread.php?226692-plugins-missing-image-and-php-5-3-safe&p=1368697#post1368697

and here:

https://www.zen-cart.com/showthread.php?226704-Github-button-does-not-appear&p=1368694#post1368694

drbyte commented 4 years ago

Update: Yes I'm aware that lots of things customized in the Zen Cart skin are NOT in the default vB skin. There are no plans to waste time backporting things.

What's the reason for NOT using the Zen Cart skin?

proseLA commented 4 years ago

as i stated on 3/23, i was playing around and liked it more. i suppose i find the orange buttons a bit offensive on my eyes.

when we were discussing it then, i suppose i would have appreciated a heads up that using that template loses some functionality, as opposed to the just curious response. perhaps it slipped your mind, i do not know... i just find it curious that you brought it up then.

my question (for which i do not need an answer) is why make a skin available if it loses functionality? perhaps one can not remove that default skin on the forum, i do not know. i have never configured a vBulletin forum before. some of the wikis that i have configured do offer multiple skins, but losing functionality has NEVER been an issue on those wikis.... as far as i can tell.

water under the bridge as far as i am concerned.

drbyte commented 4 years ago

I apologize for not adding that clarification. I remember thinking it and intending to mention it, but if I failed to do it, I'm sorry. One of the reasons for keeping the old skin enabled is because I can't delete it because it's also the "master" (kinda like template_default), and if someone "does" switch to it if there's no nav menu to switch back with then they get caught in some sort of no-man's land. A bit of an annoying trait with vB.