jrief / django-angular

Let AngularJS play well with Django
http://django-angular.awesto.com/
MIT License
1.23k stars 294 forks source link

Form defaults overwrite model data #241

Closed mathead closed 8 years ago

mathead commented 8 years ago

Hey, first of all, this is a great library, saved me a ton of time, thanks a lot!

I noticed a weird quirk that has been bugging me for quite some time - sometimes when I get a template with djangular form and simultaneously get model data for the form, defaults from the form overwrite the model. For example:

When I have the template cached this doesn't happen, so I suspect this only happens when the template gets downloaded after the actual model. Also when I comment out restoreInputField from the ngModel directive in django-angular.js, this does not happen anymore (but then the defaults stop working in blank forms).

My hacky solution is to copy the model and after a timeout set it back (dealing with the time difference of arrival between the model and the form template), but this quickly gets out of hand. I guess the proper fix would be to check if the model isn't set to anything during restoring the fields. I would submit a PR, but I'm not sure if this is the right way to fix it or if I'm not doing this entirely wrong.

Thanks!

jrief commented 8 years ago

Just a question. Have you added your own controller interacting with the form?

Conceptually Django forms and Angular forms behave very differently. In Angular there is no such thing as a bound form.

mathead commented 8 years ago

Yes, my controller:

.controller 'UserEditController', ($scope, User, $state, $timeout) ->
  $scope.user = if $state.params.id? then User.$find $state.params.id else User.$build()
  $scope.edit = $state.params.id?

  $scope.user.$then (user) ->
    name = user.name
    $timeout ->
      $scope.user.name = name
    , 1000

Template for it:

<ng-include src="'forms/user'"></ng-include>

Django template for forms/user:

<form ng-submit="$parent.save()" name="$parent.{{ form.form_name }}" novalidate>
  {% csrf_token %}
  {{form.as_div}}
  <button class="btn btn-primary" type="submit" ng-disabled="{{ form.form_name }}.$invalid">SUBMIT</button>
</form>
jrief commented 8 years ago

Sorry, this is your special use case which presumably isn't caused by any django-angular code.

mathead commented 8 years ago

I don't think so, I just replicated this with a blank project using only angular and django-angular.

My index file:

doctype html

html(ng-app="test")
  body
    div(ng-controller="CoreController")
      ng-include(src="'forms/user'")

    script(src="static/angular/angular.js")
    script(src="static/djangular/js/django-angular.js")

    script(src="static/js/app.js")

My app:

angular.module 'test', ['ng.django.forms']
.config ($httpProvider) ->
  $httpProvider.defaults.xsrfCookieName = 'csrftoken'
  $httpProvider.defaults.xsrfHeaderName = 'X-CSRFToken'

.controller 'CoreController', ($scope, $http) ->
  $http.get('api/users/1').success (user) ->
    $scope.user = user

The key to replicate this is in the ng-include directive, so that the template can be downloaded and rendered by angular after the user model data gets downloaded. restoreInputField from django-angular.js then gets fired and overwrites the model data. To get more consistent behavior I put a sleep in my django forms __init__.

jrief commented 8 years ago

how does your ng-include fetch the template from the server, or to ask in another way, who expands it?

mathead commented 8 years ago

ng-include is a directive from angular https://docs.angularjs.org/api/ng/directive/ngInclude.

If you're asking about expanding on the server, it's a standard django-angular form:

class UserForm(NgModelFormMixin, Bootstrap3FormMixin, NgFormValidationMixin, NgModelForm):
    form_name = 'user_form'
    scope_prefix = 'user'

    class Meta:
        model = User
        fields = ("username", "first_name", "last_name", "email", "is_superuser")

with template:

<ng-form name="{{ form.form_name }}" novalidate>
  {% csrf_token %}
  {{ form.as_div }}
</ng-form>
jrief commented 8 years ago

If you use your browser debug tools, how is that template expanded there?

I actually never ng-included forms rendered by the server, that's kind of a weird idea.

mathead commented 8 years ago

The form is expanded as:

<ng-form name="user_form" novalidate>
  <input type='hidden' name='csrfmiddlewaretoken' value='6NLemtqon8V08tjDvstFR4eOIfaD6h3I' />
  <div class="djng-line-spreader"><ul class="djng-form-errors" ng-show="user_form.$dirty" ng-cloak></ul><ul class="djng-form-errors" ng-show="user_form.$pristine" ng-cloak><li ng-show="user_form.$message" class="invalid" ng-bind="user_form.$message"></li></ul></div>
<div class="has-feedback form-group"><label class="control-label" for="id_username">Username</label><input class="form-control" id="id_username" maxlength="200" name="username" ng-maxlength="200" ng-model="user[&#39;username&#39;]" ng-required="true" type="text" value="Placeholder" /><ul class="djng-form-control-feedback djng-field-errors" ng-show="user_form.username.$dirty" ng-cloak><li ng-show="user_form.username.$error.required" class="invalid">Toto pole je vyžadováno.</li><li ng-show="user_form.username.$error.maxlength" class="invalid">Ensure this value has at most 200 characters</li><li ng-show="user_form.username.$valid" class="valid"></li></ul><ul class="djng-form-control-feedback djng-field-errors" ng-show="user_form.username.$pristine" ng-cloak><li ng-show="user_form.username.$valid" class="valid"></li><li ng-show="user_form.username.$message" class="invalid" ng-bind="user_form.username.$message"></li></ul></div>
<div class="has-feedback form-group"><label class="control-label" for="id_first_name">First name</label><input class="form-control" id="id_first_name" maxlength="200" name="first_name" ng-maxlength="200" ng-model="user[&#39;first_name&#39;]" ng-required="true" type="text" value="Placeholder" /><ul class="djng-form-control-feedback djng-field-errors" ng-show="user_form.first_name.$dirty" ng-cloak><li ng-show="user_form.first_name.$error.required" class="invalid">Toto pole je vyžadováno.</li><li ng-show="user_form.first_name.$error.maxlength" class="invalid">Ensure this value has at most 200 characters</li><li ng-show="user_form.first_name.$valid" class="valid"></li></ul><ul class="djng-form-control-feedback djng-field-errors" ng-show="user_form.first_name.$pristine" ng-cloak><li ng-show="user_form.first_name.$valid" class="valid"></li><li ng-show="user_form.first_name.$message" class="invalid" ng-bind="user_form.first_name.$message"></li></ul></div>
<div class="has-feedback form-group"><label class="control-label" for="id_last_name">Last name</label><input class="form-control" id="id_last_name" maxlength="200" name="last_name" ng-maxlength="200" ng-model="user[&#39;last_name&#39;]" ng-required="true" type="text" value="Placeholder" /><ul class="djng-form-control-feedback djng-field-errors" ng-show="user_form.last_name.$dirty" ng-cloak><li ng-show="user_form.last_name.$error.required" class="invalid">Toto pole je vyžadováno.</li><li ng-show="user_form.last_name.$error.maxlength" class="invalid">Ensure this value has at most 200 characters</li><li ng-show="user_form.last_name.$valid" class="valid"></li></ul><ul class="djng-form-control-feedback djng-field-errors" ng-show="user_form.last_name.$pristine" ng-cloak><li ng-show="user_form.last_name.$valid" class="valid"></li><li ng-show="user_form.last_name.$message" class="invalid" ng-bind="user_form.last_name.$message"></li></ul></div>
<div class="has-feedback form-group"><label class="control-label" for="id_email">Email</label><input class="form-control" id="id_email" maxlength="200" name="email" ng-maxlength="200" ng-model="user[&#39;email&#39;]" ng-required="true" type="text" value="Placeholder" /><ul class="djng-form-control-feedback djng-field-errors" ng-show="user_form.email.$dirty" ng-cloak><li ng-show="user_form.email.$error.required" class="invalid">Toto pole je vyžadováno.</li><li ng-show="user_form.email.$error.maxlength" class="invalid">Ensure this value has at most 200 characters</li><li ng-show="user_form.email.$valid" class="valid"></li></ul><ul class="djng-form-control-feedback djng-field-errors" ng-show="user_form.email.$pristine" ng-cloak><li ng-show="user_form.email.$valid" class="valid"></li><li ng-show="user_form.email.$message" class="invalid" ng-bind="user_form.email.$message"></li></ul></div>
<div class="has-feedback form-group"><label class="control-label" for="id_is_superuser">Is superuser</label><label class="checkbox-inline c-checkbox"><input id="id_is_superuser" name="is_superuser" ng-model="user[&#39;is_superuser&#39;]" type="checkbox" /> <span class="fa fa-check"></span></label><ul class="djng-form-control-feedback djng-field-errors" ng-show="user_form.is_superuser.$dirty" ng-cloak><li ng-show="user_form.is_superuser.$valid" class="valid"></li></ul><ul class="djng-form-control-feedback djng-field-errors" ng-show="user_form.is_superuser.$pristine" ng-cloak><li ng-show="user_form.is_superuser.$message" class="invalid" ng-bind="user_form.is_superuser.$message"></li></ul></div>
</ng-form>

Personally, I find the ng-include approach quite useful - rarely I want to have a page with just the form, so I don't set the form as a template for the controller. A good example might be a popup with a form in it, a part of a bigger page. This also allows me to use only one django template for all of the forms.

Also I think this bug can appear without the use of ng-include, but a subcontroller with template as the form. The data for the form must come from the parent controller though. I'll try to make an example.

adrienbrunet commented 8 years ago

Well, the problem is pretty clear. If you want to init your form with angular, you need to associate your user model after the form is initiated. Not the opposite.

When using a form in a uiModal, let's say I have a $scope.user loaded before, then I open up my modal with a form in it with form_name='user_form', form_prefix='dj_user' and pass to that modal controller my previous $scope.user. Only then, in the modal controller, I set dj_user = user. All the bindings work as expected. I'm not using ng-include though. (Note that I don't use any timeout)

Maybe the fix you offer (checking if the model is set before trying to restore fields) would work but I'm not really confortable with that workflow (even if I'm eager to see your PR if you think it's worth it). I don't really like the idea of mixing your data in your parent controller and the data in the form. I preafer keep it separated and only after a save, apply changes to the parent controller.

What do you think?

mathead commented 8 years ago

Yes, your solution is cleaner, I should've done it like that. Still, this behavior seems like a bug to me. Why should loading a template modify existing data? Even if using data from parent controller is not ideal as you point out, this can be unexpected by some users (including me).

I'll try to make that PR, it shouldn't be that hard.

adrienbrunet commented 8 years ago

:+1: @jrief We agree the approach from @PrehistoricTeam is far from ideal but his PR seems ok. Can you have a look and give your feedback?

Cheers

jrief commented 8 years ago

ups sorry, then I misunderstood the question -> Reopen issue. Need some more time to review.