Removal of Property

62 views
Skip to first unread message

Cédric Krier

unread,
Mar 28, 2017, 6:15:07 PM3/28/17
to Tryton
Hi,

The patches that removes the Property fields is ready for testing. It
is passing all the tests.

https://bugs.tryton.org/issue2349

I would like to commit it ASAP to get enough time to find all bugs
before the release.
Indeed I do not expect a full review of the patches (it is quite big but
almost always based the same pattern) but more a check of the design.
The MultiValue field is designed since a long time already. The main new
thing is the choice of how to define the Value model. For example is the
grouping of value right (like the sale.configuration.sequence)? Is the
choice of standard Value or the company one right?
It will be good also to have some migration test.
It is also important to read my last messages on the bug-tracker that
describes some design choice made when facing issues.

Finally, do not forget that it is not yet the removal of company rule.
It will be done in a second step with https://bugs.tryton.org/issue4080

Thanks,
--
Cédric Krier - B2CK SPRL
Email/Jabber: cedric...@b2ck.com
Tel: +32 472 54 46 59
Website: http://www.b2ck.com/

Sergi Almacellas Abellana

unread,
Mar 29, 2017, 10:13:54 AM3/29/17
to tryto...@googlegroups.com
El 29/03/17 a les 00:12, Cédric Krier ha escrit:
> Hi,
>
> The patches that removes the Property fields is ready for testing. It
> is passing all the tests.
>
> https://bugs.tryton.org/issue2349

First of all: Thanks for the big work :)
>
> I would like to commit it ASAP to get enough time to find all bugs
> before the release.

Indeed bugs can be found before commiting. It's a mather of appling all
of the reviews. I have some script in order to update them when applied
that I attach in this mail if is usefull for somebody else.

I've found the following:

- Creating a party with no stock locations clears the customer_location
and supplier_location values of all the parties. Indeed, setting the the
location value of a party to any values sets this value for all of the
parties. I've reproduced it on a migrated database but I haven't tested
on a new database.

I've tested on this specify model, but I'm wondering if the other models
are also affected.

> It will be good also to have some migration test.

After the migration I get a lot of this warnings like this:

WARNING trytond.convert Could not delete id 20 of model ir.property
because model no longer exists.

Is this the expected behaviour?

This warning are also shown when updating the database for second time.
The only way to remove this warnings was executing this query on the
database:

delete from ir_model_data where model = 'ir.property';

Should we add this to the migration page?




--
Sergi Almacellas Abellana
www.koolpi.com
Twitter: @pokoli_srk
update_modules_review.sh

Cédric Krier

unread,
Mar 29, 2017, 11:20:07 AM3/29/17
to tryto...@googlegroups.com
On 2017-03-29 16:13, Sergi Almacellas Abellana wrote:
> El 29/03/17 a les 00:12, Cédric Krier ha escrit:
> > I would like to commit it ASAP to get enough time to find all bugs
> > before the release.
>
> Indeed bugs can be found before commiting. It's a mather of appling all of
> the reviews. I have some script in order to update them when applied that I
> attach in this mail if is usefull for somebody else.
>
> I've found the following:
>
> - Creating a party with no stock locations clears the customer_location and
> supplier_location values of all the parties. Indeed, setting the the
> location value of a party to any values sets this value for all of the
> parties. I've reproduced it on a migrated database but I haven't tested on a
> new database.
>
> I've tested on this specify model, but I'm wondering if the other models are
> also affected.

It should be fixed in last patch set. Indeed this MultiValue model was
missing the One2Many so it was using all records.

> > It will be good also to have some migration test.
>
> After the migration I get a lot of this warnings like this:
>
> WARNING trytond.convert Could not delete id 20 of model ir.property because
> model no longer exists.
>
> Is this the expected behaviour?

Yes, the system does not know any more about ir.property

> This warning are also shown when updating the database for second time.
> The only way to remove this warnings was executing this query on the
> database:
>
> delete from ir_model_data where model = 'ir.property';
>
> Should we add this to the migration page?

Yes it could even if leaving them does not hurt at all but it is only
after the migration was succeeded.

Sergi Almacellas Abellana

unread,
Mar 31, 2017, 4:10:14 AM3/31/17
to tryto...@googlegroups.com
El 29/03/17 a les 17:16, Cédric Krier ha escrit:
>> This warning are also shown when updating the database for second time.
>> The only way to remove this warnings was executing this query on the
>> database:
>>
>> delete from ir_model_data where model = 'ir.property';
>>
>> Should we add this to the migration page?
> Yes it could even if leaving them does not hurt at all but it is only
> after the migration was succeeded.
I added this part to the migration page.

Regards,
Reply all
Reply to author
Forward
0 new messages