Jump to content
thirty bees forum

Recommended Posts

Posted

No, that wasn't meant to be rude, especially since I think @datakick is a gifted programmer.

From the point of view of Merchant and customer, I only wanted to show the logical consequence of such a misbehaviour of the software in an everyday process. And just for the record: If I knew exactly where the bugs were caused, I would also have posted suggestions for a solution. 

Posted
32 minutes ago, Occam said:

No, that wasn't meant to be rude, especially since I think @datakick is a gifted programmer.

From the point of view of Merchant and customer, I only wanted to show the logical consequence of such a misbehaviour of the software in an everyday process. And just for the record: If I knew exactly where the bugs were caused, I would also have posted suggestions for a solution. 

I really don't understand this problematic much. I would have to spend few hours trying to understand this domain by reverse-engineering the code. The code that is already broken, evidently. That's not an easy task to do. And it does not guaranteed that I will grasp it correctly.

By filing issue (complete with reprosteps, expected, and actual behavior) you would help immensely with this task. That doesn't mean I wouldn't have to reverse-engineer the code, or go back through git history to figure out when things went south. But it would give me some solid starting point

And most importantly, forum is not for tracking issue. That's what github is for. If it is not tracked there, it *will be forgotten*

Posted

This problem actually looks like an open and shut case. @Traumflug removed code that re-calculates totals in commit https://github.com/thirtybees/thirtybees/commit/e9e3f46ed0cc4ce083e5604595a96c6984f06856. He obviously thought this is redundant, as totals should be already calculated by Order class. That was a wrong assumption.  

Looks to me that revert of that commit rectifies the situation. Can somebody confirm? You can use core updater to update to branch issue-1116

Note that this does not remove the negative sign, for that one needs to modify the template. I'm not sure that's a bug, though -- in ps17 the amounts are printed as negative as well. But as I said -- I'm no expert in this domain area. So if you guys agree this is a bug, please file an issue for it

 

Posted (edited)

I can confirm that the negative signs currently may look like a bug, but this isn't the case. Provided the variables are filled correctly, it works that way. The past has shown that.

Damn it, I didn't even realize that @Traumflug had "raged" like that in the HTMLTemplateOrderSlip class.

Edited by Occam
Posted
2 hours ago, datakick said:

He obviously thought this is redundant, as totals should be already calculated by Order class. That was a wrong assumption.  

Well, it should be redundant. A class preparing values for display shouldn't have to recalculate numbers. It should just display the ones existing in the database already.

If reverting this commit makes a distinction, the revert likely just covers some other issue which puts these wrong numbers into the database first place.

Another topic with a revert would be the loss of precision, because removed code was obviously not subject to the price rounding audit.

Posted
2 hours ago, Traumflug said:

Well, it should be redundant. A class preparing values for display shouldn't have to recalculate numbers. It should just display the ones existing in the database already.

If reverting this commit makes a distinction, the revert likely just covers some other issue which puts these wrong numbers into the database first place.

Another topic with a revert would be the loss of precision, because removed code was obviously not subject to the price rounding audit.

The problem is that this presenter does not display (all) data from an existing order. It wants to display only data related to refunded products.

In order to do so, it loads the original order, remove all non-refunded products from it, and adjust shipping costs. Then, of course, the totals must be recalculated as well. Because you removed this final step, the order object has become inconsistent - it now contains refunded products, while the totals still represent original/ordered products.

 

 

  • Like 1
Posted (edited)
7 hours ago, datakick said:

Can somebody confirm? You can use core updater to update to branch issue-1116

Better, but still not a perfect solution. Because the tax totals are displayed even for items that were not refunded. E.g. you refund the shipping costs and not the products, then both tax details are displayed istead of only the tax details of the shipping costs. And vice versa, I suppose.

To avoid this you can use the fix I already posted on Septembre, 6. I removed this fix after severe critics by moderator @wakabayashi. Here is the zip for download again: Fix Bug Order Slip.zip

And according to my logical understanding, the minus signs are superfluous, because this is always a positive amount that the customer gets refunded. For vouchers it would even be absurd. Negative amounts would only make sense if the positive amounts were also displayed and a difference would be calculated. Therefor I had included adapted tpl files to generate correct pdfs  in this zip, too.

Edited by Occam
  • Haha 1
Posted

Thanks for the zip file, however it did not fixed the issue you have reported. Even with your version, when I refunded only shipping, product tax breakdown was displayed.

However, it pointed me towards the fix. It was deployed, you can use core updater and update to branch issue-1116 to test the fix.

I did not include the negative/positive sign yet. I'd like to hear opinions of other people first before I commit such a change

  • Thanks 1
Posted
On 11/25/2019 at 8:28 PM, datakick said:

I need somebody to actually explain to me what this is supposed to do.

Thats acutally a big problem in multiple areas of a shop system. What is the software supposed to do? IMO that's far from obvious in the return scenario. As every merchant should have an efficient way, to handle such returns. 

It's now multiple weeks ago, since I tested this things. So I don't remember the details anymore. Now it's also christmas sales, so I don't have time to look at it again. I can just propose, that every merchant post his idea/keypoints of a "perfect" return handling. For me the following points are important:

  • The stock must be corrected (in asm we have different types of stock)
  • The payment needs to be adjusted (probably the job of the payment module)
  • I need to understand the history of an order.
  • The customer needs to have clean history of an order as well.

For big shops stats like "How high is your return quote" or "How does your return quote change over time" would be relevant too.

  • 1 month later...
Posted (edited)

I noticed today that it is a must to enter total with American/English "." as seperator for decimals.

I run our shop in my local German and we use "," for small sums like delivery charge. This in my case is 2,90€ but when I enter this in "partial refund" for generating an order slip things go wrong!

It totals -2.00 though it should be -2.90€.

 

If I enter the delivery charge with a dot: 2.90€ all is fine.

This was not easy to understand for me since all numbers shown in backoffice use visually the comma as seperator. I think this should be adressed.

Happy New Year to all happy ThirtyBees!

Edited by Pedalman
Posted

This is an old theme. If I remember correctly then it tooks only 10 years for MS Excel to understand that some people use comma instead of dot. But in TB this is still problem. If You are "playing" with CSV then all prices and measures must be with dot. Even if comma is not value or field separator You still cant use this in prices.

  • Like 1
Posted (edited)

Your are absoultely right. Old problem but you have to know it. When I know about it than I can most often help myself. For exampe when dealing with csv I skip Pentaho Spoon and use "awk". But as said here it is very much misleading when you set the shop to a local. Moreover, when you can use your local decimal seperator at every other input field (as far as I know of) and if it looks like this:

 

partial refund decimal seperator.png

Edited by Pedalman
  • Like 1
  • 2 weeks later...
  • 4 months later...
Posted

I think theres still something wrong with credit slip :
creditslip.thumb.png.019fff431123b31307a1aa3464048e17.png
Not all the products are refunded, total of refunds is 865,29€ inc VAT

Total inc VAT for refund here is 1881.5€ , wich is the total amount of the order.
The credit slip total refund balance now show the full invoice amount, but only 865,29€ is refunded..

anyone?

Posted

Last bump, dont want o spam, but no one wants to help?
I am on latest TB, see screenshot above: 161,68€ + 459,60 + 244,01 doesnt add up to 18881,5€??
I foudn i nthe database the right amount, so the template is wrong??

Any advice?

Posted
2 hours ago, sennevb said:

Last bump, dont want o spam, but no one wants to help?
I am on latest TB, see screenshot above: 161,68€ + 459,60 + 244,01 doesnt add up to 18881,5€??
I foudn i nthe database the right amount, so the template is wrong??

Any advice?

What tb version are you running?

Posted
1 hour ago, datakick said:

What tb version are you running?

Thanks for answerring, i am on 1.1.0

i just checked again with core updater and there are no files that can be upgraded

Posted
7 minutes ago, sennevb said:

Thanks for answerring, i am on 1.1.0

i just checked again with core updater and there are no files that can be upgraded

Update to 1.1.x / bleeding edge

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now
×
×
  • Create New...