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

[18.0][MIG] base_location_geonames_import: Migration to 18.0 #1868

Merged

Conversation

Kimkhoi3010
Copy link

@Kimkhoi3010 Kimkhoi3010 commented Oct 15, 2024

Alexis de Lattre and others added 30 commits October 15, 2024 10:05
Add support for states (if states are already present in res.country.state).
Add POT file and FR translation.

Allow to skip entries in the _prepare method.

Insist on the fact that the wizard deletes current better zip entries in the module description and in the wizard itself.
Better error message when the country code inside the file is wrong.

Add a FOR UPDATE NOWAIT at the beginning of the transaction.

Remove France-specific code ; moved to new module l10n_fr_base_location_geonames_import on lp:openerp-french-localization

Add module base_location_geonames_import

Add support for states (if states are already present in res.country.state).
Add POT file and FR translation.

Allow to skip entries in the _prepare method.

Insist on the fact that the wizard deletes current better zip entries in the module description and in the wizard itself.
Better error message when the country code inside the file is wrong.

Add a FOR UPDATE NOWAIT at the beginning of the transaction.

Remove France-specific code ; moved to new module l10n_fr_base_location_geonames_import on lp:openerp-french-localization
…rec_name defined on better.zip.geonames.import

[REF] porting to new api

[IMP] removing 'FOR UPDATE NOWAIT' as ROW EXCLUSIVE lock is already acquired by DELETE and INSERT
http://www.postgresql.org/docs/9.2/static/explicit-locking.html

[REF] select_or_create_state
and tests

[FIX] __openerp__.py PEP8

[FIX] TypeError: unlink() got multiple values for keyword argument 'context'

[IMP] using ir.config_parameter for geonames URL

[FIX] missing cr uid
…ooks

* Added Icon.
* Improve module description and extracted to README.rst.
* Pass country instead of country_id for advance comparisons.
* Allow to transform city name.
* Some code style.
* Do not remove all entries of a country, but only not found.
* Include hooks for transforming some things.
* Include spanish translation.

[FIX] base_location_geonames_import: Use self.env and recordsets
[IMP] base_location_geonames_import: Make tests more efficient

[FIX] base_location_geonames_import: Don't alias Warning

[IMP] base_location_geonames_import: Convert tests to Python + switch imported country

Monaco country is very little and it allows to save some downloaded bytes plus making
a test for entries deletion

[IMP] base_location_geonames_import: Switch author

[FIX] base_location_geonames_import: Fix test

[FIX] base_location_geonames_import: Fix tests

[MIG] Make modules uninstallable

[MIG] Rename manifest files

[FIX] base_location_geonames_import: Test logic changed

For avoiding constant problems with this test, as Monaco data changes a lot,
what we are testing now is the existence of the data, not the exact match of them.
Add option to put city name all upper case
* Tests
* New menu location
* Wizard options fixed

[IMP] base_location_geonames_import

* Speed improvement applying cache for not doing duplicated searches
  over states.
* Tests change to SavepointCase, which only passes over setUp one time.
* [IMP] base_location: Add lat & long to `better.zip`
* Add latitude and longitude columns to `better.zip`

* [IMP] base_location_geonames_import: Add lat/long
* Add support for latitude & longitude to genomes importer
Generated new readme.
Adapted the module due to the refactoring of base_location.
Adapted tests.
Currently translated at 100.0% (23 of 23 strings)

Translation: partner-contact-12.0/partner-contact-12.0-base_location_geonames_import
Translate-URL: https://translation.odoo-community.org/projects/partner-contact-12-0/partner-contact-12-0-base_location_geonames_import/es/
If there are several cities with the same name in different states, previous code doesn't
handle correctly this situation.

We amend this storing in the cached dictionary both city name and state.

Includes a test for checking this specific condition, got from real data in US.

Fixes OCA#749
Currently translated at 100.0% (23 of 23 strings)

Translation: partner-contact-12.0/partner-contact-12.0-base_location_geonames_import
Translate-URL: https://translation.odoo-community.org/projects/partner-contact-12-0/partner-contact-12-0-base_location_geonames_import/pt_BR/
[W0622(redefined-builtin), TestBaseLocationGeonamesImport.test_import_title] Redefining built-in 'zip'
* Performance was very poor due to several reasons:

  * City name translatable.
  * Lack on indexes when deleting obsolete records.
  * Some prefetching.
* City was not being searched also by state, having the chance of returning
  incorrect matches if same city name on different states.
* Previous code was removing all previous records of zip entries!
* Remove possible obsolete FK keys after migrating from v11.
Introducing second variable has provoked an error on that part.
Currently translated at 100.0% (24 of 24 strings)

Translation: partner-contact-13.0/partner-contact-13.0-base_location_geonames_import
Translate-URL: https://translation.odoo-community.org/projects/partner-contact-13-0/partner-contact-13-0-base_location_geonames_import/pt_BR/
In that case, using remove would raise a KeyError.
On the other hand, discard fails silently.
jakobkrabbe and others added 4 commits October 15, 2024 10:05
Currently translated at 100.0% (25 of 25 strings)

Translation: partner-contact-17.0/partner-contact-17.0-base_location_geonames_import
Translate-URL: https://translation.odoo-community.org/projects/partner-contact-17-0/partner-contact-17-0-base_location_geonames_import/sv/
Currently translated at 100.0% (25 of 25 strings)

Translation: partner-contact-17.0/partner-contact-17.0-base_location_geonames_import
Translate-URL: https://translation.odoo-community.org/projects/partner-contact-17-0/partner-contact-17-0-base_location_geonames_import/ca/
@Kimkhoi3010 Kimkhoi3010 force-pushed the 18.0-mig-base_location_geonames_import branch from 39788fe to 6908dd9 Compare October 15, 2024 09:49
@Kimkhoi3010 Kimkhoi3010 mentioned this pull request Oct 16, 2024
46 tasks
@sebalix
Copy link
Contributor

sebalix commented Nov 28, 2024

Hello @Kimkhoi3010 , you can now remove your DON'T MERGE commit, the dependency has been merged, thank you!

@Kimkhoi3010 Kimkhoi3010 force-pushed the 18.0-mig-base_location_geonames_import branch 3 times, most recently from 5308374 to 712137a Compare December 2, 2024 05:16
@OCA-git-bot
Copy link
Contributor

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

Please include #1911

pedrobaeza and others added 3 commits December 4, 2024 18:35
Befor this commit, there were 2 possible errors re-importing ZIPs:

- If city names are changed on the import process (like capitalization),
  the code for detecting existing entries was not taking that into
  account, failing to find the city.
- If there's more than one found result, using `discard` for removing
  the result from the ZIPs to remove gives an error.

TT52020
@Kimkhoi3010 Kimkhoi3010 force-pushed the 18.0-mig-base_location_geonames_import branch from 712137a to 9d9eac3 Compare December 4, 2024 11:36
@Kimkhoi3010
Copy link
Author

@pedrobaeza I updated by your comment. Thanks for your time!

Copy link
Member

@pedrobaeza pedrobaeza 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 nobump

@OCA-git-bot
Copy link
Contributor

Hey, thanks for contributing! Proceeding to merge this for you.
Prepared branch 18.0-ocabot-merge-pr-1868-by-pedrobaeza-bump-nobump, awaiting test results.

@OCA-git-bot
Copy link
Contributor

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

@OCA-git-bot OCA-git-bot merged commit f1ef6f7 into OCA:18.0 Dec 4, 2024
7 checks passed
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.