proseLA / authorizenet_cim

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

Use class order_status #61

Closed scottcwilson closed 3 months ago

scottcwilson commented 3 months ago

The database setting MODULE_PAYMENT_AUTHORIZENET_CIM_ORDER_STATUS_ID might be 0 (which does not correspond to an order status, and will lead to a log in orders.php). But $this->order_status is protected against being set to 0 in the constructor.

scottcwilson commented 3 months ago

Here's what the log looks like when this happens:

--> PHP Warning: Undefined array key 0 in /home/mailboxg/public_html/store/admin/orders.php on line 1111.

which corresponds to

<td><?php echo $orders_status_array[$item['orders_status_id']]; ?></td>
proseLA commented 3 months ago

The database setting MODULE_PAYMENT_AUTHORIZENET_CIM_ORDER_STATUS_ID might be 0 (which does not correspond to an order status, and will lead to a log in orders.php). But $this->order_status is protected against being set to 0 in the constructor.

scott, i can not make changes because the data in your database is wrong.

lets follow through on what you are asking. for those people who have a correct status value for MODULE_PAYMENT_AUTHORIZENET_CIM_ORDER_STATUS_ID and are using authorize first and then capture later, when they capture the amount, the order's status will now be equal to the DEFAULT_ORDERS_STATUS_ID which is incorrect.

this change would invalidate all users of this payment module to completely ignore this key if they are set to authorize first and capture later.

please think these things out before you make dig through what is going on.

i see that the default for this value is 0; and that may be an incorrect status, but what would you suggest for changing the default?

this idea is not a good one.

your client's log files do not necessitate a change in my code because you have incorrect values in your database.

scottcwilson commented 3 months ago

The default should be what is set in the c'tor. This change would use that. You can see the value '0' is not used for the default value because of the check at includes/modules/payment/authorizenet_cim.php line 96.

proseLA commented 3 months ago

no your change would not use what is set in the constructor.

https://github.com/proseLA/authorizenet_cim/blob/9a292f1060ebe622c27d37d3e2e8a787c504358b/includes/modules/payment/authorizenet_cim.php#L96

this part of line 96 is false:

MODULE_PAYMENT_AUTHORIZENET_CIM_AUTHORIZATION_TYPE !== 'Authorize'

so it would never get changed. when one captures according your change the orders status would always be equal to the DEFAULT_ORDERS_STATUS_ID.

scottcwilson commented 3 months ago

Which would be good! Because DEFAULT_ORDERS_STATUS_ID is non zero, it represents the index of an actual status. (You created a value you called "default" which has a value of 0. )

proseLA commented 3 months ago

scott, you are wrong here. i do not know how many more ways to tell you.

when a payment gets captured by an admin AFTER placing of the order; the orders status should change to the value of MODULE_PAYMENT_AUTHORIZENET_CIM_ORDER_STATUS_ID; NOT the value of DEFAULT_ORDERS_STATUS_ID. that is the whole point of having that key. if your client has 0 in there, i'm not real sure what you expect from me.

change their value, and stop posting here.

this PR makes no sense. i have looked it quite thoroughly.

scottcwilson commented 3 months ago

My client has 0 in there because this is the value provided by the installer. Your code sets this config value to 0. PayPal sets their corresponding value to 2, which would make more sense. But you're right - I can easily change the value that my client is using. Thanks for looking at this.