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

[14.0][MIG] base_location_geonames_import #1024

Closed

Conversation

alexis-via
Copy link
Contributor

Replaces PR #1016

The Geonames service is currently down, cf https://download.geonames.org/export/zip/ that only has 1 zip file, so I could not test.

I removed the INSTALL part of README because requests is now required for the base module of odoo v14.

Alexis de Lattre and others added 30 commits November 26, 2020 15:37
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/
This is a rare case where Black needs some
manual help to provide a beautiful output.
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

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/
[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.
@OCA-git-bot
Copy link
Contributor

This PR looks fantastic, let's merge it!
Prepared branch 14.0-ocabot-merge-pr-1024-by-pedrobaeza-bump-nobump, awaiting test results.

OCA-git-bot added a commit that referenced this pull request Nov 30, 2020
Signed-off-by pedrobaeza
Copy link

@OsoTranquilo OsoTranquilo left a comment

Choose a reason for hiding this comment

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

It works properly.

Copy link

@OsoTranquilo OsoTranquilo left a comment

Choose a reason for hiding this comment

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

Works OK

@pedrobaeza
Copy link
Member

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

This PR looks fantastic, let's merge it!
Prepared branch 14.0-ocabot-merge-pr-1024-by-pedrobaeza-bump-nobump, awaiting test results.

OCA-git-bot added a commit that referenced this pull request Dec 4, 2020
Signed-off-by pedrobaeza
@OsoTranquilo
Copy link

@pedrobaeza what's necessary for travis run again?

@pedrobaeza
Copy link
Member

I'm afraid this is another problem: OCA/oca-addons-repo-template#25

@OsoTranquilo
Copy link

I'm afraid this is another problem: OCA/oca-addons-repo-template#25

Do I understand that this is already fixed?
Can you try passing Travis now?

@pedrobaeza
Copy link
Member

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

On my way to merge this fine PR!
Prepared branch 14.0-ocabot-merge-pr-1024-by-pedrobaeza-bump-nobump, awaiting test results.

OCA-git-bot added a commit that referenced this pull request Dec 12, 2020
Signed-off-by pedrobaeza
@HaraldPanten
Copy link
Contributor

What should be done here? Relaunch Travis?

@pedrobaeza
Copy link
Member

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

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

OCA-git-bot added a commit that referenced this pull request Dec 15, 2020
Signed-off-by pedrobaeza
@OCA-git-bot
Copy link
Contributor

@pedrobaeza your merge command was aborted due to failed check(s), which you can inspect on this commit of 14.0-ocabot-merge-pr-1024-by-pedrobaeza-bump-nobump.

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.

@pedrobaeza
Copy link
Member

Migration is not complete, so I have finished it in #1039.

@pedrobaeza pedrobaeza closed this Dec 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.