libracore / erpnextswiss

ERPNext application for Switzerland-specific use cases
GNU Affero General Public License v3.0
77 stars 63 forks source link

Bank Import by Template #48

Closed jHetzer closed 5 years ago

jHetzer commented 5 years ago

Expected behaviour

Possibility to create/select templates which define how a csv file is mapped by a custom doctype

Actual behaviour

To add custom bank csv files it would be necessary to extent the Python file bankimport.py and other html files. Not possible from the web interface

Observed in (version)

v0.9.2

Steps to reproduce


It would be nice to have such templating possibility as described in "Expected behaviour". Also, nice would be:

I hope this feature request is interesting. By the way: Nice Work

lasalesi commented 5 years ago

Hi @jHetzer,

thank you for your input and interest in ERPNextSwiss!

Indeed, it is an issue that each bank has their own csv format for the export. We had also considered a column-based parser. However, there are some banks that use multi-line csv (e.g. Raiffeisen), where one transaction consists of one or two lines (details in the second line). A pure column-based parser would not be able to cope with this. Another issue is that in some cases there needs to be in-line parsing (e.g. for dates), where we do not only define that thare is a date in a column, but also define how to parse it into the correct UNC date, or the allocation of hashcodes when the csv does not provide a unique identification of a transaction. Normally, under operating conditions, there should be no changes in the parameters (configure once, train operator/team, execute), which would indicate a configuration option.

These considerations have led to the implementation where the Python code implements a parser for each bank (when using csv). We also consider the csv-exports to become deprecated at some point and to be replaced with standard formats like CAMT (053 and 054).

Any pull request is highly appreciated!

jHetzer commented 5 years ago

Thanks for your reply.

Based on the current code it should be quite simple (not yet familiar with pyhton or erpnext development) to implement the bank I need. It will be the german bank Volksbank "Raiffeisenbank". If you like I can make a pull request when done even though it's a german bank.

I'm planing on extending the ERPNextSwiss Settings to hide unused bank imports and maybe the possibility to define a default import option (CAMT, CSV).

I understand your considerations but templating would still be nice. If I have time I will check/search for a neat way to accomplish it (if there is any).

lasalesi commented 5 years ago

Much appreciated!

You can alternatively pack a few sample records - annonymised - and PM them to me, then I'll look into it. I like the settings idea to reduce options and set defaults!

jHetzer commented 5 years ago

I created a pull request to add support for the german banks Kreissparkasse and Volksbank #53 I did some testing to enable and disable Banks in "ERPNext Swiss Settings" DocType: erpnext_banksettings_2 erpnext_banksettings

Following is working

I think with release 1.0.0 the format option can be removed because for camt... the import wizard should be used?

lasalesi commented 5 years ago

Dear @jHetzer,

many thanks for your pull request! We will review the code and merge it as soon as possible...

Yes, the bank wizard is the new version of banking integration. However, for compatibility, for now we will still keep the "old" csv modes...

jHetzer commented 5 years ago

I did some work on the csv templating topic. Would be nice if someone could check out the solution and give feedback: https://github.com/jHetzer/erpnextswiss/tree/csvTemplate

Following "BankImport Template" export can be imported as example templates: BankImport_Template.xlsx

Screenshot 1: BankImport Template image

Screenshot 2: ERPNextSwiss Settings image Screenshot 3: Bankimport page image

Would this functionality be interesting for ERPNextSwiss?

lasalesi commented 5 years ago

Highly interesting ;-) merged into develop for testing. Feedback from other users highly welcome!

jHetzer commented 5 years ago

I consider to change the "field ID" fields to "Data" type instead of "Int" because "data" allows non- mandatory fields to be empty (instead of "-1"). The field input could be validated with:

[field name]: function(frm) {
    if(isNaN(frm.doc.[field name])){
        if(frm.doc.[field name] || frm.doc.[field name] !== ""){
            frappe.msgprint(__("ID Field needs to be numeric and at least '0'"));
            frm.set_value("[field name]","");
        }
    }
}

This code checks if the input is numeric and not less then '0'. Changes in bankimport.py function getFieldDefinition() would then be also required.

Any remarks?

lasalesi commented 5 years ago

Successfully tested in ERPNext v11 with Python 3...

lasalesi commented 5 years ago

Merged