Odoo Web (MOVED TO GITHUB)

Merge lp:~serpentcs/openerp-web/fix-1191037-web into lp:openerp-web/7.0

  1. fix-1191037-web
  2. Merge into 7.0
Proposed by Serpent Consulting Services
Status: Needs review
Proposed branch: lp:~serpentcs/openerp-web/fix-1191037-web
Merge into: lp:openerp-web/7.0
Diff against target: 11 lines (+1/-0)
1 file modified
addons/web/static/src/js/view_list.js (+1/-0)
To merge this branch: bzr merge lp:~serpentcs/openerp-web/fix-1191037-web
Reviewer Review Type Date Requested Status
Serpent Consulting Services (community) Needs Resubmitting
Xavier (Open ERP) (community) Disapprove
Review via email: mp+192141@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Xavier (Open ERP) (xmo-deactivatedaccount) wrote :

- tabs
- screwed indentation
- leftover condition on `value` in moved if is useless
- caller expects a string, not `undefined`. The error is not seen here because the caller simply passes the value to $.fn.html and does not do anything with the result of $.fn.html. Passing undefined makes $.fn.html return the html content instead of setting it, if more operations are chained on the node in the future, the rendering will blow up on empty binary fields.
- 45 lines of diff where a single one would have sufficed, why not just

    if (!value) { return ''; }

  added right before the existing conditionals?

review: Disapprove
Reply
Revision history for this message
Serpent Consulting Services (serpent-consulting-services) wrote :

Xavier,

Will surely improve but the indentations and tabs are as it were.

We had just to shift right!

Anyways, will improve and resubmit.

Thanks.

Reply
Revision history for this message
Xavier (Open ERP) (xmo-deactivatedaccount) wrote :

> Will surely improve but the indentations and tabs are as it were.

Nope, the original code had no tabs, you modified the indentation by adding tabs, look at the launchpad-generated diff it's completely screwed (all indentation in the web client, outside of third-party libraries and maybe sass files, should be spaces only and 4 spaces).

Reply
lp:~serpentcs/openerp-web/fix-1191037-web updated
4043. By Yogesh (SerpentCS)

Revert last commit.

4044. By Yogesh (SerpentCS)

Merge with lp:~serpentcs/openerp-web/fix-1191037-web revision no 4118.

4045. By Yogesh (SerpentCS)

[FIX] web : Binary field when binay file not attached in records then list view should not show download link.

Revision history for this message
Serpent Consulting Services (serpent-consulting-services) :
review: Needs Resubmitting
Reply

Unmerged revisions

4045. By Yogesh (SerpentCS)

[FIX] web : Binary field when binay file not attached in records then list view should not show download link.

4044. By Yogesh (SerpentCS)

Merge with lp:~serpentcs/openerp-web/fix-1191037-web revision no 4118.

4043. By Yogesh (SerpentCS)

Revert last commit.

4042. By Serpent Consulting Services

[FIX] web : Binary field when on list view should not show download link

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'addons/web/static/src/js/view_list.js'
--- addons/web/static/src/js/view_list.js 2014-01-15 11:00:38 +0000
+++ addons/web/static/src/js/view_list.js 2014-01-18 07:34:42 +0000
@@ -2240,6 +2240,7 @@
2240 _format: function (row_data, options) {2240 _format: function (row_data, options) {
2241 var text = _t("Download");2241 var text = _t("Download");
2242 var value = row_data[this.id].value;2242 var value = row_data[this.id].value;
2243 if (!value) { return '';}
2243 var download_url;2244 var download_url;
2244 if (value && value.substr(0, 10).indexOf(' ') == -1) {2245 if (value && value.substr(0, 10).indexOf(' ') == -1) {
2245 download_url = "data:application/octet-stream;base64," + value;2246 download_url = "data:application/octet-stream;base64," + value;