feat(Bank Reconciliation): more options in configure statement import #23616
Conversation
gwhitney
requested review from
deepeshgarg007 and
nextchamp-saqib
as code owners
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed within a week if no further activity occurs, but it only takes a comment to keep a contribution alive :) Also, even if it is closed, you can always reopen the PR when you're ready. Thank you for contributing. |
|
The Travis failure is complaining about something called overflow_wrap, and does not seem to have anything to do with importing bank statements, so I do not believe it is related to this PR. Looking forward to your feedback. |
Prior to this commit, it was impossible for a single column in a (imported, spreadsheet) bank statement to populate different fields in a bank transaction depending on its value. Thus, for example, if your statements just list a signed "Amount" rather than having separate "Credit" and "Debit" columns, it was impossible to import them. This commit adds an optional "If Value Matches" field to the "Bank Transaction Mapping" record. This field, if defined, should be a regular expression, and the value will only be assigned to the associated Bank Transaction field if the value matches the given regular expression. Moreover, the value used to specify the Bank Transaction field will be the first "group" (parenthesized sub-expression in the regular expression) in the match. So this way, you can detect a debit by a minus sign included in the value of an "Amount" column, and then filter out that minus sign for setting the debit field of the Bank Transaction. Therefore, it now can make sense to have more than one Bank Transaction Mapping record with the same "Column in Bank File" value, as long as they have different regular expressions in the "If Value Matches" field. Similarly, this facility could be used to extract a transaction_id and a text description from a single "Description" column with a structured format. (In our case, the "Description" column in our exported bank statements often has the format "Deposit ID Number 999999.") In addition, prior to this commit, if the bank statement included no currency column, the currency of the generated Bank Transactions would end up taking on a rather arbitrary value, presumably based on the order of entries in the internal Currency list. This commit causes the Bank Transaction currency to default to the Default Currency of the Company which holds the Bank Account, which seems to generally produce more useful results.
gwhitney
force-pushed
the
conditional_bank_mapping
branch
from
ceed87f
to
027ae75
Compare
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed within a week if no further activity occurs, but it only takes a comment to keep a contribution alive :) Also, even if it is closed, you can always reopen the PR when you're ready. Thank you for contributing. |
|
I now understand that I am required to make an associated documentation PR for the checks to pass. (I think that's a fairly new requirement?) I will get to that as soon as I can; probably in the new year, with year-end workloads as they are. |
|
Hi @gwhitney
We have redesigned the Bank Reconciliation process. It now uses the data import functionality of Frappe Framework. This feature if needed should now be implemented in the framework
This is not very clear. Can you please elaborate on the proposed solution? |
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed within a week if no further activity occurs, but it only takes a comment to keep a contribution alive :) Also, even if it is closed, you can always reopen the PR when you're ready. Thank you for contributing. |
Prior to this commit, it was impossible for a single column in a
(imported, spreadsheet) bank statement to populate different fields
in a Bank Transaction depending on its value. Thus, for example, if
your statements just list a signed "Amount" rather than having
separate "Credit" and "Debit" columns, it was impossible to import
them. (Our primary bank generates CSV bank statements with
just the one signed "Amount" field, and we need to be able to
import them.)
This commit adds an optional "If Value Matches" field to the "Bank
Transaction Mapping" record. This field, if defined, should be a
regular expression, and the value will only be assigned to the
associated Bank Transaction field if the value matches the given
regular expression. Moreover, the value used to specify the Bank
Transaction field will be the first "group" (parenthesized
sub-expression in the regular expression) in the match. So this
way, you can detect a debit by a minus sign included in the value
of an "Amount" column, and then filter out that minus sign for
setting the debit field of the Bank Transaction. Therefore, it now
can make sense to have more than one Bank Transaction Mapping
record with the same "Column in Bank File" value, as long as they
have different regular expressions in the "If Value Matches" field.
Similarly, this facility could be used to extract a transaction_id
and a text description from a single "Description" column with a
structured format. (In our case, the "Description" column in our
exported bank statements often has the format "Deposit ID Number
999999" and so this feature also allows us to extract that
transaction_id.) This aspect justifies adding the more general
regular-expression feature, as opposed to some more
specialized facility created to sort one field into credit/debit
depending on its sign.
In addition, prior to this commit, if the bank statement included
no currency column, the currency of the generated Bank
Transactions would end up taking on a rather arbitrary value,
presumably based on the order of entries in the internal Currency
list. This commit causes the Bank Transaction currency to default
to the Default Currency of the Company which holds the Bank
Account, which seems generally to produce more useful results.