ngMessages support

[jamesbrobb]

Hi Jacob, i've just updated my forms to use ngMessages and was wondering if it's something you think would be useful for the project?

It adds a better flow for dealing with server errors, using a 'rejected' validator to display a returned field error. Plus there's also a significant reduction in watchers. I have 3 forms with a total of 9 fields and have seen a reduction of 40.

One thing to note is that ngMessages are dependent on angular 1.3+

The new logic consists of:

A new TupleErrorList - NgMessagesTupleErrorList
A django filter to add the validator to the input - although there might be a better solution for this?
An angular service to process and apply the errors
An angular 'rejected' validator directive

Anyway, let me know what you think?

[jrief]

Yeah! This is a perfect fit. The docs even say that you should use it for displaying form validation errors.
When I added support to form validation errors, AngularJS was <=1.2 and ngMessages not available yet.

One problem however remains, look at this example: http://djangular.aws.awesto.com/classic_form/
Here no validation model is available. Here we must work differently.

I really like that idea.

[jamesbrobb]

I've added an ngMessages page to the example. At this point it's only using both model and form validation like the 'Combined Validation' page. There are also a couple of things to note:

1. I've added lodash to the example base.html. This is just temporary so that you can see how many watchers there are on each page. I was quite surprised to see that there's a 50% drop from 142 on 'Combined Validation' to 70 on 'angular-messages'. Just run this code in your console on each page.

EDIT with the reintroduction of the valid message, there's a drop of 41.

function getScopes(root) {
    var scopes = [];
    function traverse(scope) {
        scopes.push(scope);
        if (scope.$$nextSibling)
            traverse(scope.$$nextSibling);
        if (scope.$$childHead)
            traverse(scope.$$childHead);
    }
    traverse(root);
    return scopes;
}
var rootScope = angular.element(document.querySelectorAll("[ng-app]")).scope();
var scopes = getScopes(rootScope);
var watcherLists = scopes.map(function(s) { return s.$$watchers; });
_.uniq(_.flatten(watcherLists)).length;

2. I've added a form mixin NgMessagesMixin to add the rejected validator attribute to each field. I'm not sure whether this is the best way to deal with this? Any suggestions would be welcome.

3. I also noticed a minor bug in the djangoForm service, that causes an error in 1.3+. There's a new property on form called 'pending' that relates to the new $asyncValidators . This is initially set to ùndefined`, so can cause this line to throw an error, as subfield is undefined. I've added a fix for that as well.

angular.isArray(subField.$viewChangeListeners)

4. I'm using the existing 'patched_fields' module to generate the error messages, then stripping off '$error' when the value is assigned to the ng-messageattribute in NgMessagesTupleErrorList. I'm not 100% happy with this approach, but it wouldn't be very DRY to create a new module that pretty much replicates the existing one.

EDIT Just realised the project i'm working on is using 0.7.3, i'm going to update tomorrow and take a look at the new field_mixins module.

5. There's currently no 'valid' message being generated (and therefore displayed), as this will now need to be a separate html element. I'm not sure what the best way to deal with this will be, as i don't use them in my current project. Any thoughts?

[jamesbrobb]

I'd like to propose a change to the way that field.$message works when using ngMessages. This would allow multiple validators on the same field to bind to a specific message.

I don't think this is a breaking change, as it'll only affect people who opt to use ngMessages. As this is field specific, an an individual field would only ever be handled by 1) the existing method (assign error string directly to $message) or 2) the proposed method for compatibility with the new 1.3 validators (assign error string to a related property on $message). There will be no crossover even if a dev decides to validate different forms with different methods.

e.g

For 'rejected' errors you'd bind to field.$message.rejected

This validator specific property can then be used by the validator to check if it has as error.

[jrief]

Hi James,
sorry again for not merging this.
For me its quite hard to understand that code, but just reading this PR.
Something which could help, would be some kind of docs in .rst format.
Currently your PR does has merging conflicts.

[jrief]

why did you close this?

[jamesbrobb]

Because unfortunately I don't have time to go through and merge the conflicts. They didn't exist 3 months ago when the PR was made and i'm now too busy to look at this. Turned out to be one very simple conflict...

[jamesbrobb]

Hi Jacob, i'm not sure i understand what docs you need from me? Would you like me to write docs which explain how to use the ngMessages functionality or docs that explain the code for you?

[jrief]

Docs which explain it to the end user. These are also very helpful for me.

BTW, I've never used ngMessages, so I have no clue how they work. Is ist correct, that ngMesages is available only for Angular>1.3?

But another consideration. Django has a messaging framework. Wouldn't it make sense to combine them? Django messages are rendered statically, and remain on the page, until it is reloaded.

[jamesbrobb]

Hi Jacob, i've added docs to explain the usage, let me know what you think?

Yes, they only work with 1.3+, so this would need to be a separate (opt in) module that's not minified with the rest.

The name follows the trend of angular naming conventions, in that it's slightly misleading. I can see the logic, as you could essentially use them to monitor key/value pairs on any object, but as their primary use seems to be responding to the ngModel $error object, maybe ngErrorMessages would have been a better name. This is the description from the docs:

ngMessages is a directive that is designed to show and hide messages based on the state of a key/value object that it listens on. The directive itself compliments error message reporting with the ngModel $error object (which stores a key/value state of validation errors).

RE: django messages, i'm actually working on an angular ajax solution for this at the moment.

[jrief]

@jamesbrobb
This seems to be a major new independent feature, which does not influence the remaining code.
Thank you very much this contribution!

As I currently have no use for it (and btw. busy on something else), would you like to maintain this additional functionality now and in the future? I then would give you write access to the repository. This also means, that you bear some responsibility for the estim. >1000 users of this project.

@dasf
What's your opinion on this?

[jkosir]

I don't really know ngMessages, but it certainly looks good. I think it would make a nice addition.

[jamesbrobb]

@jrief Sorry for the delay in my response, thanks, i'd be very happy to maintain this feature.

I've finished adding docs and unit tests for both the client and backend. I have one small issue to resolve and then it should be good to go.

[jamesbrobb]

@jrief I'm just working on getting this to work when submitting the classic form and model form via POST, but have noticed an issue with the classic form.

There's an input directive in the ng.django.forms module that handles adding an ngModel to any input that doesn't have one (when specific criteria are met). This occurs on the classic form, but if you submit an empty form, although there are error messages returned for the select and textarea, they never actually get displayed, as neither have an ngModel associated with them.

Does there also need to be the same directive for a select and textarea, so that an ngModel is added to them as well? If so, i'm happy to create a new branch to implement this. Unless there's a specific reason i'm missing as to why this does not already exist?

EDIT

I've added the suggested fix to my working copy and it fixes the issue i've described without breaking any tests

[jamesbrobb]

So this is pretty much done, although it may require a few tweaks depending on your thoughts regarding #160 and my 2nd point below.

A couple of things to note:

1. The NgMessagesMixin is for use in combination with the NgFormValidationMixin. I have it working correctly for both POST and AJAX form submissions.

I spent a bit of time trying to get it to work with both the 'Classic Subscription' and 'Model Scope' forms (with #160 being the result of that process). It's doable, but will involve a bit of extra work, but i'm not sure it's worth the effort. I started on it but then came to the conclusion that it probably wasn't necessary for those forms, as its purpose is to manage the display of 'validation' messages. Which don't occur on either of them. I'd be interested to hear what you think?

2. It also occurred to me whilst finishing this off, that it actually adds support for 2 separate features.

• the Python adds support for the ngMessages module, as it renders error lists in that format.

• but the JS, although needed when using the ngMessages module, doesn't have a dependency on it. What it actually does is add support for the 1.3+ $validators pipeline. So could just as easily be used without ngMessages.

  Essentially all it does is replace the way that the ng.django.forms module handles 'rejected' server errors, with a version that uses the 1.3+ $validators pipeline instead. So it should definitely still be a separate 'opt-in' module, but i'm thinking it should be named appropriately to reflect it's purpose, instead of being called something that relates it to ngMessages.

  Perhaps ng.django.forms.1.3 and then any other 'opt-in' features that are 1.3 specific could be added to it? Or maybe it would be better to keep it completely separate and name it ng.django.forms.validators?

[papasax]

Hi,
I need to display a list of non_field_errors on my forms, and the current version of the library does not allow this yet. It seems to display only the first one.
Does your PR address this problem?

Regards,

[jrief]

If we add ngMessages support to django-angular, the minimum required version of Angular will be 1.3. Everybody whom I asked told me that they had no problem so far with 1.3.
@papasax do you ask @jamesbrobb or me about the form field errors. I personally had no need for ngMessages yet, but I still see it as useful feature, so from my personal opinion it should be merged.
@adrienbrunet whats your opinion about this?

[adrienbrunet]

I really like this feature even if I'm not using (yet) ngMessage. @jamesbrobb has done an amazing job! We should merge it very soon. I thought we were just waiting for the PR#160 to merge this one.

[papasax]

I really did not have time to dig into this PR although this looks very
interesting, but since I have this problem, if someone tells me that this
PR can potentially fix my problem, i will. Right now, on my forms, i have
to concat multiple messages to be rendered at the form level and it does
not look nice at all. The problem is that with bigger pages, it's often
needed to have the error message displayed next to the field and next to
the "OK" button. Django is not aimed to work that way but I guess this is a
new need.

2015-07-10 19:29 GMT+02:00 Adrien Brunet [email protected]:

I really like this feature even if I'm not using (yet) ngMessage.
@jamesbrobb https://github.com/jamesbrobb has done an amazing job! We
should merge it very soon. I thought we were just waiting for the PR#160 to
merge this one.

—
Reply to this email directly or view it on GitHub
#132 (comment).

[papasax]

As a workaround, I can double all {% field.error %} and put it in both
places but it's not fun ;)

2015-07-11 14:04 GMT+02:00 Alexandre Desenfant <
[email protected]>:

I really did not have time to dig into this PR although this looks very
interesting, but since I have this problem, if someone tells me that this
PR can potentially fix my problem, i will. Right now, on my forms, i have
to concat multiple messages to be rendered at the form level and it does
not look nice at all. The problem is that with bigger pages, it's often
needed to have the error message displayed next to the field and next to
the "OK" button. Django is not aimed to work that way but I guess this is a
new need.

2015-07-10 19:29 GMT+02:00 Adrien Brunet [email protected]:

I really like this feature even if I'm not using (yet) ngMessage.
@jamesbrobb https://github.com/jamesbrobb has done an amazing job! We
should merge it very soon. I thought we were just waiting for the PR#160 to
merge this one.

—
Reply to this email directly or view it on GitHub
#132 (comment)
.

[jrief]

@jamesbrobb finally merged! Thanks and sorry for the long delay.

I ran into an issue with your unit tests. Please check the file test_ng_messages.py a look for some comments I've added to the code. I few attributes resolve as .$submitted instead of messages_form.$submitted. Do you have any idea why, or did I introduce a stupid regression?

[adrienbrunet]

👍 Well done @jrief!