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.

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

[WMS][12.0] - stock_location_zone - alpha version #653

Merged
merged 8 commits into from
Sep 11, 2019
Merged

[WMS][12.0] - stock_location_zone - alpha version #653

merged 8 commits into from
Sep 11, 2019

Conversation

jbaudoux
Copy link
Contributor

@jbaudoux jbaudoux commented Jul 12, 2019 •
edited by jgrandguillaume
Loading

@jgrandguillaume jgrandguillaume changed the title [ADD] 12.0 - stock_location_zone [WIP] 12.0 - stock_location_zone Jul 12, 2019
@jgrandguillaume jgrandguillaume changed the title [WIP] 12.0 - stock_location_zone [WIP][12.0] - stock_location_zone Jul 12, 2019
@jgrandguillaume jgrandguillaume changed the title [WIP][12.0] - stock_location_zone [WIP][WMS][12.0] - stock_location_zone Jul 12, 2019
@jbaudoux jbaudoux force-pushed the 12.0-add-stock_location_zone branch from f49960e to 7c322fe Compare July 12, 2019 13:53
@jbaudoux jbaudoux force-pushed the 12.0-add-stock_location_zone branch from 7c322fe to 3f4f540 Compare July 12, 2019 14:46
@jgrandguillaume jgrandguillaume mentioned this pull request Jul 15, 2019
Closed
@jbaudoux jbaudoux force-pushed the 12.0-add-stock_location_zone branch 2 times, most recently from 9848456 to 83cdfbf Compare August 15, 2019 07:48
@jbaudoux jbaudoux force-pushed the 12.0-add-stock_location_zone branch from 83cdfbf to d9527d6 Compare August 15, 2019 07:57
@guewen guewen mentioned this pull request Aug 15, 2019
Closed
@max3903 max3903 added this to the 12.0 milestone Aug 20, 2019
@guewen
Copy link
Member

guewen commented Aug 27, 2019

I'll push a commit

@guewen
Add corrections to stock_location_zone
545a8b4 
* Allow copy of stock locations (was blocked by constraint on unique name)
* Correct loop in _compute_name returning too early if a record had no
parent with a 'location_name_format'
* Rename field pick_type_id to picking_type_id for coherency
* Add missing _description on stock.picking.zone
* Correct location_name_format format when the record is a NewId
@guewen
fixup! Add corrections to stock_location_zone
00dd814
@guewen
Replace SQL unique constraint by a python one
dd28e0e 
The new constraints prevents having 2 locations with the same name in
the same zone. The SQL UNIQUE constraint was blocking the installation
of some addons due to demo data creating, by default, some locations
with the same name for the warehouses' locations.

Alternative: we could create an INDEX UNIQUE (name, location_id) WHERE
picking_zone_id IS NOT NULL
But it depends if we want to check the whole tree or not.
jgrandguillaume
jgrandguillaume approved these changes Aug 28, 2019
View reviewed changes
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.

LGTM, functional test ok. To add the development status alpha and ready to merge

@guewen
Use unique index for unique location name per zone
508519a 
The python one was much too slow with large datasets. Here we only
check one level (only when the zone is directly assigned), the previous
SQL constraint was checking the locations of the same level only anyway.
@guewen
Set Alpha development status
a6998d6 
* Add copyright header
* Add myself as contributor of stock_location_zone
* Fix lint
@guewen guewen force-pushed the 12.0-add-stock_location_zone branch from 9972bdc to a6998d6 Compare August 28, 2019 13:04
@guewen guewen changed the title [WIP][WMS][12.0] - stock_location_zone [WMS][12.0] - stock_location_zone Aug 28, 2019
@guewen guewen changed the title [WMS][12.0] - stock_location_zone [WMS][12.0] - stock_location_zone - alpha version Aug 28, 2019
grindtildeath
grindtildeath approved these changes Sep 10, 2019
View reviewed changes
Copy link
Contributor

@grindtildeath grindtildeath left a comment

Choose a reason for hiding this comment

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

Approved to add more changes later.

@jgrandguillaume
Copy link
Member

/ocabot merge

@OCA-git-bot
Copy link
Contributor

Hey, thanks for contributing! Proceeding to merge this for you.
Prepared branch 12.0-ocabot-merge-pr-653-by-jgrandguillaume-bump-no, awaiting test results.

OCA-git-bot added a commit that referenced this pull request Sep 10, 2019
@OCA-git-bot
Merge PR #653 into 12.0
1b675f3 
Signed-off-by jgrandguillaume
@OCA-git-bot
Copy link
Contributor

@jgrandguillaume your merge command was aborted due to failed check(s), which you can inspect on this commit of 12.0-ocabot-merge-pr-653-by-jgrandguillaume-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.

@jgrandguillaume
Copy link
Member

/ocabot merge

I try again as Travis was red because of unaccessibility...

OCA-git-bot added a commit that referenced this pull request Sep 11, 2019
@OCA-git-bot
Merge PR #653 into 12.0
373d958 
Signed-off-by jgrandguillaume
@OCA-git-bot
Copy link
Contributor

Hey, thanks for contributing! Proceeding to merge this for you.
Prepared branch 12.0-ocabot-merge-pr-653-by-jgrandguillaume-bump-no, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit a6998d6 into OCA:12.0 Sep 11, 2019
@OCA-git-bot
Copy link
Contributor

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

@rousseldenis
Copy link
Sponsor Contributor

@jbaudoux Sorry arriving late in this.

Just a comment about sql constraint with WHERE.

Actually, you can use _sql_constraints. See what I did there https://github.com/OCA/partner-contact/blob/10.0/partner_bank_sort_code/models/res_bank.py#L14

You can use sql clause EXCLUDE.

See https://www.postgresql.org/docs/11/ddl-constraints.html#DDL-CONSTRAINTS-EXCLUSION

or https://www.cybertec-postgresql.com/en/postgresql-exclude-beyond-unique/

@grindtildeath grindtildeath mentioned this pull request Sep 11, 2019
Merged
@grindtildeath
Copy link
Contributor

grindtildeath commented Sep 11, 2019 •
edited
Loading

@rousseldenis Thanks for the hint, anyway I took care of this in #711 where it's not needed after simplification

EDIT: Was still needed and your solution works like a charm :)

This was referenced Sep 11, 2019
Closed
Closed
@jgrandguillaume jgrandguillaume mentioned this pull request Sep 13, 2019
Closed
32 tasks
@jgrandguillaume jgrandguillaume mentioned this pull request May 28, 2020
Closed
62 tasks
manuelcalerosolis pushed a commit to xtendoo-corporation/stock-logistics-warehouse that referenced this pull request Oct 21, 2020
@OCA-git-bot
Merge PR OCA#653 into 12.0
0462c06 
Signed-off-by pedrobaeza
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@guewen guewen guewen left review comments

@jgrandguillaume jgrandguillaume jgrandguillaume approved these changes

@grindtildeath grindtildeath grindtildeath approved these changes

Assignees
No one assigned
Labels
approved merged 🎉
Projects
None yet
Milestone
12.0
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@jbaudoux @guewen @jgrandguillaume @OCA-git-bot @rousseldenis @grindtildeath @max3903