Fix embedded list validation #1286
Conversation
|
Your changes are failing the maEmbeddedListFieldSpec.js test. Can you please have a look? |
Rebase #1286 to fix the Integration tests
|
IT tests are failing but this is not a problem with your code. |
jpetitcolas
left a comment
jpetitcolas
left a comment
There was a problem hiding this comment.
Not sure to understand the fix? Can you elaborate? :)
Moreover, can you add a test to prove this fix?
| } | ||
| scope.fields = targetFields; | ||
| scope.formName = []; | ||
| // scope.formNamex = [] |
There was a problem hiding this comment.
Commented out code, please remove.
| scope.entries = scope.entries.filter(e => e !== entry); | ||
| }; | ||
| scope.init = ($index) => { | ||
| scope.formName[$index] = scope.$parent.form?scope.$parent.form['subform_' + $index] : ""; |
There was a problem hiding this comment.
Coding style: please add spaces before and after ?. :)
| </div> | ||
| </div> | ||
| </div></div>` | ||
| </div> |
There was a problem hiding this comment.
An indentation is missing here.
| } | ||
|
|
||
| maEmbeddedListField.$inject = []; | ||
| maEmbeddedListField.$inject = []; |
There was a problem hiding this comment.
Missing file end of line.
|
@Phocea: no idea for tests. I just relaunched Travis. Wait and see. :) |
|
@jpetitcolas same problem as last year. If merging from a fork, Selenium test are started on a fixed IP (http://172.17.0.12:54413/wd/hub), if merging from a master branch then they are started using dns (http://ondemand.saucelabs.com:80/wd/hub).. @SzHeJason I agree with @jpetitcolas, the unit test has been fixed but would be great to add a specific test case verifying that the validation is working. And also add an example in the blog config |
|
still fail,I will try to make unit test |
|
@SzHeJason if only the IT tests are failing, dont worry about it for now. What we would like is a better understanding of your fix. |
|
I optimized and resubmitted my code that it is very close to master branch |
|
Ok, the PR #1288 proved that you just need to rebase to pass the current tests. Can you rebase from master and add a test for your fix as requested by @jpetitcolas? |
|
@jpetitcolas are the changes done ok for you ? |
|
@SzHeJason Thanks for the rebase. Can you apply the @jpetitcolas request changes ? |
|
I'm not sure to understand this fix. Can you elaborate and add a test proving the bug resolution? From what I see, there is no test updates. :) And, yes, don't worry about failing E2E tests. |
|
So is embedded_list validation fixed after all? I have the latest version of ng-admin and it still doesn't work. |
|
Sorry @hardmaster92, we're focusing our attention to marmelab/react-admin as of now and the activity on ng-admin is limited to absolute necessary. |
Sorry to hear that. Then I hope some git user with free time to fix this issue. |
5 participants
fix #1083