Merge lp:~therp-nl/therp-backports/web-6.1-lp1013636-one2many_fields_honour_requiredness into lp:therp-backports/web-6.1

Proposed by Stefan Rijnhart (Opener)
Status: Merged
Merged at revision: 2504
Proposed branch: lp:~therp-nl/therp-backports/web-6.1-lp1013636-one2many_fields_honour_requiredness
Merge into: lp:therp-backports/web-6.1
Diff against target: 13 lines (+3/-0)
1 file modified
addons/web/static/src/js/view_form.js (+3/-0)
To merge this branch: bzr merge lp:~therp-nl/therp-backports/web-6.1-lp1013636-one2many_fields_honour_requiredness
Reviewer Review Type Date Requested Status
Holger Brunn (Therp) Approve
Review via email: mp+156301@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Holger Brunn (Therp) (hbrunn) wrote :

For the other widgets, I see them touching this.invalid mostly in the function validate(), wheras is_valid returns false when the widget is invalid.

I think you either should move your code to validate() or just return false in your case.

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

Setting this.invalid in validate() works for other widgets, because the super class checks for the value of this member in the is_valid() method. However, the FieldOne2Many class overrides this method and skips the check. I could still refactor of course, by putting the stanza in an override of validate() and adding this check to is_valid() but it does not seem useful to me.

2504. By Stefan Rijnhart (Opener)

[RFR] Do not imply that setting this.invalid implements the fix

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

OK, misunderstood. I now refactored not to set this.invalid but return false instead.

Revision history for this message
Holger Brunn (Therp) (hbrunn) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'addons/web/static/src/js/view_form.js'
2--- addons/web/static/src/js/view_form.js 2013-03-13 09:26:29 +0000
3+++ addons/web/static/src/js/view_form.js 2013-04-09 13:27:21 +0000
4@@ -2697,6 +2697,9 @@
5 }, this));
6 },
7 is_valid: function() {
8+ if (this.required && _(this.dataset.ids).isEmpty()) {
9+ return false;
10+ }
11 if (!this.viewmanager.views[this.viewmanager.active_view])
12 return true;
13 var view = this.viewmanager.views[this.viewmanager.active_view].controller;

Subscribers

People subscribed via source and target branches

to all changes: