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

8.0 Add module account_bank_statement_import_paypal #7

Closed
wants to merge 159 commits into from

Conversation

alexis-via
Copy link
Contributor

This module is designed for the new bank statement system of the PR #6

This module is certainly not perfect, in particular on the problems of multi-language. The problem is that the Paypal CSV files use strings in the language of the user... and I use a few of those strings in the parsing. But I don't see any easy solution for this...

@pedrobaeza
Copy link
Member

For the parsing question, I would use _() to also translate the string you are using. What do you think?

vals_line = False
user = self.pool['res.users'].browse(cr, uid, uid, context=context)
company_currency_name = user.company_id.currency_id.name
comission_total = 0.0
Copy link
Member

Choose a reason for hiding this comment

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

commission

@alexis-via
Copy link
Contributor Author

@pedrobaeza I think it's not as easy as that. For example: I use OpenERP in English because I prefer to use it in english to avoid all the bad French translation and to avoid the mix of translated and untranslated strings. But my Paypal bank statements are in French because Paypal knows I'm from France.

I think that the best solution is to have some _prepare method that can be inherited in a country-specific module. It is needed for:

  • date parsing
  • file encoding
  • amount parsing (decimal separator)
  • status of the line

i just implemented it. Tell me what you think.

@pedrobaeza
Copy link
Member

You can add then a parameter to say in which language is the file, and use .with_context(lang='...') in all parse calls.

@alexis-via
Copy link
Contributor Author

@pedrobaeza I'm really not sure that using translations is the good answer for this. The problem is that I don't have enough information on Paypal CSV bank statements to decide what is the best solution, that's why I propose to use the prepare*() solution for the moment.

@pedrobaeza
Copy link
Member

But this is also not a good option, because this forces to make subsequent account_bank_statement_import_paypal_xx for each language, and with no possibility of mixing paypal files from one language and another. We should provide a configurable mechanism.

@sbidoul
Copy link
Member

sbidoul commented May 19, 2015

@alexis-via there is a pep8 issue:

./account_bank_statement_import_paypal/account_bank_statement_import_paypal.py:164:25: W503 line break before binary operator

And unicodecsv must be added to the travis config.

[ENH] Might need info in camt import from PmtInfId element.
@legalsylvain legalsylvain added this to the 8.0 milestone Mar 9, 2016
@pedrobaeza
Copy link
Member

As there's no plans to work on this from the author, I close. Feel free to reopen in case there's interest.

@pedrobaeza pedrobaeza closed this Oct 14, 2016
@elicoidal
Copy link

@pedrobaeza @jbeficent Actually this coould be interesting to fix for the particular case of OCA Paypal Bank Statement.

Would it be worth the effort to fix it only for our particular case?

@pedrobaeza
Copy link
Member

Well, it's very specific to the downloaded format, but if you pass me a sample, I can check the coincidences. I have set up a variant of this one for an Spanish Paypal statement.

@elicoidal
Copy link

Thanks I will private email you

daramousk pushed a commit to daramousk/bank-statement-import that referenced this pull request Apr 11, 2018
[ADD] more tests, thanks to Louis Bettens
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.