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

[FIX] account_reconcile_oca: Make it work with foreign currency #577

Merged
merged 4 commits into from
Sep 4, 2023

Conversation

etobella
Copy link
Member

Fixes #574

@nagybal Can you test it please?

@etobella
Copy link
Member Author

etobella commented Aug 19, 2023

Still missing a test in order to leave this documented, but should be functional 😄

@nagybal
Copy link

nagybal commented Aug 21, 2023

@etobella Dear Enric! Thank You for taking a look into it!

  • I have updated the account_reconcile_oca module only.
  • Gave it a new version, so I can verify it on the instance too.

Unfortunately, the issue is the same:

The widget takes the amount of the main currency and puts it where the amount in the secondary currency should be.
Screen Shot 2023-08-21 at 10 59 06

I can manually correct the amount, but odoo still does not allow to reconcile:
Screen Shot 2023-08-21 at 10 59 37

It messes up the currencies again
Screen Shot 2023-08-21 at 10 59 54

The move (BNK2/2023/00032 (2023-03-16)) is not balanced.
The total of debits equals 1,919,047 Ft and the total of credits equals 5,037 Ft.
You might want to specify a default account on journal "Bank (EUR)" to automatically balance each move.

@etobella
Copy link
Member Author

Did you use the one from the PR? I mean, when you come from an external currency, it should show the in currency column 🤔

@nagybal
Copy link

nagybal commented Aug 21, 2023

I have used the below files. I just checked a couple of lines with nano on the server. They are the files from the PR #577

Screen Shot 2023-08-21 at 11 38 20

@etobella
Copy link
Member Author

Ok, I will check again, thanks!

@etobella
Copy link
Member Author

I added some more info in order to show when a reconcile involves foreign currencies. You can use runboat in order to test it.

@nagybal
Copy link

nagybal commented Aug 21, 2023

You have to create a new bank account (a new journal)!
http://oca-account-reconcile-16-0-pr577-a52523510ef3.runboat.odoo-community.org/

In my case:
Base currency: HUF

Statement: EUR
Bank Account: EUR

I should be possible to reconcile EUR with EUR!

@nagybal
Copy link

nagybal commented Aug 21, 2023

Using Your USD EUR example, the problem occurs on the second bank account:
Screen Shot 2023-08-21 at 13 11 41

@etobella
Copy link
Member Author

I see!!! The problem is raised when we use a journal from a different currency!

Thanks!

@etobella
Copy link
Member Author

Well, now it worked for me. Let's see if it works for you 😄

@nagybal
Copy link

nagybal commented Aug 26, 2023

@etobella unfortunately I have little, basically no knowledge of coding. I tried to figure out what have You added. Shall I look at this database?
http://oca-account-reconcile-16-0-pr577-994ff0a16c3a.runboat.odoo-community.org/

@etobella
Copy link
Member Author

You just need to reproduce the error in this runboat database. If you cannot reproduce it again and all works as expected, then it is fixed. Don't worry about the code, I was looking for a functional review 😁

@nagybal
Copy link

nagybal commented Aug 26, 2023

Then the issue is the same. If You take a look at the runboat database. The base currency is USD the invoice is in HUF and the payment was made in HUF as well. For some reason the reconciliation widget turns the HUF amount into USD. The amount is 500 000 HUF(Ft) and it should stay 500 000 and NOT 1413.71 USD which is the "bookkeeping" amount.

Screen Shot 2023-08-26 at 10 33 32

@etobella
Copy link
Member Author

I see, the problem comes from the data in "amount in Currency" that is not properly showed. Right now it should work properly. It only failed on the first line.

@nagybal
Copy link

nagybal commented Aug 26, 2023

You can reconcile now. If You have multiple invoices to choose from. This collum should be the amount in currency as well:

Screen Shot 2023-08-26 at 11 39 22

ps: The amount in currency changes because of decimal currency. It can be fixed in developer mode.

@etobella
Copy link
Member Author

True, I added the field, but it is hidden. BTW, I found a small Odoo issue with that, as an aggregation line is added by default, but it shouldn't 😞 odoo/odoo#133274

@nagybal
Copy link

nagybal commented Aug 26, 2023

odoo/odoo#133274

The two screenshots seem to me identical

@etobella
Copy link
Member Author

Look at the footer. In the first one there's a gray line with a black line

@nagybal
Copy link

nagybal commented Aug 26, 2023

I just logged in to 49355b2 and changed noting. But if I hit the reconcile button, I get an error straight away:

RPC_ERROR
Odoo Server Error

Traceback (most recent call last):
  File "/opt/odoo/odoo/api.py", line 984, in get
    cache_value = field_cache[record._ids[0]]
KeyError: 8

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/opt/odoo/odoo/fields.py", line 1160, in __get__
    value = env.cache.get(record, self)
  File "/opt/odoo/odoo/api.py", line 991, in get
    raise CacheMiss(record, field)
odoo.exceptions.CacheMiss: 'account.bank.statement.line(8,).can_reconcile'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/opt/odoo/odoo/api.py", line 984, in get
    cache_value = field_cache[record._ids[0]]
KeyError: 8

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/opt/odoo/odoo/fields.py", line 1160, in __get__
    value = env.cache.get(record, self)
  File "/opt/odoo/odoo/api.py", line 991, in get
    raise CacheMiss(record, field)
odoo.exceptions.CacheMiss: 'account.bank.statement.line(8,).reconcile_data_info'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/opt/odoo/odoo/http.py", line 1584, in _serve_db
    return service_model.retrying(self._serve_ir_http, self.env)
  File "/opt/odoo/odoo/service/model.py", line 133, in retrying
    result = func()
  File "/opt/odoo/odoo/http.py", line 1611, in _serve_ir_http
    response = self.dispatcher.dispatch(rule.endpoint, args)
  File "/opt/odoo/odoo/http.py", line 1815, in dispatch
    result = self.request.registry['ir.http']._dispatch(endpoint)
  File "/opt/odoo/odoo/addons/base/models/ir_http.py", line 154, in _dispatch
    result = endpoint(**request.params)
  File "/opt/odoo/odoo/http.py", line 697, in route_wrapper
    result = endpoint(self, *args, **params_ok)
  File "/opt/odoo/addons/web/controllers/dataset.py", line 42, in call_kw
    return self._call_kw(model, method, args, kwargs)
  File "/opt/odoo/addons/web/controllers/dataset.py", line 33, in _call_kw
    return call_kw(request.env[model], method, args, kwargs)
  File "/opt/odoo/odoo/api.py", line 461, in call_kw
    result = _call_kw_multi(method, model, args, kwargs)
  File "/opt/odoo/odoo/api.py", line 448, in _call_kw_multi
    result = method(recs, *args, **kwargs)
  File "/opt/odoo/odoo/models.py", line 2982, in read
    return self._read_format(fnames=fields, load=load)
  File "/opt/odoo/odoo/models.py", line 3154, in _read_format
    vals[name] = convert(record[name], record, use_name_get)
  File "/opt/odoo/odoo/models.py", line 5876, in __getitem__
    return self._fields[key].__get__(self, type(self))
  File "/opt/odoo/odoo/fields.py", line 1209, in __get__
    self.compute_value(recs)
  File "/opt/odoo/odoo/fields.py", line 1387, in compute_value
    records._compute_field_value(self)
  File "/opt/odoo/odoo/models.py", line 4211, in _compute_field_value
    fields.determine(field.compute, self)
  File "/opt/odoo/odoo/fields.py", line 100, in determine
    return needle(records, *args)
  File "/opt/odoo/addons/base_sparse_field/models/fields.py", line 52, in _compute_sparse
    values = record[self.sparse]
  File "/opt/odoo/odoo/models.py", line 5876, in __getitem__
    return self._fields[key].__get__(self, type(self))
  File "/opt/odoo/odoo/fields.py", line 1209, in __get__
    self.compute_value(recs)
  File "/opt/odoo/odoo/fields.py", line 1387, in compute_value
    records._compute_field_value(self)
  File "/opt/odoo/odoo/models.py", line 4211, in _compute_field_value
    fields.determine(field.compute, self)
  File "/opt/odoo/odoo/fields.py", line 97, in determine
    return needle(*args)
  File "/mnt/data/odoo-addons-dir/account_reconcile_oca/models/account_bank_statement_line.py", line 326, in _compute_reconcile_data_info
    record.reconcile_data_info = record._default_reconcile_data()
  File "/mnt/data/odoo-addons-dir/account_reconcile_oca/models/account_bank_statement_line.py", line 432, in _default_reconcile_data
    *self._reconcile_data_by_model(
  File "/mnt/data/odoo-addons-dir/account_reconcile_oca/models/account_bank_statement_line.py", line 353, in _reconcile_data_by_model
    for line in reconcile_model._get_write_off_move_lines_dict(
  File "/opt/odoo/addons/account/models/account_reconcile_model.py", line 523, in _get_write_off_move_lines_dict
    if currency.is_zero(balance):
UnboundLocalError: local variable 'balance' referenced before assignment

The above server error caused the following client error:
null

@etobella
Copy link
Member Author

The one you used, was a mistake I did. I merged two different PRs by error. You can test the current one 😄

@etobella
Copy link
Member Author

Well, I added some tests. It should be ready for review now 😄

@pedrobaeza pedrobaeza added this to the 16.0 milestone Aug 28, 2023
@rinaldifirdaus
Copy link

Functional test in runboat: the debit and credit amount works as expected and can be reconciled now.
but I found that when i register a transaction in bank journal that use secondary currency and reconcile the invoice that use secondary currency, in the statement line the invoice "amount in currency" showing the main currency not the secondary currency. (main currency is USD, secondary is EUR)
image
I think it should show the currency that in use in the invoice, isn't it?

@etobella
Copy link
Member Author

@rinaldifirdaus I checked runboat and I saw your item, but I am unable to reproduce what you did 😢
However, I detected an issue with automatic reconciliation models

@rinaldifirdaus
Copy link

@etobella , thanks for checking!
here's step what i did:

  1. create invoice in EUR
  2. in Bank (EUR) kanban, create transaction (assuming we receive the payment on our EUR account)
  3. fill the label, partner, and EUR amount in "amount" field, then save
  4. invoice and the bank statement will automatically reconciled, and the amount in currency for invoice showing the main currency here

@nagybal
Copy link

nagybal commented Sep 4, 2023

Functional test in runboat: the debit and credit amount works as expected and can be reconciled now.
but I found that when i register a transaction in bank journal that use secondary currency and reconcile the invoice that use secondary currency, in the statement line the invoice "amount in currency" showing the main currency not the secondary currency. (main currency is USD, secondary is EUR)
I think it should show the currency that in use in the invoice, isn't it?

I can not confirm this either. On 91e9748 checks out almost perfectly. You can reconcile but I do not understand this negative sign on front. If this is some kind of accounting stuff. I can live it it.

Screen Shot 2023-09-04 at 13 40 54

@nagybal
Copy link

nagybal commented Sep 4, 2023

LGTM

@etobella
Copy link
Member Author

etobella commented Sep 4, 2023

/ocabot merge minor

@OCA-git-bot
Copy link
Contributor

This PR looks fantastic, let's merge it!
Prepared branch 16.0-ocabot-merge-pr-577-by-etobella-bump-minor, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit 8ed9be8 into OCA:16.0 Sep 4, 2023
6 checks passed
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at dc7f1ea. Thanks a lot for contributing to OCA. ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants