-
-
Notifications
You must be signed in to change notification settings - Fork 511
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] contract_sale_generation #329
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for the work 👍
I did a code review: LGTM,
but please use the new fragment readme
plus some optional changes if you like.
<?xml version="1.0" encoding='UTF-8'?> | ||
<odoo> | ||
|
||
<record model="ir.cron" id="account_analytic_cron_for_sale"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
optional change:
id before model
</field> | ||
</record> | ||
|
||
<record id="act_recurring_sales" model="ir.actions.act_window"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
optional change:
action id pattern
For an action: the main action respects <model_name>_action. Others are suffixed with _, where detail is an underscore lowercase string explaining the action (should not be long). This is used only if multiple actions are declared for the model.
<?xml version="1.0" encoding="utf-8"?> | ||
<odoo> | ||
|
||
<record id="account_analytic_account_recurring_sale_form" model="ir.ui.view"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
optional change:
use original id (account_analytic_account_recurring_form_form)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested on runbot
Hi @tbaden , @pedrobaeza and @luismontalba . I made required changes on my PR. |
* Sale Autoconfirm, validate Sales Orders if type is "Sales" | ||
|
||
.. image:: https://odoo-community.org/website/image/ir.attachment/5784_f2813bd/datas | ||
:alt: Try me on Runbot |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Runbot Button gets generated. Please remove this part from usage
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, please squash your commits into one. Thanks 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, noticed you removed it from readme instead of usage. Readme gets generated by oca bot. You have to change the usage.rst
b38e03f
to
4377a1d
Compare
4377a1d
to
6d0007a
Compare
As I said this PR is duplicated. We should review previous one #320 |
/ocabot merge |
This PR looks fantastic, let's merge it! |
@pedrobaeza your merge command was aborted due to failed check(s), which you can inspect on this commit of 11.0-ocabot-merge-pr-329-by-pedrobaeza-bump-no. After fixing the problem, you can re-issue a merge command. Please refrain from merging manually as it will most probably make the target branch red. |
/ocabot merge |
On my way to merge this fine PR! |
sale_line_vals.update({ | ||
'name': name, | ||
'discount': line.discount, | ||
'price_unit': line.price_unit, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this part we're missing UOM . Is it possible to add this part https://github.com/OCA/contract/pull/284/files
@pedrobaeza your merge command was aborted due to failed check(s), which you can inspect on this commit of 11.0-ocabot-merge-pr-329-by-pedrobaeza-bump-no. After fixing the problem, you can re-issue a merge command. Please refrain from merging manually as it will most probably make the target branch red. |
@sylvainvh can you attend @flotho's comment? |
Hi @sylvainvh , any chance to have a review ? Regards |
Hi, @pedrobaeza , I propose a merge and I will propose a PR. |
Hi @sylvainvh Any chance to have a review on my PR on your repo ? |
If @sylvainvh doesn't answer in a while, you can supersede this PR with a new one starting from this and adding your extra commit. |
…ata (OCA#130) * [FIX+IMP] contract: Improve usability and don't fail on wrong data * Cron create invoices masked for avoiding silent errors * New constraints for assuring data consistency * UI helps for entering consistent data * Spanish translation * Remove double company_id field on form * [FIX] contract_sale_generation: Adapt tests to upstream contract
* Change the method called in the view * Complete the create_invoice method * Bump version + authoring * Correct bad call of method Small Documentation * Add super call in python test * FIX bad field names causing bad quantities in sale.order.line
Updated by Update PO files to match POT (msgmerge) hook in Weblate.
Currently translated at 100.0% (10 of 10 strings) Translation: contract-10.0/contract-10.0-contract_sale_generation Translate-URL: https://translation.odoo-community.org/projects/contract-10-0/contract-10-0-contract_sale_generation/gl/
Currently translated at 100.0% (10 of 10 strings) Translation: contract-10.0/contract-10.0-contract_sale_generation Translate-URL: https://translation.odoo-community.org/projects/contract-10-0/contract-10-0-contract_sale_generation/de/
Currently translated at 100.0% (10 of 10 strings) Translation: contract-10.0/contract-10.0-contract_sale_generation Translate-URL: https://translation.odoo-community.org/projects/contract-10-0/contract-10-0-contract_sale_generation/fi/
6d0007a
to
250d26f
Compare
There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days. |
Syncing from upstream OCA/contract (15.0)
Migration of module contract_sale_generation
I added the field contract_id on sale.order to replace the previous field journal_id