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] database_cleanup: Migration to 18.0 #9

Conversation

nguyenminhchien
Copy link

It found only 3 commits to port:

  • 1 ported successfully: d65c78c
  • 2 were ignored, i added them to blacklist: 7e81317

@nguyenminhchien nguyenminhchien marked this pull request as ready for review October 2, 2024 10:38
Copy link
Member

@sebalix sebalix left a comment

Choose a reason for hiding this comment

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

Hello, thank you @nguyenminhchien !

LG, some questions/comments

Comment on lines 59 to 65
def tearDownClass(self):
super().tearDownClass()
with environment() as env:
model = env["ir.model"].search(
[("name", "=", "Database cleanup test field-model")]
)
if model:
model.unlink()
Copy link
Member

Choose a reason for hiding this comment

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

Curiosity: why this is now needed? Any error or warning raised by Odoo regarding this data model?

Otherwise I would store the model name ""Database cleanup test field-model" in a class attribute to not copy it.

Copy link
Author

Choose a reason for hiding this comment

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

Hi, the model Database cleanup test field-model is added inside the new env with environment() as env: (at the line 22). It will not be deleted after the test.

So, i have to delete it in def tearDownClass(self), otherwise i will face an error when running the test from the second time:

bad query: b'INSERT INTO "ir_model" ("create_date", "create_uid", "model", "name", "order", "state", "write_date", "write_uid") VALUES (\'2024-10-10 03:29:45.693857\', 1, \'x_database.cleanup.test.field.model\', \'{"en_US": "Database cleanup test field-model"}\', \'id\', \'manual\', \'2024-10-10 03:29:45.693857\', 1) RETURNING "id"'
ERROR: duplicate key value violates unique constraint "ir_model_obj_name_uniq"
DETAIL:  Key (model)=(x_database.cleanup.test.field.model) already exists.

super().tearDownClass()
with environment() as env:
module = env["ir.module.module"].search(
[("name", "=", "database_cleanup_test")]
Copy link
Member

Choose a reason for hiding this comment

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

Module name could be store in a class attribute

super().tearDownClass()
with environment() as env:
record = env["ir.property"].search(
[("value_text", "=", "My default partner name")]
Copy link
Member

Choose a reason for hiding this comment

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

Same remark

@nguyenminhchien nguyenminhchien force-pushed the 18.0-mig-database_cleanup branch from 810f96d to 49ca6ee Compare October 10, 2024 04:16
Stefan Rijnhart and others added 26 commits October 10, 2024 11:29
avoid ''NoneType' object has no attribute 'exists'' error when purging models
fix my change [email protected], wrong lines replaced...
    raw SQL query (but never read afterwards). Workaround for
    lp:1277899

[FIX] Preserve dangling workflow table which is in use

[RFR] Group models per table when detecting columns to purge
      to prevent problems with models sharing the same table

[ADD] Allow purging of dangling data entries

[FIX] Data purging now working

[IMP] Docstrings

[FIX] Label
[FIX] Catch attempt to unlink field from nonexisting model

[RFR] Flake8
[CHG] database_cleanup: move description to README.rst
[IMP] order wizard lines by name

[IMP] deal with modules whose models can't be loaded

[IMP] double quotes for docstring

[FIX] use exists query instead of huge in list

[IMP] hide unnecessary buttons in wizard II

[IMP] readability

[FIX] cope with purging nonexisting models
[ADD] test purging modules

[ADD] test purging tables
[FIX] database_cleanup reloads the registry

which has weird side effects during testing. Take care
database_cleanup's tests don't mess up the following tests
* [FIX] database_cleanup: Isolate build
* Isolate `database_cleanup` into its own build in Travis file to fix OCA#689

* [FIX] database_cleanup: Remove KeyError assertion
* Remove KeyError assertion in tests due to PR in comment being merged
* [ADD] allow creating missing indexes

* [FIX] tests; installation

* [ADD] allow purging properties

* [ADD] missing file

* [ADD] test purging properties

* [ADD] missing parent_id for menu entry

* [FIX] don't delete too many and wrong properties
Using new base model inheritance.
[FIX] don't try to uninstall uninstalled modules

[DEL] weird code

[FIX] actually cleanup where we can
oca-ci and others added 26 commits October 10, 2024 11:29
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: server-tools-17.0/server-tools-17.0-database_cleanup
Translate-URL: https://translation.odoo-community.org/projects/server-tools-17-0/server-tools-17-0-database_cleanup/
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: server-tools-17.0/server-tools-17.0-database_cleanup
Translate-URL: https://translation.odoo-community.org/projects/server-tools-17-0/server-tools-17-0-database_cleanup/
Currently translated at 100.0% (75 of 75 strings)

Translation: server-tools-17.0/server-tools-17.0-database_cleanup
Translate-URL: https://translation.odoo-community.org/projects/server-tools-17-0/server-tools-17-0-database_cleanup/it/
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: server-tools-17.0/server-tools-17.0-database_cleanup
Translate-URL: https://translation.odoo-community.org/projects/server-tools-17-0/server-tools-17-0-database_cleanup/
Currently translated at 100.0% (81 of 81 strings)

Translation: server-tools-17.0/server-tools-17.0-database_cleanup
Translate-URL: https://translation.odoo-community.org/projects/server-tools-17-0/server-tools-17-0-database_cleanup/it/
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: server-tools-17.0/server-tools-17.0-database_cleanup
Translate-URL: https://translation.odoo-community.org/projects/server-tools-17-0/server-tools-17-0-database_cleanup/
Currently translated at 100.0% (84 of 84 strings)

Translation: server-tools-17.0/server-tools-17.0-database_cleanup
Translate-URL: https://translation.odoo-community.org/projects/server-tools-17-0/server-tools-17-0-database_cleanup/it/
Currently translated at 100.0% (84 of 84 strings)

Translation: server-tools-17.0/server-tools-17.0-database_cleanup
Translate-URL: https://translation.odoo-community.org/projects/server-tools-17-0/server-tools-17-0-database_cleanup/zh_CN/
Currently translated at 100.0% (84 of 84 strings)

Translation: server-tools-17.0/server-tools-17.0-database_cleanup
Translate-URL: https://translation.odoo-community.org/projects/server-tools-17-0/server-tools-17-0-database_cleanup/es_AR/
@nguyenminhchien
Copy link
Author

@sebalix i updated by your comments, and created a new PR OCA#3064 to OCA/server-tools.
(This PR was closed auto after rebased)

@sebalix
Copy link
Member

sebalix commented Oct 10, 2024

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.