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

[13.0][MIG] account_credit_control: Migration to 13.0 #54

Merged
merged 76 commits into from
Jun 11, 2020

Conversation

manuelcalerosolis
Copy link

This is the migration to v 13.0 module account_credit_control

@Tecnativa TT21599

Copy link
Member

@ernestotejeda ernestotejeda left a comment

Choose a reason for hiding this comment

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

Thanks @manuelcalerosolis , Good job !!!!

@sergio-teruel sergio-teruel force-pushed the 13.0-mig-account_credit_control branch 3 times, most recently from 2b56336 to 97bc743 Compare February 27, 2020 16:43
@manuelcalerosolis manuelcalerosolis force-pushed the 13.0-mig-account_credit_control branch 2 times, most recently from 86778c9 to 1879a7f Compare March 4, 2020 10:10
@sergio-teruel sergio-teruel force-pushed the 13.0-mig-account_credit_control branch from 1879a7f to 8c006cf Compare March 5, 2020 09:09
<tr t-foreach="doc.credit_control_line_ids" t-as="l">
<t t-if="l.invoice_id">
<td>
<span t-field="l.invoice_id.number"/>
Copy link
Contributor

@CasVissers-360ERP CasVissers-360ERP Mar 23, 2020

Choose a reason for hiding this comment

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

Number is not a field of account.move, name should be sufficient?

Copy link
Contributor

Choose a reason for hiding this comment

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

@pedrobaeza
I still get:

  File "/Users/brahoo/PycharmProjects/cust/odoo-13.0/odoo/addons/base/models/ir_qweb.py", line 363, in _get_field
    field = record._fields[field_name]
KeyError: 'number'

Error to render compiling AST
KeyError: 'number'
Template: account_credit_control.report_credit_control_summary_document
Path: /t/t/div/table/tbody/tr/t[1]/td/span
Node: <span t-field="l.invoice_id.number"/>

I think the line should be removed. Only name is sufficient.

Copy link
Contributor

@CasVissers-360ERP CasVissers-360ERP left a comment

Choose a reason for hiding this comment

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

Initial feedback

{
"name": att[0],
"datas": att[1],
"datas_fname": att[0],
Copy link
Contributor

Choose a reason for hiding this comment

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

Field name is changed to store_fname in v13?

Copy link
Member

Choose a reason for hiding this comment

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

Changed, thanks

Copy link
Contributor

@CasVissers-360ERP CasVissers-360ERP left a comment

Choose a reason for hiding this comment

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

Incorrect field on ir.attachment found

@sergio-teruel sergio-teruel force-pushed the 13.0-mig-account_credit_control branch from 3c926ae to 3505f06 Compare March 31, 2020 14:42
@rafaelbn
Copy link
Member

Please don't merge until my review. Thanks

Copy link
Contributor

@CasVissers-360ERP CasVissers-360ERP left a comment

Choose a reason for hiding this comment

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

Record rules are incorrect.

@ and others added 7 commits April 2, 2020 10:22
* Modify scenarios to be compatible with python scenarios
* Voucher/statement related steps
* Deprecated step implementation
* Print report generation
* All tests green
* Mail + print of reminder
* Broken translations
* Fix failure to confirm invoice when both account_credit_control  and account_constraints are installed
* use except_orm instead of except_osv
* reindent code
* Cleanup
* use of deprecated field user_email causes permission error in multicompany setting fixed by using the correct 'email' field in the template
* Address in mako template to make sure address is displayed
* Remove 'more info here' from report
* Add some css to reminder report and set texts taking the whole width of the document
* Add a subject in reminder report
* Report use precise mode by default
* date_entry fields on line model to be used by report or in next MP on filter group by
* reporting layer, + add hook function to get contact address
* credit control mail are not in plain text but send as attachement
* policy level name is now translatable as it is use in report and mail
* permission on invoices because onetomany widget load data even if hidden ...
* translation files + lang source
* french translation
@pedrobaeza pedrobaeza force-pushed the 13.0-mig-account_credit_control branch 2 times, most recently from 4b11e3c to 0960e87 Compare April 2, 2020 12:00
@pedrobaeza pedrobaeza force-pushed the 13.0-mig-account_credit_control branch from 0960e87 to 162cde2 Compare April 2, 2020 12:07
@pedrobaeza
Copy link
Member

@CasVissers @rafaelbn can you please re-review?

@CasVissers-360ERP
Copy link
Contributor

https://drive.google.com/file/d/1piDHuSzMDWha06kQoXrDMh89bijioySg/view

Even though a credit control line has been processed it seems that it is created a 2nd time? Or am I doing something wrong?

@pedrobaeza
Copy link
Member

Is that happening the same on v12?

@CasVissers-360ERP
Copy link
Contributor

No I can't reproduce the same behavior in v12. I think it's wrong any way we look at it?

@pedrobaeza
Copy link
Member

If this doesn't happen in v12, then indeed it's a problem of the migration. I will take over this PR and check that. Thanks for reviewing.

@CasVissers-360ERP
Copy link
Contributor

@pedrobaeza
any progress?

@pedrobaeza
Copy link
Member

Sorry, not yet. I will try ASAP.

Copy link
Contributor

@CasVissers-360ERP CasVissers-360ERP left a comment

Choose a reason for hiding this comment

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

Fix found for level not increasing

@CasVissers-360ERP
Copy link
Contributor

@bhaveshselarka
Maybe if you stop spamming it will go a bit faster.

Done in an extra commit for not changing the SHA of the record rules addition, as
it's referenced on the migration guide.

This can be squashed when migrating the module to upper versions.
@pedrobaeza pedrobaeza force-pushed the 13.0-mig-account_credit_control branch from 64aa139 to 998bbd9 Compare June 11, 2020 07:38
@pedrobaeza
Copy link
Member

Let's finally merge it:

/ocabot merge nobump

@OCA-git-bot OCA-git-bot mentioned this pull request Jun 11, 2020
6 tasks
@OCA-git-bot
Copy link
Contributor

Hey, thanks for contributing! Proceeding to merge this for you.
Prepared branch 13.0-ocabot-merge-pr-54-by-pedrobaeza-bump-nobump, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit 2615473 into OCA:13.0 Jun 11, 2020
@OCA-git-bot
Copy link
Contributor

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

@CasVissers-360ERP
Copy link
Contributor

@pedrobaeza
Maybe I missed something here..

Is and not demo data? They are now created when installing/updating the module.

@pedrobaeza pedrobaeza deleted the 13.0-mig-account_credit_control branch August 19, 2020 08:49
@pedrobaeza
Copy link
Member

I don't get you.

@CasVissers-360ERP
Copy link
Contributor

CasVissers-360ERP commented Aug 19, 2020

<record id="a_sale" model="account.account">
        <field name="code">X2020</field>
        <field name="name">Product Sales - (test)</field>
        <field name="user_type_id" ref="account.data_account_type_revenue" />
        <field name="tag_ids" eval="[(6,0,[ref('account.account_tag_operating')])]" />
    </record>
    <!-- sales journal -->
    <record id="sales_journal" model="account.journal">
        <field name="name">Customer Invoices - Test</field>
        <field name="code">TINV</field>
        <field name="type">sale</field>
        <field name="default_credit_account_id" ref="a_sale" />
        <field name="default_debit_account_id" ref="a_sale" />
        <field name="refund_sequence" eval="True" />
    </record>

Is in data.xml, "data/data.xml", is in manifest? I think this is demo data?
image

@pedrobaeza
Copy link
Member

Yes, I don't know why they are there. Can you please do a PR amending it?

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

Successfully merging this pull request may close these issues.