-
-
Notifications
You must be signed in to change notification settings - Fork 698
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
[16.0][OU-ADD] delivery: migration #4112
Conversation
/ocabot migration delivery Depends on :
|
Bianca, I'm not familiar with the Eire postal system, but I found before the problem of different zip code ranges. For example, you could have:
In case it goes from D01 to D99, then no need for a second zone, as D6W is included in that range (just after D5Z ZZZZ and before D70 000). |
Continuing work on this at OCA Days sprint |
Let me know if you want to discuss something. |
Possible solutions:
I don't know how Odoo EE solves this problem. I'm inclined to say that we just log a warning with an explanation to the user that they should fix this manually. It's a rubbish solution, but the other ones aren't great either. Do you have thoughts @pedrobaeza ? |
Hi, Carmen, welcome to the wild OpenUpgrade, where hard decisions (or lots of work) have to be made! OpenUpgrade should preserve at maximum the information of the previous version, so we shouldn't opt for the option on not doing anything. I don't get why Odoo has gone to this ugly format. It's OK to admit regexes, but they shouldn't limit the possibilities to only that. Keep the intervals on this new model! But we are there at least for this version, so we have to tackle it. I think the only viable option to go is the 1st, and replicate the tooling for creating the regexp for the interval. The second one will provoke a lot of noise. |
Signed-off-by: Carmen Bianca BAKKER <[email protected]>
Signed-off-by: Carmen Bianca BAKKER <[email protected]>
c6794f4
to
38cebdf
Compare
I do not know why the CI is broken on some stock thing. It appears entirely unrelated, but the 16.0 branch is passing fine. |
That's because |
Ah. That's a little unfortunate. Thank you @pedrobaeza |
prefixes = range_to_prefixes(method.zip_from, method.zip_to) | ||
except Exception as error: | ||
_logger.error( | ||
f"Failed to convert the zip range '{method.zip_from} --" |
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.
hello, , If this code is performed from odoo 16.0 environment, make a AttributeError: 'delivery.carrier' object has no attribute 'zip_from', because zip_from is no longer in 16.0
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.
ah, I may need to directly query the database for these values.
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.
@carmenbianca any update on this?
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 this huge reflexion and work!
One more comment. Btw, do you plan on fixing the script or do you need some help?
Thanks
|
||
@openupgrade.migrate() | ||
def migrate(env, version): | ||
delivery_methods = env["delivery.carrier"].search([]) |
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.
One may restrict to delivery carriers having values in zip_from
and / or zip_to
.
(As @dansanti mentioned it, you should query this info in DB)
Superseded and completed by #4370 Thanks Carmen from the excellent work done so far. |
So this is a work-in-progress. I'm writing the challenges involved before having a go at tackling this problem. I need to clearly write the problem before I can start, and I figure I might as well write the thoughts publicly.
For delivery methods,
zip_from
andzip_to
were changed to 'zip prefixes'.The old method worked like this. If you wanted a delivery method to only apply to destinations in lovely Brussels, you would set
zip_from
to '1000' andzip_to
to '1200'. All numbers between those two values cover all possible postal codes for the Brussels Capital Region. Each (entire) municipality has its own concrete number.However, the world is scarcely always so simple. To take just one example, in Ireland, they use Eircode. So if you wanted to restrict a delivery method to Dublin, you would need to match all the following prefixes:
An example Eircode postal code looks like 'D04 1234', where 'D04' maps to Dublin 4 (a part of Dublin), and '1234' is a unique identifier for any address in that part of Dublin. In fact, a different apartment number in the same building may have a totally different unique identifier. The unique identifier can use any alphanumeric character (except 'O', and maybe some others, but it's not extremely clear to me, and it doesn't matter very much for this example).
In the case of Dublin, I'm not sure how the Odoo 15 system ever worked.
zip_from
= D01 0000,zip_to
= D6W ZZZZ? Or D24 ZZZZ? Or D99 ZZZZ (which doesn't exist)? Does Odoo handle letter sorting like Python?The new Odoo 16 system is relatively simple. Instead of defining a single range, you can define any number of 'prefixes'. For Brussels, this means that you can use ['1'] (no other postal code in Belgium uses >1200,<2000). For Dublin, you can use ['D'] (Dublin is the only city in Ireland whose postal code begins with D). However, if you wanted to, you could also define ['10', '11', '12'] for Brussels, and the big list of Dublin postal codes from the above list.
The new system has one more trick up its sleeve: you can use regexes!
Enough prelude. Let's solve this problem. We need to translate the old system to the new system in a way that accounts for all possible postal code systems. Or at least: postal code systems that are hierarchical when read from left to right (or sequentially), elsewise prefixes and ranges make no sense to begin with.
I just don't really know how. I happen to know that Brussels (
zip_from
= 1000,zip_to
= 1200) maps to prefixes ['1'] or ['10', '11', '12'], but I couldn't confidently say that without my pre-existing knowledge. In an alternate universe, there may be another Belgian municipality outside of Brussels that uses postal code '1201'. So I guess you'd need to write a regex that maps to any alphanumeric value between '1000' and '1200' (incl)? That sounds doable, even if it's a minor exercise in masochism, but it gets even more complicated with esoteric systems like Eircode. And what if there exists some postal code system that uses non-Latin characters that aren't easily included in the regex?I'll have another look at this later.