On 24/12/12 01:15 +0100, Albert Cervera i Areny wrote: > A Diumenge, 4 de març de 2012 03:49:24, [email protected] va escriure: > > Reviewers: , > > > > > > > > Please review this at http://codereview.tryton.org/266002/ > > > > Affected files: > > M trytond/model/modelsql.py > > M trytond/model/modelstorage.py > > M trytond/tests/test_tryton.py > > I have a couple of questions that I'd like to ask before adding the changes > to > this patch: > > - In the sale module, the code for creating an invoice uses the ActiveRecord > pattern and ends up with a list of SaleLine instances. Current code uses a > loop over those objects and calls .save() on each of them. This is can be > improved with the new create() but I thought that the following would work > and > it didn't: > > invoice = self._get_invoice_sale(invoice_type) > + invoice.lines = invoice_lines > invoice.save() > > I thought this would be similar to the non-ActiveRecord pattern and would be > equivalent to [('create', invoice_line_values)] but it is not.
When reading the code, for me it is equivalent. How do you conclude
this?
But of course in your patch the code should be improved to group the
create.
> So what I did
> is the following (using ._save_values property):
>
> + to_create = []
> for line in self.lines:
> if line.id not in invoice_lines:
> continue
> for invoice_line in invoice_lines[line.id]:
> invoice_line.invoice = invoice.id
> - invoice_line.save()
> - SaleLine.write([line], {
> - 'invoice_lines': [('add', [invoice_line.id])],
> - })
> + to_create.append(invoice_line._save_values)
> + if to_create:
> + InvoiceLine.create(to_create)
>
>
> Is that Ok?
Not for me, _save_values should not be used as it is not part of the API.
--
Cédric Krier
B2CK SPRL
Rue de Rotterdam, 4
4000 Liège
Belgium
Tel: +32 472 54 46 59
Email/Jabber: [email protected]
Website: http://www.b2ck.com/
pgpN0uWpYwAnK.pgp
Description: PGP signature
