63: Fix discountCode export with deleted codes #64
Conversation
There was a problem hiding this comment.
Thanks for this PR @ValeSauer 🥇
Is it possible that you could add a test for it?
| @@ -68,6 +68,11 @@ class CsvMapping | |||
| else [entry] | |||
|
|
|||
| formatDiscountCodes = (discountCodes) -> | |||
| discountCodes = _.each(discountCodes, ({ discountCode }) -> | |||
| if !discountCode.obj | |||
| discountCode.obj = { code: 'discountCode_deleted' } | |||
There was a problem hiding this comment.
Hello @ValeSauer .. is this a correct solution? What if somebody wants to export discount codes to the external system? He would import also discountCode_deleted code. Shouldn't we filter out deleted discount codes instead of replacing them with some constant value?
@wizzy25 what do you think?
There was a problem hiding this comment.
Not sure which approach is better. My first idea was to export nothing in this case, but after I discussed with a customer I switched over to above's solution. It gives you more insights and is (as long as you now what discountCode_deleted means) better to understand. If you just export nothing, this could be misunderstood as no discountCode used.
@wizzy25 Do you want to decide?
There was a problem hiding this comment.
@ValeSauer I agree with @junajan. Using any form of constant value (even discountCode_deleted) could also mean that the discount code exists (in this case, that discountCode_deleted was the discount code used.
A solution may be to add a new boolean CSV column (for example: isDiscountCodeDeleted) which we set to true or false and leave the actual code column empty. Doesn't have to be exactly this, but something along this line.
A flag can also be used so it's not breaking
There was a problem hiding this comment.
@wizzy25 Is there already an example in which order-export exports columns that were not specified in the csv template? This might be unexpected too.
Suggestion:
- We simply export nothing for now. By doing so we at least make it possible to export such orders again.
- We create an issue to discuss how to deal with these cases in a proper way.
Alright?
There was a problem hiding this comment.
@ValeSauer I don't think there's an example for that, although it can be done. If it should be done is another discussion.
By exporting "nothing", do you mean no order should be exported?
There was a problem hiding this comment.
By exporting "nothing", I mean no discount code should be exported.
This PR
Fixes #63