Jump to content

Welcome, Guest!

By registering with us, you'll be able to discuss, share and private message with other members of our community.

wakabayashi

How to handle returns?

Recommended Posts

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. 

Share this post


Link to post
Share on other sites
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*

Share this post


Link to post
Share on other sites

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

 

Share this post


Link to post
Share on other sites

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

Share this post


Link to post
Share on other sites
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.

Share this post


Link to post
Share on other sites
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

Share this post


Link to post
Share on other sites
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

Share this post


Link to post
Share on other sites

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

Share this post


Link to post
Share on other sites

Yep, I must have been blind.

 

image.png

Thank you very much. Works perfect now:

image.png.d867c7ebaef8c9d1e5c6175c8ab74813.png

Edited by Occam

Share this post


Link to post
Share on other sites
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.

Share this post


Link to post
Share on other sites

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

Share this post


Link to post
Share on other sites

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

Share this post


Link to post
Share on other sites

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

Share this post


Link to post
Share on other sites

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...