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

adding deposit transactions #3730

Conversation

christen90
Copy link
Contributor

Adding support for deposit transactions to Barclays credit card statements.

Comment on lines 80 to 81
.match("^[\\d]{2}\\.[\\d]{2}\\.[\\d]{4} (?<date>[\\d]{2}\\.[\\d]{2}\\.[\\d]{4}) " //
+ "(?<note>.{1,36}).+ +" //
Copy link
Member

Choose a reason for hiding this comment

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

I think this regulare expression is not good.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note seems to have a fixed length (max 36 characters) and is followed by whitespaces. For now it was working in all cases. I will monitor it in the following months.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi. I hope I understood your concerns correctly, that the combination of two '.' followed by each other allow multiple outcomes. For now it was working, but it could also happen, that the first character is catched into 'note'-subgroup and rest is lost.

I updated PR. Did it become worse or better?🤣
BR Christian

@Nirus2000
Copy link
Member

Hello @christen90
Thanks for this pull request.
I would redesign the whole thing as a section and change the transaction type based on the character (plus or minus).
As an example --> here <--

You can also take a look at the new variant of the test cases... --> here <--
These are more simply structured....

Regards
Alex

@Nirus2000 Nirus2000 added the pdf label Jan 15, 2024
@christen90
Copy link
Contributor Author

Hello Alex,
thanks for the ideas. I'll consider it in upcoming pull requests. I'll not modify this one as it is working fine.

BR Christian

use trim function
fixed test case. anonymization changed length of string.
@christen90 christen90 force-pushed the adds-barclays-visa-pdf-import-deposit branch from d8a15ea to 73d0647 Compare January 27, 2024 07:08
buchen pushed a commit that referenced this pull request Jan 28, 2024
@buchen
Copy link
Member

buchen commented Jan 28, 2024

Thanks for the updated pull request @christen90
rebased, squashed, merged.

@buchen buchen closed this Jan 28, 2024
@christen90 christen90 deleted the adds-barclays-visa-pdf-import-deposit branch January 28, 2024 21:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants