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

[MIG] 10.0 db cleanup #894

Merged
merged 15 commits into from Sep 6, 2017
Merged

[MIG] 10.0 db cleanup #894

merged 15 commits into from Sep 6, 2017

Conversation

ecino
Copy link

@ecino ecino commented Jul 10, 2017

Takes #607 with 9.0 commits for module database_cleanup.

@pedrobaeza
Copy link
Member

Please squash together "OCA Transbot" commits.

oca-transbot and others added 5 commits July 10, 2017 18:15
* [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
@ecino
Copy link
Author

ecino commented Jul 10, 2017

Squashed some transbot commits (not all to preserve the order)

@yajo yajo added this to the 10.0 milestone Jul 11, 2017
@ecino
Copy link
Author

ecino commented Jul 11, 2017

Strange that no ir.property are present in the test database, tests are failing because of this. Any ideas?

@pedrobaeza
Copy link
Member

Create some of them in your setUp phase to see

@ecino ecino force-pushed the 10.0-db-cleanup branch 3 times, most recently from 0d9210c to a958d8c Compare July 11, 2017 08:31
@ecino
Copy link
Author

ecino commented Jul 11, 2017

Tests are fixed 😄

'views/menu.xml',
],
'installable': False,
'installable': True,
"application": True,
Copy link
Member

Choose a reason for hiding this comment

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

Not an application IMHO

Copy link
Author

Choose a reason for hiding this comment

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

You're right, I removed it.

<field name="type">ir.actions.server</field>
<field name="state">code</field>
<field name="model_id" ref="database_cleanup.model_cleanup_purge_wizard_property" />
<field name="code">action = self.get_wizard_action(cr, uid, context=context)</field>
Copy link
Member

Choose a reason for hiding this comment

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

I get spurious errors

ValueError: <type 'exceptions.NameError'>: "name 'self' is not defined" while evaluating u'action = self.get_wizard_action(cr, uid, context=context)'

when clicking on some of the menu items. It seems to help to replace the code contents of lines like this with

action = env['cleanup.purge.wizard.property'].get_wizard_action()

Copy link
Author

Choose a reason for hiding this comment

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

Corrected

Copy link
Member

Choose a reason for hiding this comment

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

Thanks! You missed one, the one I referred to initially.

Copy link
Author

Choose a reason for hiding this comment

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

Sorry for that, it's now included

</footer>
</form>
<div attrs="{'invisible': [('purge_line_ids', '!=', [])]}">
Nothing found to clean up.
Copy link
Member

Choose a reason for hiding this comment

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

This always shows up, even when there are result lines present. Maybe the way to check for an empty one2many widget has changed.

image

Copy link
Author

Choose a reason for hiding this comment

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

I tried to figure out how to fix it but didn't came out with a working solution. By inspecting odoo source code I found code which is exactly the same as here. Any help from others would be appreciated.

Copy link
Member

Choose a reason for hiding this comment

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

The problem is that the div is in the field tag itself. It works when the div is moved before the field tag.

Copy link
Author

Choose a reason for hiding this comment

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

Thanks, now it works.

@ecino ecino force-pushed the 10.0-db-cleanup branch 2 times, most recently from 9176134 to 28c6d37 Compare July 26, 2017 13:32
<field name="type">ir.actions.server</field>
<field name="state">code</field>
<field name="model_id" ref="database_cleanup.model_cleanup_create_indexes_wizard" />
<field name="code">action = env.get('cleanup.purge.wizard.data').get_wizard_action()</field>
Copy link
Member

Choose a reason for hiding this comment

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

Wrong model. Should be 'cleanup.create_indexes.wizard'

<field name="type">ir.actions.server</field>
<field name="state">code</field>
<field name="model_id" ref="database_cleanup.model_cleanup_purge_wizard_property" />
<field name="code">action = env.get('cleanup.purge.wizard.data').get_wizard_action()</field>
Copy link
Member

Choose a reason for hiding this comment

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

Wrong model. Should be 'cleanup.purge.wizard.property'

Copy link
Author

Choose a reason for hiding this comment

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

Fixed. Sorry for the confusion.

Using new base model inheritance.
Copy link
Member

@StefanRijnhart StefanRijnhart left a comment

Choose a reason for hiding this comment

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

Thanks for all the quick fixes!

Copy link
Member

@hbrunn hbrunn left a comment

Choose a reason for hiding this comment

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

small things. I also made CompassionCH#1 for an issue when purging modules

@@ -74,7 +75,8 @@ def find(self):
if not self.env.cr.rowcount:
continue

yield (0, 0, {
Copy link
Member

Choose a reason for hiding this comment

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

what was the problem with the iterator?

for line in self:
if self:
objs = self
else:
Copy link
Member

Choose a reason for hiding this comment

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

if this happens, I think something went wrong in the UI. How can I reproduce this?

Copy link
Member

Choose a reason for hiding this comment

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

PS: If you really need this for whatever reason, just assign to self and leave the rest of the code as it is. It's just a variable like any other

# The super method crashes if the model cannot be instantiated
if self.env.context.get('no_prepare_update'):
return True
return super(IrModelFields, self)._prepare_update()
Copy link
Member

Choose a reason for hiding this comment

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

can't you filter self for existing models as done above? This also would make the extra context key unnecessary

[FIX] really uninstall modules and avoid a crash on cached data
@moylop260
Copy link
Contributor

@ecino What about @hbrunn comments?

@ecino
Copy link
Author

ecino commented Aug 24, 2017

@moylop260 I missed those comments, I will look into it when I have time.

@hbrunn
Copy link
Member

hbrunn commented Sep 5, 2017

@ecino I addressed those in CompassionCH#2

@ecino
Copy link
Author

ecino commented Sep 5, 2017

thanks @hbrunn, I merged your commits.

@pedrobaeza pedrobaeza merged commit 4ff0c09 into OCA:10.0 Sep 6, 2017
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.

None yet