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

[12.0][MIG] stock_cycle_count: Migration to 12.0 #625

Merged
merged 37 commits into from Jul 8, 2019

Conversation

bodedra
Copy link
Member

@bodedra bodedra commented Jun 24, 2019

@bodedra bodedra force-pushed the bodedra_12.0-mig-stock_cycle_count branch from 3a1aeb2 to 1836db8 Compare June 24, 2019 12:52
@rousseldenis rousseldenis added this to the 12.0 milestone Jun 24, 2019
@OCA-git-bot OCA-git-bot mentioned this pull request Jun 24, 2019
54 tasks
@bodedra bodedra force-pushed the bodedra_12.0-mig-stock_cycle_count branch 2 times, most recently from 0c78e87 to c12e41a Compare June 24, 2019 14:36
@bodedra
Copy link
Member Author

bodedra commented Jun 25, 2019

@jbeficent @lreficent @aheficent

Would you please help me find out root cause to fail ('Zero confirmation not being created.') test case ?

Screenshot_2019-06-25 Job #3890 3 - OCA stock-logistics-warehouse - Travis CI

@LoisRForgeFlow LoisRForgeFlow self-requested a review July 3, 2019 11:47
@bodedra bodedra force-pushed the bodedra_12.0-mig-stock_cycle_count branch from c12e41a to 794928e Compare July 4, 2019 12:29
@LoisRForgeFlow
Copy link
Contributor

LoisRForgeFlow commented Jul 5, 2019

@bodedra just opened a PR to fix the remaining issue: ursais#2. Could you review?

@bodedra
Copy link
Member Author

bodedra commented Jul 5, 2019

@lreficent

480b6216b16fb1a21acc318c3d805f53

Thanks for your fix.

Now all test case are passed. Still ci/runbot fail.

@bodedra bodedra force-pushed the bodedra_12.0-mig-stock_cycle_count branch from 5e33471 to acec4c5 Compare July 5, 2019 15:25
@@ -40,8 +39,7 @@ def get_horizon_date(self):
@api.model
def _get_cycle_count_locations_search_domain(
self, parent):
domain = [('parent_left', '>=', parent.parent_left),
('parent_right', '<=', parent.parent_right),
domain = [('parent_path', '=like', parent.parent_path + '%'),
Copy link
Sponsor Contributor

Choose a reason for hiding this comment

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

I'm wondering why child of/parent of would not works here ?

See https://github.com/odoo/odoo/blob/12.0/odoo/osv/expression.py#L755

Copy link
Member Author

@bodedra bodedra Jul 5, 2019

Choose a reason for hiding this comment

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

Copy link
Sponsor Contributor

Choose a reason for hiding this comment

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

Yeah, that's weird because it's in the same commit that implements parent_path to optimize queries...

Copy link
Member Author

Choose a reason for hiding this comment

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

Additionally, parent_left and parent_right fields are removed from stock.location object.

Copy link
Sponsor Contributor

Choose a reason for hiding this comment

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

Yes, that was the aim of parent_path: to optimize child of/parent of search operators, so, normally, you should use them as before.

Copy link
Member Author

Choose a reason for hiding this comment

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

so what do you recommend on these ?

Copy link
Sponsor Contributor

@rousseldenis rousseldenis left a comment

Choose a reason for hiding this comment

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

Code review

@rousseldenis
Copy link
Sponsor Contributor

@bodedra Could you squash your commits ?

@bodedra
Copy link
Member Author

bodedra commented Jul 5, 2019

@rousseldenis Sure.

@bodedra bodedra force-pushed the bodedra_12.0-mig-stock_cycle_count branch from acec4c5 to f76efd9 Compare July 5, 2019 16:47
@bodedra bodedra changed the title [WIP][12.0][MIG] stock_cycle_count: Migration to 12.0 [12.0][MIG] stock_cycle_count: Migration to 12.0 Jul 5, 2019
@bodedra
Copy link
Member Author

bodedra commented Jul 6, 2019

@rousseldenis @mreficent Thanks for your review and suggestion.

@lreficent can you update your review ?

@LoisRForgeFlow LoisRForgeFlow force-pushed the bodedra_12.0-mig-stock_cycle_count branch from f76efd9 to 8387920 Compare July 8, 2019 06:54
Bhavesh Odedra and others added 2 commits July 8, 2019 08:57
Now zero-qty quants are not removed straight away but by a cron job.
Therefore, we have to also check that the quants are not zero-qty ones.
@LoisRForgeFlow LoisRForgeFlow force-pushed the bodedra_12.0-mig-stock_cycle_count branch from 8387920 to e19fd84 Compare July 8, 2019 06:58
Copy link
Contributor

@LoisRForgeFlow LoisRForgeFlow left a comment

Choose a reason for hiding this comment

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

LGTM (just fixed history a little to recover my contribution to the migration)

Let's merge after travis goes green.

@LoisRForgeFlow
Copy link
Contributor

/ocabot merge

@OCA-git-bot
Copy link
Contributor

What a great day to merge this nice PR. Let's do it!
Rebased to 12.0-ocabot-merge-pr-625-by-lreficent-bump-no, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit e19fd84 into OCA:12.0 Jul 8, 2019
@OCA-git-bot
Copy link
Contributor

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

PS: Don't worry if GitHub says there are unmerged commits: it is due to a rebase before merge. All commits of this PR have been merged into 12.0.

OCA-git-bot added a commit that referenced this pull request Jul 8, 2019
Signed-off-by lreficent
@bodedra bodedra deleted the bodedra_12.0-mig-stock_cycle_count branch July 8, 2019 08:59
manuelcalerosolis pushed a commit to xtendoo-corporation/stock-logistics-warehouse that referenced this pull request Oct 21, 2020
Signed-off-by LoisRForgeFlow
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.

None yet

9 participants