mgr/dashboard: Remove _filterValue from CdFormGroup #24719
Conversation
| @@ -113,6 +113,7 @@ export class RgwUserFormComponent implements OnInit { | |||
| ] | |||
| ] | |||
| }); | |||
| this.userForm._filterValue = (value) => value; | |||
There was a problem hiding this comment.
Please add a comment here why it is necessary to override this function.
There was a problem hiding this comment.
IMHO, a comment on that function doesn't make the implementation better. What we really need, and that's what you've already said @votdev, is a refactoring.
|
Can you please add a comment that explains the need of the I think the current implementation of getValue will fail if the processed control is a checkbox that return 'false' in unchecked state. If you call the method then you can not differentiate anymore between unchecked state and "if it has none it will return false". In my opinion this method is missdesigned and should be refactored. |
Devp00l
force-pushed
the
wip-issue-26861
branch
from
95dd6b7
to
e054cae
Compare
Devp00l
changed the title
|
jenkins test dashboard |
Devp00l
force-pushed
the
wip-issue-26861
branch
from
e054cae
to
836c0fd
Compare
Devp00l
changed the title
|
Addressed all comments |
|
lgtm, but it feels a little bit odd that a (by now) refactoring PR contains a test addition for an issue. It might make sense to have two commits here. |
| spyOn(rgwUserService, 'update'); | ||
| component.editing = true; | ||
|
|
||
| const user_id = component.userForm.get('email'); |
It turned out that "_filterValue" wasn't needed for any form. Fixes: http://tracker.ceph.com/issues/26861 Signed-off-by: Stephan Müller <smueller@suse.com>
Devp00l
force-pushed
the
wip-issue-26861
branch
from
836c0fd
to
d4963c3
Compare
|
Addressed all comments |
The current behavior when submitting an empty mail field in the RGW user form that was previously filled was to replace it with "false". This was a regression introduced through the use of "CdFormGroup" which had a "_filterValue" method which converted empty strings into false, but it was removed now. Fixes: http://tracker.ceph.com/issues/26861 Signed-off-by: Stephan Müller <smueller@suse.com>
Devp00l
force-pushed
the
wip-issue-26861
branch
from
d4963c3
to
a159f5a
Compare
|
jenkins retest this please |
The current behavior when submitting an empty mail field in the RGW user form
that was previously filled was to replace it with "false".
This was a regression introduced through the use of "CdFormGroup" which had
a "_filterValue" method which converted empty strings into false, but it was
removed now.
Fixes: http://tracker.ceph.com/issues/26861
Signed-off-by: Stephan Müller smueller@suse.com