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] agreement -> base_contract #397

Closed
wants to merge 23 commits into from

Conversation

i-vyshnevska
Copy link
Member

migrate agreement

ygol and others added 19 commits October 16, 2019 17:37
[UPD] README.rst
Currently translated at 100.0% (27 of 27 strings)

Translation: contract-12.0/contract-12.0-agreement
Translate-URL: https://translation.odoo-community.org/projects/contract-12-0/contract-12-0-agreement/pt_PT/
we move the is_template field definition and the agreement.type model from
the agreement_legal module to the agreement module.

The fields are not displayed by default, unless the feature is enabled through a
technical feature group, this is configurable in the agreement_sale module (because agreement
in itself has no UI, and agreement_legal enables the feature by default)
@max3903
Copy link
Member

max3903 commented Oct 16, 2019

@i-vyshnevska Please have a look at #386

@gurneyalex gurneyalex mentioned this pull request Oct 17, 2019
20 tasks
@gurneyalex
Copy link
Member

@i-vyshnevska can you rename the module base_contract?

@i-vyshnevska i-vyshnevska changed the title [13.0] [MIG] agreement [13.0] [MIG] agreement -> base_contract Oct 22, 2019
@pedrobaeza pedrobaeza added this to the 13.0 milestone Oct 29, 2019
@pedrobaeza
Copy link
Member

Do we finally have an agreement - hehe - on the renaming?

Copy link
Contributor

@sbejaoui sbejaoui left a comment

Choose a reason for hiding this comment

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

Please consider the discussion about 13.0 road-map #386

Copy link
Member

@jgrandguillaume jgrandguillaume left a comment

Choose a reason for hiding this comment

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

Work well on functional side

company_id = fields.Many2one(
"res.company",
string="Company",
default=lambda self: self.env["res.company"]._company_default_get(),
Copy link
Member

Choose a reason for hiding this comment

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

_company_default_get() should be removed: https://github.com/odoo/odoo/blob/58492733b81073694a7e2f5e790b31983f91df7d/odoo/addons/base/models/res_company.py#L205

It shows a warning at loading of the module.

Suggested change
default=lambda self: self.env["res.company"]._company_default_get(),
default=lambda self: self.env.company.id,

Comment on lines 24 to 26
<field name="agreement_type_id"
groups="agreement.group_use_agreement_type"/>
<field name="is_template" groups="agreement.group_use_agreement_template"/>
Copy link
Member

Choose a reason for hiding this comment

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

Warnings:

2020-05-06 13:44:29,775 115 WARNING odoodb odoo.addons.base.models.ir_ui_view: The group agreement.group_use_agreement_type defined in view agreement.form does not exist! 
2020-05-06 13:44:29,776 115 WARNING odoodb odoo.addons.base.models.ir_ui_view: The group agreement.group_use_agreement_template defined in view agreement.form does not exist! 
Suggested change
<field name="agreement_type_id"
groups="agreement.group_use_agreement_type"/>
<field name="is_template" groups="agreement.group_use_agreement_template"/>
<field name="agreement_type_id"
groups="base_contract.group_use_agreement_type"/>
<field name="is_template" groups="base_contract.group_use_agreement_template"/>

I didn't check for other places

Replace deprecated _company_default_get() by standard v13 syntax
Add domain field
Add chatter on agreement object
@alexis-via
Copy link
Contributor

I fixed and improved several points in this PR via a PR on @i-vyshnevska 's branch i-vyshnevska#1

@tbaden
Copy link
Member

tbaden commented Jun 16, 2020

What's the progress on this one? Is there still something todo or does it just need some more reviews?

@sbejaoui sbejaoui self-requested a review November 6, 2020 20:59
Copy link

@Chabagutor Chabagutor left a comment

Choose a reason for hiding this comment

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

Tested and installed for prodution purpose.

@alexis-via
Copy link
Contributor

This is ready to be merged. @sbejaoui 's comment is one year old and he asks to take into account the roadmap discussion, and it's precisely what has been implemented here. So let's move on and merge.

@alexis-via
Copy link
Contributor

There is something I don't understand with "pre-commit run -a" : when I run it on this branch, it doesn't change anything... although Travis complains about black/isort stuff.
If someone with more experience than me on this can sort it out... it's the last thing needed to merge.

@rvalyi
Copy link
Member

rvalyi commented Nov 13, 2020

@alexis-via probably you should try to rebase this branch on 13.0 before trying your local precommit.
Because from Travis logs it complains about XML formatting from the Prettier tool. But in this 13-mig-agreement, the precommit configuration .pre-commit-config.yaml (but may bve also some of the other dot files) is not up to date like in the 13 branch, specially this branch version has no such prettier formating instruction that's why it wont even try to run locally. Instead on Travis it looks it's running the 13.0 pre-commit configuration.

compare:
https://github.com/i-vyshnevska/contract/blob/13-mig-agreement/.pre-commit-config.yaml
https://github.com/OCA/contract/blob/13.0/.pre-commit-config.yaml#L24

Is it clearer now?

@alexis-via
Copy link
Contributor

A few days ago, I created another PR to replace this one: #578

@newtratip
Copy link
Member

@i-vyshnevska please close this pr supersede #628

@pedrobaeza pedrobaeza closed this Feb 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.