Skip to content

[12.0][FIX] missing sale_order_id #2288

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.

Sign up for GitHub

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

Jump to bottom
Merged
OCA-git-bot merged 1 commit into from
May 1, 2020
Merged

[12.0][FIX] missing sale_order_id #2288

OCA-git-bot merged 1 commit into from
May 1, 2020

Conversation

@sergiocorato
Copy link

@sergiocorato sergiocorato commented Apr 10, 2020

Description of the issue/feature this PR addresses: missing sale order id on project.project

Current behavior before PR: sql constrains on sale_order_id fail

Desired behavior after PR is merged: sql constrains do not fail

--
I confirm I have signed the CLA and read the PR guidelines at www.odoo.com/submit-pr

legalsylvain
legalsylvain approved these changes Apr 10, 2020
View reviewed changes
Copy link
Contributor

@legalsylvain legalsylvain left a comment

Choose a reason for hiding this comment

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

Lgtm. Thanks

@pedrobaeza pedrobaeza added this to the 12.0 milestone Apr 10, 2020
@pedrobaeza
Copy link
Member

pedrobaeza commented Apr 10, 2020

Please explain why is this needed, and modify accordingly openupgrade_analisis_work.txt and maybe module documentation (from Nothing to do to Done if applicable).

@sergiocorato
Copy link
Author

sergiocorato commented Apr 10, 2020 •
edited
Loading

Please explain why is this needed, and modify accordingly openupgrade_analisis_work.txt and maybe module documentation (from Nothing to do to Done if applicable).

This constrains fail because migration in v. 11.0 create the sale_line_id here but in v. 12 is added the field sale_order_id here

pedrobaeza
pedrobaeza reviewed Apr 11, 2020
View reviewed changes
MiquelRForgeFlow
MiquelRForgeFlow approved these changes Apr 14, 2020
View reviewed changes
Copy link
Contributor

@MiquelRForgeFlow MiquelRForgeFlow left a comment

Choose a reason for hiding this comment

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

squash

@sergiocorato sergiocorato force-pushed the 12.0_fix_sale_timesheet_order branch from 278c008 to d2bd30a Compare April 14, 2020 09:08
@sergiocorato
Copy link
Author

sergiocorato commented Apr 14, 2020

done!

ivantodorovich
ivantodorovich approved these changes Apr 15, 2020
View reviewed changes
Copy link

@ivantodorovich ivantodorovich left a comment

Choose a reason for hiding this comment

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

Don't know about @pedrobaeza 's comment:

... modify accordingly openupgrade_analisis_work.txt and maybe module documentation (from Nothing to do to Done if applicable).

But otherwise LGTM 👍

@OCA-git-bot
Copy link
Contributor

OCA-git-bot commented Apr 15, 2020

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). 🤖

@pedrobaeza
Copy link
Member

pedrobaeza commented Apr 15, 2020

I mean to annotate this:

https://github.com/OCA/OpenUpgrade/blob/d2bd30a700a948cde0ceb393de7fb68c2b02faa9/addons/sale_timesheet/migrations/12.0.1.0/openupgrade_analysis_work.txt#L15

legalsylvain
legalsylvain requested changes May 1, 2020
View reviewed changes
Copy link
Contributor

@legalsylvain legalsylvain left a comment

Choose a reason for hiding this comment

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

@sergiocorato could change the analysis file, regarding @pedrobaeza comment ?

thanks !

@sergiocorato sergiocorato force-pushed the 12.0_fix_sale_timesheet_order branch from d2bd30a to 8ddc9b1 Compare May 1, 2020 12:27
@sergiocorato
Copy link
Author

sergiocorato commented May 1, 2020

I missed it, done!

@sergiocorato sergiocorato changed the title [FIX] missing sale_order_id [12.0][FIX] missing sale_order_id May 1, 2020
legalsylvain
legalsylvain approved these changes May 1, 2020
View reviewed changes
Copy link
Contributor

@legalsylvain legalsylvain left a comment

Choose a reason for hiding this comment

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

Thanks.

/ocabot merge

@OCA-git-bot
Copy link
Contributor

OCA-git-bot commented May 1, 2020

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). 🤖

@legalsylvain
Copy link
Contributor

legalsylvain commented May 1, 2020

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

OCA-git-bot commented May 1, 2020

Hey, thanks for contributing! Proceeding to merge this for you.
Prepared branch 12.0-ocabot-merge-pr-2288-by-legalsylvain-bump-nobump, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit c6e9043 into OCA:12.0 May 1, 2020
@OCA-git-bot
Copy link
Contributor

OCA-git-bot commented May 1, 2020

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

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

Reviewers

@pedrobaeza pedrobaeza pedrobaeza left review comments

@legalsylvain legalsylvain legalsylvain approved these changes

@MiquelRForgeFlow MiquelRForgeFlow MiquelRForgeFlow approved these changes

+1 more reviewer

@ivantodorovich ivantodorovich ivantodorovich approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

approved merged 🎉 needs review ready to merge

Projects

None yet

Milestone

12.0

Development

Successfully merging this pull request may close these issues.

6 participants

@sergiocorato @pedrobaeza @OCA-git-bot @legalsylvain @ivantodorovich @MiquelRForgeFlow