Testing credits for costcenters

[eynimeni]

It is now possible to assign credits to costcenters.

So in cashiers desk some changes apply:

• Credits for different costcenters are listed in the history
  

• each costcenter has a button confirm refund (via cash or transfer)

• Credits Manager modal contains a field to define costcenter
  
  

• there are settings in shoppingcart about how to handle credits with no costcenter applied:
  samecostcenterforcredits (If this setting is turned on and a user receives credits, these credits can only be used for items with the same costcenter.) & defaultcostcenterforcredits (If no cost center is specified, credits can be redeemed for items from this cost center. If no value is entered here, credits without defined costcenters can be used for any item.)

• there is also a setting in booking "cfcostcenter" that defines which custom booking option field should be used for the costcenter.

We would need to test:
behat

• correct handling of costcenters by credits manager: if credit is corrected or refunded, it should be displayed correctly in the list
• correct handling of buying and cancelling of items with credits -> right costcenter should be used
• settings should apply and change the behaviour of availabilty of credits for items with given costcenters

phpunit

• correct handling of costcenters by credits manager: if credit is corrected or refunded, it should be displayed correctly in the list

[semteacher]

An issue found:
For the defaults (consideredto be correct)

      | config                      | value       | plugin              |
      | samecostcenterforcredits    | 1           | local_shopping_cart |
      | defaultcostcenterforcredits | CostCenter1 | local_shopping_cart |

When I create a default credits via generator this way (credits with cc - credits witohut cc):

 And the following "local_shopping_cart > user credits" exist:
      | user  | credit | currency | costcenter  |
      | user1 | 42     | EUR      | CostCenter1 |
      | user1 | 31     | EUR      |             |

result looks OK:

But when I try to create credits in opposite order (credits witohut cc - credits with cc):

    And the following "local_shopping_cart > user credits" exist:
      | user  | credit | currency | costcenter  |
      | user1 | 31     | EUR      |             |
      | user1 | 42     | EUR      | CostCenter1 |

I got this:

Have checked code, but not sure that I properly understand combinations of conditions there

As per description I was given above - defaultcostcenterforcredits should be used only for cost redeeming? As per code - it is involved into correction as well...

So I keep version of test which fails - until resolution.

[georgmaisser]

Hi,

the idea is, that the amount of the empty costcenter can be used by any other costcenter. The reason is, that currently the booking fee has no costcenter. when we cancel a purchase, including the booking fee, we still want to be able to reuse it.

Also, when we introduce costcenters only later and there are still credits without costcenters, we want to be able to use them later on.

[georgmaisser]

so, having 10 Euros without coustcenter and 10 euros with cc1, we will get 20 Euros in the get balance function. but also, when we have additionally 10 euros in cc, we should get 25 on cc2.

But this is not true for the view where we see all costcenters. There, we will see 10 on empty, 10 on cc1 and 15 on cc2

Thank you for finding the problem. We will solve this. Obviously, there can't be a difference only in the order in which we add the costcenters. We found a strange behaviour on the production site as well, which might be linked to this bug, actually.

[semteacher]

Have started with pay back an found potentially more serious issue during manual tests:
For the configuration

I made a payback using "Credit manager" dialog with settings

And got negative values:

[semteacher]

Potential issue (Issue 1) found:

• For the following settings
  
• With testitems
• 
  a) Got the following checkout
  
  (suggestion: how about to present which costcenter has been used as deductible?)
  b) Finally, the following checkout results were obtained:
  
  I consider it as incorrect. Have expected 10EUR to be deducted from CostSenter1, 20.30 from CostSenter2 and 13.80 - from credits without costcenter... Am I correct?
  (Will try to debug this)

[georgmaisser]

Hi, sorry, the amounts don't add up, no?

The logic should be, that we always deduce first from the empty costcenter. Only when sth is left, we get it from the matching cost center. but these are tricky operations. the problem here seems to me only that we took credit from costcenter 2.

[semteacher]

@georgmaisser Thank you for clarification...
But even in this case - behaviour still incorrect. With 51 Eur balance in the "unnamed" costcenter - deduction of 44,10EUR fully covered... And it is presented on screens. Good. But it looks like that for some reason, the remaining credits (6.90) had been deduced form CostCenter1. It should not happens in any case...?
PS: CostCenter2 is untouched so it is fit your explanation

[semteacher]

I suspect much more issues with costcenter credits (or I do not understand its logic at all)
Issue 2

• For the following settings:
  
• With testitems
  
• Got the following checkout and history of purchases
  
  
  Costcenters where provided explicitly for both test items. Both Costcenters have credits > item price. Only Costcenter2 balahce was shown at checkout. BUT only Costcenter1 was used for payment. It is not that behaviour of defaultcostcenterforcredits described above...
  Issue 3
• For the following settings:
  
• And the same test items as above
• Got the following checkout
  
  This is closely to correct (Costcenter2 balahce < corresponded Item 7 price) Except - total price (30.30) has been shown (correct) but only Costcenter2 balahce counted for deduction... Moreover - remaining total (18.30) looks like Costcenter1 was not counted / processed at all. I would expect remaining total to be 8.30 only: 10EUR price of item 6 covered by 32EUR balance of Costcenter1 and for item 7 and Costcenter2 we should have 20.30-12 = 8.30EUR

[semteacher]

I have analized code of cartstore and shopping_cart_credits classes. Current logic of how credits and costcenters being applied looks overcomplicated. Probably it is result of moving from simle credits model without costcenters to the model which has to support multiple costcenters and different credits per costcenter...
At the monent - I have lack of complete understanding of processing sequence...
Beside, I would suggest that some refactoring of at least cartstore or both classes is necessary.
I would try to strictly separate credits from costcenters with older "no costcenter" credits...

1. In the "add_item" method cartitems have been stored as multi-dimensional array and later cached this way. But costcenter and credits values being stored on the top level of array. Make sense for credits when no costcenter used... But not for separate costcenters... As a result when multiple items have been added to cart, upon retreiving cartitems via get_cache method wegot something like this
   

• credits = "51" make some sense becaue come from "no costcenter"... But as we ise 1 row for credits at checkout page - maybe it would be better to summarize all user credits there (from all costcenters)?
• but costcenter = "CostCenter3" does not make sense at all for given config. Not sure if this field could be useful at all.
  I would suggest to store information about all costcenter balances (those from get_balance_for_all_costcenters method). But it could lead to another complications and necessity of wider refactoring.

2. In the shopping_cart_credits -> prepare_checkout method only a single (last?) costcenter has been used to retreive balance of credits
   
   Than this single balance use to decrease total price
   
   But if samecostcenterforcredits was forced - this has to be done separately for each item / costcenter pair?
   (it might be more issues - I have stopped at this point for now upon decision being made)

PS: It might be sort of "brainstorming" necessary to decide on solution

[semteacher]

Have started of testing the following recommendation

Have enforced samecostcenter=1 for testing.

1. It seems to me that samecostcenterforcredits setting and related logic will be no longer necessary if samecostcenter will be enforced in code (and even could cause errors if will not be removed - see "add_item" method in the cartstore class).
2. For items which has costcenter provided explicitly - credits wotking OK.
3. For items with no costcenter:

• under following configuration
  
• the credits without costcenter have been used, Seems correct.
• BUT for the following configuration
  
• credits have been taken from CC2
  
• Are the default CC1 should be used in this case?

PS: If we will fall to use same costcenter for checkout, I think that samecostcenter=1 should be enforced on code level and removed from settings to avoid issues

[georgmaisser]

Hi,

If we will fall to use same costcenter for checkout, I think that samecostcenter=1 should be enforced on code level and removed from settings to avoid issues

I think you are right! Can you implement that?

[semteacher]

@georgmaisser

1. As for my understanding - both samecostcenter and samecostcenterforcredits are no longer necessary as they have to be enforced in code as the only approach to use credits. Yes, I can try to remove it and adjust code accordingly. Except it will take some time because of my irregular working schedule.
2. After that - additional fixes might be necessary to propoerly dealt with defaultcostcenterforcredits (see last screns above)

[semteacher]

1. I have removed both samecostcenter and samecostcenterforcredits params. All erxisting tests (under both shopping cart and booking) passed OK.
2. Bug has been found: for the new behat with the following settings
   

• I got negative balance again:
  

3. Question. For the following settings
   

• should the "defaultcostcenter" has to be used? By above description - I would expect yes....

defaultcostcenterforcredits (If no cost center is specified, credits can be redeemed for items from this cost center. If no value is entered here, credits without defined costcenters can be used for any item.)

But it does not used:

[semteacher]

@georgmaisser I made few fixes and adjustments. All tests seems working OK...
Specifically, I forced what you wrote about:

principle: no costcenter is always ( with all settings) used first. So if we have 10 on empty and 10 on cc1, price is 15 we keep 5 in cc1. This is regardless of settings.

because above conditions as well as definitions of defaultcostcenter

defaultcostcenterforcredits (If no cost center is specified, credits can be redeemed for items from this cost center. If no value is entered here, credits without defined costcenters can be used for any item.)

had preventing of usage of "no costcenter" credits

[semteacher]

@georgmaisser In last commit I have proposed the approach to deal with $defaultcostcenter. Working (#86)

• If item(s) do not have costcenter defined, chain of costcenters will be: "nocostcenter" -> default costcenter (if set)
• If item(s) have costcenter defined, chain of costcenters will be: "nocostcenter" -> given costcenter

I have created few behat and phpunit tests . Definitely, much more phpunit will be necessary to cover all possible combinations (costcenters/credits). But before it would be fine to know if proposed approach is acceptable at all.

[georgmaisser]

Hi Andrii, your proposition sounds fine to me! Thank you