OCA / l10n-finland

GNU Affero General Public License v3.0
4 stars 20 forks source link

[11.0] [FIX] Payment Reference Visibility #30

Closed Tatider closed 4 years ago

Tatider commented 5 years ago

Always show "Payment Reference Number" for Customers and Vendors invoices.

oca-clabot commented 5 years ago

Hey @Tatider, thank you for your Pull Request.

It looks like some users haven't signed our Contributor License Agreement, yet. You can read and sign our full Contributor License Agreement here: http://odoo-community.org/page/cla Here is a list of the users:

Appreciation of efforts, OCA CLAbot

oca-clabot commented 5 years ago

Hey @Tatider, thank you for your Pull Request.

It looks like some users haven't signed our Contributor License Agreement, yet. You can read and sign our full Contributor License Agreement here: http://odoo-community.org/page/cla Here is a list of the users:

Appreciation of efforts, OCA CLAbot

aisopuro commented 5 years ago

@mtnarjan the lint is failing:

l10n_fi_payment_reference/models/account_invoice.py:119:21: E126 continuation line over-indented for hanging indent
aisopuro commented 5 years ago

@pedrobaeza the tests are failing because of failing to connect to the database:

Testing ['l10n_fi_banks', 'l10n_fi_business_code', 'l10n_fi_business_code_validate', 'l10n_fi_payment_reference']:

createdb: could not connect to database template1: could not connect to server: No such file or directory

    Is the server running locally and accepting

    connections on Unix domain socket "/var/run/postgresql/.s.PGSQL.5432"?

Is there someone we can contact to help us with this?

pedrobaeza commented 5 years ago

That's because a change in Travis infrastructure. Fixed in https://github.com/OCA/l10n-finland/commit/9d3d8f2d5983e391b89c71ff201c2ef7b3501aa0.

Please rebase over new 11.0 branch

oca-clabot commented 5 years ago

Hey @Tatider, We acknowledge that the following users have signed our Contributor License Agreement:

Appreciation of efforts, OCA CLAbot

coveralls commented 5 years ago

Coverage Status

Coverage remained the same at 84.762% when pulling 0154b680722fd4f5964e997c18e48284b391672c on avoinsystems:11.0-payment_ref_visibility into 9d3d8f2d5983e391b89c71ff201c2ef7b3501aa0 on OCA:11.0.

coveralls commented 5 years ago

Coverage Status

Coverage increased (+11.4%) to 96.19% when pulling e4c574a94825c7b26457fde118fa917d76bd7ec1 on avoinsystems:11.0-payment_ref_visibility into 9d3d8f2d5983e391b89c71ff201c2ef7b3501aa0 on OCA:11.0.

aisopuro commented 4 years ago

The coverage check seems to be failing because 10n_fi_payment_reference` doesn't have any tests that make sure that validating invoices actually generates the payment reference as expected. A new test should be added for doing that.

Tatider commented 4 years ago

No, % of coverage is the same and test for it is passed (greed highlight). But there is no new test for new changes (red highlight), what impossible in this case because:

  1. Change is in xml
  2. Another changes in code style, to pass style guide check pass. So no test can be added for changes Screenshot from 2019-10-01 11-26-06
aisopuro commented 4 years ago

The problem is basically that when the code was originally added, this particular coverage check seems to not have been in place. That means that the implementation of invoice_validate was accepted even though it was not covered by tests. However, now that is no longer OK, and so a test covering invoice_validate needs to be added, since it was changed.

aisopuro commented 4 years ago

Rebased commits to be clearer about changes.

OCA-git-bot commented 4 years ago

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

mlaitinen commented 4 years ago

/ocabot merge patch

OCA-git-bot commented 4 years ago

Hey, thanks for contributing! Proceeding to merge this for you. Prepared branch 11.0-ocabot-merge-pr-30-by-mlaitinen-bump-patch, awaiting test results.

OCA-git-bot commented 4 years ago

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