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

[11.0][MIG] : mig account_invoice_import #41

Merged
merged 83 commits into from
Oct 17, 2018

Conversation

njeudy
Copy link
Collaborator

@njeudy njeudy commented Jan 28, 2018

@pedrobaeza pedrobaeza added this to the 11.0 milestone Jan 29, 2018
@pedrobaeza pedrobaeza mentioned this pull request Jan 29, 2018
27 tasks
@njeudy njeudy force-pushed the 11-mig-account_invoice_import branch from 92d9533 to 3c1afd4 Compare January 29, 2018 13:30
@coveralls
Copy link

Coverage Status

Coverage remained the same at 41.483% when pulling 3c1afd4e57d034d1288cca189344edc0be412f8a on njeudy:11-mig-account_invoice_import into b13c8e9 on OCA:11.0.

@coveralls
Copy link

coveralls commented Jan 29, 2018

Coverage Status

Coverage decreased (-9.3%) to 55.517% when pulling 957392f on njeudy:11-mig-account_invoice_import into 6994600 on OCA:11.0.

@njeudy njeudy force-pushed the 11-mig-account_invoice_import branch from 3c1afd4 to 1721baf Compare January 29, 2018 14:09
@njeudy njeudy changed the title WIP: 11 mig account invoice import [MIG] 11.0: mig account_invoice_import Jan 29, 2018
<field name="model">res.partner</field>
<field name="inherit_id" ref="account.view_partner_property_form"/>
<field name="arch" type="xml">
<field name="currency_id" position="after">
Copy link
Member

Choose a reason for hiding this comment

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

Here I would prefer:

<xpath expr="//group[@name='acc_purchase']" position="inside">

<field name="arch" type="xml">
<xpath expr="//h2[contains(text(),'Taxes')]" position="before">
<h2 attrs="{'invisible': [('has_chart_of_accounts','=',False)]}">Invoice Import</h2>
<div class="row mt16 o_settings_container" attrs="{'invisible': [('has_chart_of_accounts','=',False)]}">
Copy link
Member

Choose a reason for hiding this comment

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

Could you indent a bit less?

@@ -0,0 +1,91 @@
.. image:: https://img.shields.io/badge/licence-AGPL--3-blue.svg
Copy link
Member

Choose a reason for hiding this comment

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

Could you use the latest template? The .png image is preferable.

@njeudy njeudy force-pushed the 11-mig-account_invoice_import branch 2 times, most recently from 2e49e72 to 87a5dd7 Compare January 31, 2018 13:20
f.seek(0)
invoice = f.read()
f.close()
inv_b64 = invoice.encode('base64')
Copy link
Member

Choose a reason for hiding this comment

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

Use base64.b64encode(invoice)

self.ensure_one()
assert self.invoice_file, 'No invoice file'
logger.info('Starting to import invoice %s', self.invoice_filename)
file_data = self.invoice_file.decode('base64')
Copy link
Member

Choose a reason for hiding this comment

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

Use base64.b64decode(self.invoice_file)

@njeudy njeudy force-pushed the 11-mig-account_invoice_import branch 2 times, most recently from fe7ea04 to 9c87252 Compare January 31, 2018 14:00
@astirpe
Copy link
Member

astirpe commented Feb 1, 2018

@njeudy you may want to approve this easy one OCA/community-data-files#34, to speed up the pending PRs that are required.

@njeudy njeudy force-pushed the 11-mig-account_invoice_import branch 2 times, most recently from f0fa9ff to bad58b3 Compare February 21, 2018 14:26
@njeudy
Copy link
Collaborator Author

njeudy commented Feb 21, 2018

@alexis-via @astirpe coverage decrease with this PR, can you help me to improve test coverage ?

Need I add real pdf invoice to import and try différent config on it ?

new_res = []
for (inv_id, name) in res:
inv = self.browse(inv_id)
display_currency = inv.currency_id
Copy link
Member

Choose a reason for hiding this comment

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

Add line

                pre = post = u''

to avoid this error:

Error:
Odoo Server Error

Traceback (most recent call last):
  File "/opt/odoo/odoo-11.0/odoo/http.py", line 647, in _handle_exception
    return super(JsonRequest, self)._handle_exception(exception)
  File "/opt/odoo/odoo-11.0/odoo/http.py", line 307, in _handle_exception
    raise pycompat.reraise(type(exception), exception, sys.exc_info()[2])
  File "/opt/odoo/odoo-11.0/odoo/tools/pycompat.py", line 87, in reraise
    raise value
  File "/opt/odoo/odoo-11.0/odoo/http.py", line 689, in dispatch
    result = self._call_function(**self.params)
  File "/opt/odoo/odoo-11.0/odoo/http.py", line 339, in _call_function
    return checked_call(self.db, *args, **kwargs)
  File "/opt/odoo/odoo-11.0/odoo/service/model.py", line 97, in wrapper
    return f(dbname, *args, **kwargs)
  File "/opt/odoo/odoo-11.0/odoo/http.py", line 332, in checked_call
    result = self.endpoint(*a, **kw)
  File "/opt/odoo/odoo-11.0/odoo/http.py", line 933, in __call__
    return self.method(*args, **kw)
  File "/opt/odoo/odoo-11.0/odoo/http.py", line 512, in response_wrap
    response = f(*args, **kw)
  File "/opt/odoo/odoo-server/addons/web/controllers/main.py", line 930, in call_kw
    return self._call_kw(model, method, args, kwargs)
  File "/opt/odoo/odoo-server/addons/web/controllers/main.py", line 922, in _call_kw
    return call_kw(request.env[model], method, args, kwargs)
  File "/opt/odoo/odoo-11.0/odoo/api.py", line 689, in call_kw
    return call_kw_multi(method, model, args, kwargs)
  File "/opt/odoo/odoo-11.0/odoo/api.py", line 680, in call_kw_multi
    result = method(recs, *args, **kwargs)
  File "/opt/odoo/custom/addons/account_invoice_import/models/account_invoice.py", line 26, in name_get
    inv.amount_untaxed, pre=pre, post=post)
UnboundLocalError: local variable 'post' referenced before assignment

Copy link
Member

Choose a reason for hiding this comment

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

In Python 3 every string is unicode, so no need for the u

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ok will do this. @astirpe any idea to improve coverage ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

Copy link
Member

Choose a reason for hiding this comment

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

Thanks!
I cannot check the coverage, as one test is failing...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I create a merge request for failing test because it is not on this module

here it is : #53

Copy link
Member

Choose a reason for hiding this comment

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

The line you added pre = post = '' is in the wrong place. It should be placed inside the for loop.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

Copy link
Member

Choose a reason for hiding this comment

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

Thanks. Now the other line, the one outside the loop (pre = post = ''), can be removed.

Copy link
Member

Choose a reason for hiding this comment

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

I mean line 13

@njeudy njeudy force-pushed the 11-mig-account_invoice_import branch 2 times, most recently from e0dacbf to 94bf720 Compare April 11, 2018 12:44
@njeudy
Copy link
Collaborator Author

njeudy commented Apr 11, 2018

@pedrobaeza is it possible in oca dependency to import other branch of same repo to have travis green ?

@pedrobaeza
Copy link
Member

Uhm, I don't think so... Try to put on oca_dependencies.txt the name of the organization and the branch to see it if works. Any way, better to work on merging the other PR, so we don't need this hack.

@njeudy
Copy link
Collaborator Author

njeudy commented Apr 11, 2018

Ok will remove hack and work on merging PR thanks

@astirpe
Copy link
Member

astirpe commented Apr 12, 2018

Scenario:

  • the vendor A has "Invoice Import Configuration" set to "Multi Line, Auto-selected Product"
  • create an new Vendor Bill and set vendor A; do not add any line yet, leave it empty; save the draft vendor bill
  • try to import the vendor bill lines by clicking on "Import Invoice File"

I get this Server Error - Warning:

The total amount is different from the untaxed amount, but no tax has been configured ! 

The error above is raised because the vendor bill does not contain any tax line yet, as it was created empty.
But I would expect that the tax line is automatically imported as well.

As a workaround, I did this:

  • create an new Vendor Bill and set vendor A; do not add any product line but add one tax line; save the draft vendor bill
  • try to import the vendor bill lines by clicking on "Import Invoice File"

The product line and the tax line are correctly imported.

My request is: is it possible to automatically import the tax lines without that workaround? The calculation of the tax line can be based on the just imported product lines and the fiscal position of the vendor (as usual).

@njeudy
Copy link
Collaborator Author

njeudy commented Apr 12, 2018

@astirpe i will check this .. can you reproduce this on V10 ? (V10 is the standard up to date branch for this module I think .. )

@astirpe
Copy link
Member

astirpe commented Apr 13, 2018

@njeudy I guess is the same in V10, since this part of code is the same. In this case we could open a separate issue if you prefer.

parsed_inv['amount_untaxed']
invoice.tax_line_ids[0].amount = tax_amount
cur_symbol = invoice.currency_id.symbol
invoice.message_post(
Copy link
Member

Choose a reason for hiding this comment

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

Could you make the message translatable, as it was done in V10? (the fix of PR #51 should be ported to V11)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

@njeudy njeudy force-pushed the 11-mig-account_invoice_import branch from 94bf720 to e20c44c Compare April 13, 2018 09:26
alexis-via and others added 25 commits October 12, 2018 15:00
One of the zugferd-specific code has been standardised in UNECE, so moved to the account_tax_unece module
Fix update invoice via import wizard
… better support out customer invoice import

Fix test suite of account_invoice_import (Agrolait is only a customer in v10)
* Update to work with latest version of invoice2data

* Add requirements.txt file
…not deduct VAT (OCA#64)

* Move code to a new dedicated method
* FIX update of existing draft invoice
  Improve error message of partner matching
  Move hook of partner matching to put it after VAT matching, which is the
  most reliable
* Add support for importing an invoice with VAT in a company that cannot deduc VAT
  Display nice error when account is missing on product
* Improve compliancy with Factur-X standard (using schematron)
… has visible discounts

Code improvements in sale_order_import
Add unit tests in sale_order_import
Use display_name instead of name_get()[0][1]
Update the account_invoice_download_weboob following the changes in account_invoice_download
Add README.rst for the 2 modules
Add weboob module management
Improve log handling (explicit error when wrong pwd)
account_invoice_import: by default, set tax to default tax of account when invoice_line_method = 1line_no_product
Update text displayed in the invoice import wizard and make list of
supported formats modular (like in bank statement import)
… in multi-company environnement

Fix my previous commit on sequence field on invoice import config (caused a crash when creating a new record)
Usability improvements on download config
…iness document is imported in the right company
@astirpe astirpe force-pushed the 11-mig-account_invoice_import branch from d29c854 to 957392f Compare October 12, 2018 13:00
@astirpe
Copy link
Member

astirpe commented Oct 12, 2018

@pedrobaeza squashed, the commits are reduced from 115 to 83. Would you give it a check?

@astirpe
Copy link
Member

astirpe commented Oct 17, 2018

@pedrobaeza I'm tempted to merge this one. Is it ok?

@pedrobaeza
Copy link
Member

OK, we can reduce a bit more the history on 12.0. Thanks all for the efforts!

@astirpe astirpe merged commit 1955b90 into OCA:11.0 Oct 17, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet