Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OBPIH-4837 Transactions Report in csv makes credits and debits 0 for … #3696

Merged
merged 2 commits into from
Dec 14, 2022

Conversation

kchelstowski
Copy link
Collaborator

@kchelstowski kchelstowski commented Dec 9, 2022

…locale in Fr

Like I told to @awalkowiak I don't feel good about this solution, but couldn't figure out anything better for now.
The problem was, that transactionData stores transactionTypeName always in English, for example: Transfer in, but transactionTypeNames example is: Transfer in|fr:<french_translation>, which resulted in comparing in line 534 for example Transfer In == <its_french_translation>, so the result was always 0.
So the solution I figured out was just to split the transationTypeName by the delimiter (|) and take the first element (English), so we compare it propely.
Still, the translated version is needed in order to put it to the header of CSV, so I kept it

}
def quantity =
transactionData.find {
it.productCode == product.productCode && it.transactionTypeName == transactionTypeName
it.productCode == product.productCode && it.transactionTypeName == transactionTypeName.split("\\|")[0]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this necessary? Shouldn't getLocalizedString be doing that for us?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does, but it compares English == French which always results in false.
The transactionData in line 468 stores the transactionTypeName as English only, so that's why I made it

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll send some images from debugging in a moment to show what I mean

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, the transactionData example object looks like this:
Transaction data

and the transationTypeName example looks like this:
Transaction data

so it was comparing Consumption == Consommantion for example, instead of comparing Consumption == Consumption

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok got it. I am going to write a comment in the ticket to explain what I'd like to see in the long term. But for now, you have the right approach. Let's just move that logic to a convenience method on the TransactionType class if possible.

@awalkowiak Are we allowed to do something like this

Boolean compareTo(String transactionTypeName)
Boolean equals(String transactionTypeName)

or would it be better not to implement methods that are defined by common interfaces?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jmiranda I think I'd go with something like Boolean compareTransactionType(String transactionTypeName) because the string that might be passed not always will be the transactionTypeName, so for clarity, I would avoid using compareTo and equals and strictly let others know what I want to compare.
cc @kchelstowski

@awalkowiak awalkowiak merged commit a8c90b7 into develop Dec 14, 2022
@awalkowiak awalkowiak deleted the OBPIH-4837 branch December 14, 2022 14:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants