Briljander Posted November 25, 2019 Posted November 25, 2019 @Occam I think you should take the opportunity to get this fixed instead of being rude...
Occam Posted November 26, 2019 Posted November 26, 2019 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.
datakick Posted November 26, 2019 Posted November 26, 2019 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*
datakick Posted November 26, 2019 Posted November 26, 2019 I've created an issue for this to not be forgotten https://github.com/thirtybees/thirtybees/issues/1116
datakick Posted November 26, 2019 Posted November 26, 2019 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
Occam Posted November 26, 2019 Posted November 26, 2019 (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 November 26, 2019 by Occam
Traumflug Posted November 26, 2019 Posted November 26, 2019 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.
datakick Posted November 26, 2019 Posted November 26, 2019 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. 1
Occam Posted November 26, 2019 Posted November 26, 2019 @Traumflug, this is not about rounding! The rounding works.
Occam Posted November 26, 2019 Posted November 26, 2019 (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 November 26, 2019 by Occam 1
datakick Posted November 26, 2019 Posted November 26, 2019 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 1
Occam Posted November 26, 2019 Posted November 26, 2019 (edited) Yep, I must have been blind. Thank you very much. Works perfect now: Edited November 26, 2019 by Occam
wakabayashi Posted November 27, 2019 Author Posted November 27, 2019 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.
Pedalman Posted December 30, 2019 Posted December 30, 2019 (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 December 30, 2019 by Pedalman
led24ee Posted December 30, 2019 Posted December 30, 2019 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. 1
Pedalman Posted December 31, 2019 Posted December 31, 2019 (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: Edited December 31, 2019 by Pedalman 1
RabbitZzZ Posted January 9, 2020 Posted January 9, 2020 Hi, issue-1116 isn't available anymore on core updater. How can I put the fix in my shop?
sennevb Posted June 4, 2020 Posted June 4, 2020 I think theres still something wrong with credit slip : 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?
sennevb Posted June 9, 2020 Posted June 9, 2020 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?
datakick Posted June 9, 2020 Posted June 9, 2020 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?
sennevb Posted June 9, 2020 Posted June 9, 2020 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
datakick Posted June 9, 2020 Posted June 9, 2020 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
sennevb Posted June 9, 2020 Posted June 9, 2020 1 minute ago, datakick said: Update to 1.1.x / bleeding edge safe for live shop?
Recommended Posts
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 accountSign in
Already have an account? Sign in here.
Sign In Now