openerp-dev-web team mailing list archive

["Xavier \(Open ERP\)" <xmo@xxxxxxxxxxx>]

Review: Needs Fixing
* The validation message is terrible, what is "the record"? How is anybody going to understand that? It was OK for form, but for list is needs to be much clearer.
* old listgridValidation(name, o2m) have become far too complex and redundant, please extract redundancies into a function
* In validateForm, `_list_view` can not be `0` or an empty string (or id selector will fail anyway), so you don't need `_list_view != null`, `_list_view` should be enough to test that there is a list view id.
* You probably should not use `is_list_changed` (denoting a boolean value) to store the id of the modified list. Either rename key (to e.g. `modified_list`) or use two different data keys (a boolean to know if a list changed, and then a string to know *which* list changed)
-- listgrid.js
* Why async=false in listgrid save?
* Do not use alert() for errors
* If you have an id (as string) and want to turn it into a jquery id selector, there is now an `idSelector` function in openobject.dom.js, this should be used in ListView.save and in listgridValidation
* Also the `return` after $focus_field is redundant
* In listgridValidation, `0` is not a valid second parameter for parseInt, use `10`.
* jQuery.ajax can take a context keyword argument, this means the `var self=this` aliasing is now unnecessary (just pass `context: this` and in the success callback replace `self` by `this`)

Also more general comments:
* create more, smaller and more focused commits. It's easier to review and cherrypick if each commit does a single job instead of having a single big commit doing 3000 things at once. Intent of change also becomes clearer due to commit messages. You're not limited in number of commits and you don't have to pay for them so...
* Please remember to add comment if you commit/push anything new as launchpad doesn't warn reviewers
-- 
https://code.launchpad.net/~openerp-dev/openobject-client-web/inline-list-prompt-action/+merge/39436
Your team OpenERP R&D Team is subscribed to branch lp:~openerp-dev/openobject-client-web/inline-list-prompt-action.