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

[14.0][FIX] account_statement_import_txt_xlsx: empty balance_column field #703

Open
wants to merge 1 commit into
base: 14.0
Choose a base branch
from

Conversation

Tiago370
Copy link

This change addresses the error obtained when the column cell amount_column is empty.

@OCA-git-bot
Copy link
Contributor

Hi @alexey-pelykh,
some modules you are maintaining are being modified, check this out!

@Tiago370 Tiago370 force-pushed the fix/empty_balance_column_field branch 2 times, most recently from 518f194 to 44db952 Compare June 21, 2024 18:56
@mileo
Copy link
Member

mileo commented Jun 27, 2024

Backport of #693

@@ -61,7 +62,9 @@ def parse(self, data_file, mapping, filename):
lines = self._parse_lines(mapping, data_file, currency_code)
if not lines:
return currency_code, account_number, [{"transactions": []}]

if not isinstance(lines[0]["timestamp"], datetime):
Copy link
Contributor

Choose a reason for hiding this comment

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

How this is related to empty balance column?

Copy link
Author

Choose a reason for hiding this comment

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

Hello @alexey-pelykh , this line was necessary, because some banks' statements put the first line without a date, containing only the description and the previous balance, so the solution I found was to copy the date of the next line. But I'm open to suggestions, thanks for the review.

Copy link
Contributor

Choose a reason for hiding this comment

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

This should be done as a separate PR. Backport is a backport, sneaky changes aren't good :)

Copy link
Author

Choose a reason for hiding this comment

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

I understand. I will provide a new PR and fix this one.

@Tiago370 Tiago370 force-pushed the fix/empty_balance_column_field branch from 44db952 to 50fbb8e Compare June 27, 2024 20:22
@pedrobaeza pedrobaeza added this to the 14.0 milestone Jul 30, 2024
@pedrobaeza
Copy link
Member

This is replacing the attribution of @mileo . Why aren't you doing a cherry-pick?

@Tiago370
Copy link
Author

Good morning @pedrobaeza , thanks for the review. I did this because the module name is different in version 16.0. Is there any way I can do a cherry-pick in this situation? I am available for suggestions.

@pedrobaeza
Copy link
Member

Well, you can cherry-pick and when giving the error, then manually insert the changes and remove the offending files, and do git add -A; git cherry-pick --continue. Another option is to simply do git commit --all --amend --author ... with the name and mail of Luis for preserving at least the attribution.

@Tiago370
Copy link
Author

@pedrobaeza Perfect, I'll try that.

@Tiago370 Tiago370 force-pushed the fix/empty_balance_column_field branch from 50fbb8e to 3460bcc Compare August 7, 2024 14:58
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.

5 participants