Merge lp:~nbessi-c2c-deactivatedaccount/account-invoice-report/7.0-better-x-path into lp:~account-core-editors/account-invoice-report/7.0

Proposed by Nicolas Bessi - Camptocamp
Status: Superseded
Proposed branch: lp:~nbessi-c2c-deactivatedaccount/account-invoice-report/7.0-better-x-path
Merge into: lp:~account-core-editors/account-invoice-report/7.0
Diff against target: 12 lines (+1/-1)
1 file modified
invoice_webkit/view/invoice_view.xml (+1/-1)
To merge this branch: bzr merge lp:~nbessi-c2c-deactivatedaccount/account-invoice-report/7.0-better-x-path
Reviewer Review Type Date Requested Status
Niels Huylebroeck (community) Needs Fixing
Guewen Baconnier @ Camptocamp Approve
Alexandre Fayolle - camptocamp code review, no test Approve
Review via email: mp+143448@code.launchpad.net

This proposal has been superseded by a proposal from 2013-01-18.

To post a comment you must log in.
Revision history for this message
Alexandre Fayolle - camptocamp (alexandre-fayolle-c2c) wrote :

LGTM

review: Approve (code review, no test)
Revision history for this message
Guewen Baconnier @ Camptocamp (gbaconnier-c2c) wrote :

LGTM

review: Approve
Revision history for this message
Nicolas Bessi - Camptocamp (nbessi-c2c-deactivatedaccount) wrote :

Can somebody merge this one please.

Regards

Nicolas

Revision history for this message
Stefan Rijnhart (Opener) (stefan-opener) wrote :

Will do so

Revision history for this message
Niels Huylebroeck (red15) wrote :

Hold it, the intent is good, the xpath won't work afaik.

You have the following arch:

<field name="invoice_line">
   <tree string="Invoice Lines">
      ...
   </tree>
</field>

Thus your xpath of "//tree[@name='invoice_line']" cannot match, the tree itself does not have a name, it's the field above the tree that has the name !

This is why I suggested "//field[name='invoice_line']/tree" instead.

If the current xpath actually worked for you (tested) I will withdraw my remark but would be interested in knowing where my xpath understanding went wrong ?

review: Needs Fixing
Revision history for this message
Nicolas Bessi - Camptocamp (nbessi-c2c-deactivatedaccount) wrote :

Hello,

Well your right.

I was tricked because updating the addon made a silent exception. But if you install it from scratch it wont install.

I will provide a fix soon.

Regards

Nicolas

Revision history for this message
Nicolas Bessi - Camptocamp (nbessi-c2c-deactivatedaccount) wrote :

I revert the commit now as it induce a bug.

Nicolas

Revision history for this message
Nicolas Bessi - Camptocamp (nbessi-c2c-deactivatedaccount) wrote :

FYI if you update the server to tag 7.0 update of addon will now also throw an exception.

Regards

Nicolas

10. By Nicolas Bessi - Camptocamp

[FIX] better xpath

Unmerged revisions

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'invoice_webkit/view/invoice_view.xml'
2--- invoice_webkit/view/invoice_view.xml 2013-01-14 13:42:45 +0000
3+++ invoice_webkit/view/invoice_view.xml 2013-01-16 08:26:21 +0000
4@@ -69,7 +69,7 @@
5 <field name="model">account.invoice</field>
6 <field name="inherit_id" ref="account.invoice_form"/>
7 <field name="arch" type="xml">
8- <xpath expr="//tree[@string='Invoice Lines']" position="attributes" >
9+ <xpath expr="//tree[@name='invoice_line']" position="attributes" >
10 <attribute name="editable" eval="False"/>
11 </xpath>
12 </field>

Subscribers

People subscribed via source and target branches