fancy tag editor now has input validation

10 files changed, 184 insertions, 26 deletions

diff --git a/askbot/const/__init__.py b/askbot/const/__init__.py
index f3091800..f7255455 100644
--- a/askbot/const/__init__.py
+++ b/askbot/const/__init__.py
@@ -99,11 +99,13 @@ UNANSWERED_QUESTION_MEANING_CHOICES = (
 #however it will be hard to expect that people will type
 #correct regexes - plus this must be an anchored regex
 #to do full string match
+#IMPRTANT: tag related regexes must be portable between js and python
 TAG_CHARS = r'\w+.#-'
 TAG_REGEX_BARE = r'[%s]+' % TAG_CHARS
 TAG_REGEX = r'^%s$' % TAG_REGEX_BARE
 TAG_SPLIT_REGEX = r'[ ,]+'
 TAG_SEP = ',' # has to be valid TAG_SPLIT_REGEX char and MUST NOT be in const.TAG_CHARS
+#!!! see const.message_keys.TAG_WRONG_CHARS_MESSAGE
 EMAIL_REGEX = re.compile(r'\b[A-Z0-9._%+-]+@[A-Z0-9.-]+\.[A-Z]{2,4}\b', re.I)
 
 TYPE_ACTIVITY_ASK_QUESTION = 1
diff --git a/askbot/const/message_keys.py b/askbot/const/message_keys.py
index 0ee99e0c..caee6b07 100644
--- a/askbot/const/message_keys.py
+++ b/askbot/const/message_keys.py
@@ -33,6 +33,10 @@ _('click to see the most answered questions')
 _('click to see least voted questions')
 _('by votes')
 _('click to see most voted questions')
+TAGS_ARE_REQUIRED_MESSAGE = _('tags are required')
+TAG_WRONG_CHARS_MESSAGE = _(
+    'please use letters, numbers and characters "-+.#"'
+)
 
 def get_i18n_message(key):
     messages = {
diff --git a/askbot/forms.py b/askbot/forms.py
index 3fd3d705..81354cad 100644
--- a/askbot/forms.py
+++ b/askbot/forms.py
@@ -1,6 +1,7 @@
 import re
 from django import forms
 from askbot import const
+from askbot.const import message_keys
 from django.utils.translation import ugettext_lazy as _
 from django.utils.translation import ungettext_lazy, string_concat
 from django.utils.text import get_text_list
@@ -256,7 +257,8 @@ class TagNamesField(forms.CharField):
     def need_mandatory_tags(self):
         """true, if list of mandatory tags is not empty"""
         from askbot import models
-        return askbot_settings.TAGS_ARE_REQUIRED and len(models.tag.get_mandatory_tags()) > 0
+        num_mandatory_tags = len(models.tag.get_mandatory_tags())
+        return askbot_settings.TAGS_ARE_REQUIRED and num_mandatory_tags > 0
 
     def tag_string_matches(self, tag_string, mandatory_tag):
         """true if tag string matches the mandatory tag"""
@@ -282,7 +284,9 @@ class TagNamesField(forms.CharField):
         data = value.strip()
         if len(data) < 1:
             if askbot_settings.TAGS_ARE_REQUIRED:
-                    raise forms.ValidationError(_('tags are required'))
+                    raise forms.ValidationError(
+                        _(message_keys.TAGS_ARE_REQUIRED_MESSAGE)
+                    )
             else:
                 return ''#don't test for required characters when tags is ''
         split_re = re.compile(const.TAG_SPLIT_REGEX)
@@ -309,17 +313,20 @@ class TagNamesField(forms.CharField):
             if tag_length > askbot_settings.MAX_TAG_LENGTH:
                 #singular form is odd in english, but required for pluralization
                 #in other languages
-                msg = ungettext_lazy('each tag must be shorter than %(max_chars)d character',#odd but added for completeness
-                                'each tag must be shorter than %(max_chars)d characters',
-                                tag_length) % {'max_chars':tag_length}
+                msg = ungettext_lazy(
+                    #odd but added for completeness
+                    'each tag must be shorter than %(max_chars)d character',
+                    'each tag must be shorter than %(max_chars)d characters',
+                    tag_length
+                ) % {'max_chars':tag_length}
                 raise forms.ValidationError(msg)
 
             #todo - this needs to come from settings
             tagname_re = re.compile(const.TAG_REGEX, re.UNICODE)
             if not tagname_re.search(tag):
-                raise forms.ValidationError(_(
-                    'In tags, please use letters, numbers and characters "-+.#"'
-                ))
+                raise forms.ValidationError(
+                    _(message_keys.TAG_WRONG_CHARS_MESSAGE)
+                )
             #only keep unique tags
             if tag not in entered_tags:
                 entered_tags.append(tag)
diff --git a/askbot/skins/common/media/js/post.js b/askbot/skins/common/media/js/post.js
index 918f9856..9bb46fe7 100644
--- a/askbot/skins/common/media/js/post.js
+++ b/askbot/skins/common/media/js/post.js
@@ -2391,6 +2391,7 @@ GroupJoinButton.prototype.decorate = function(elem) {
 var TagEditor = function() {
     WrappedElement.call(this);
     this._has_hot_backspace = false;
+    this._settings = JSON.parse(askbot['settings']['tag_editor']);
 };
 inherits(TagEditor, WrappedElement);
 
@@ -2421,17 +2422,66 @@ TagEditor.prototype.getTagDeleteHandler = function(tag){
     var me = this;
     return function(){
         me.removeSelectedTag(tag.getName());
+        me.clearErrorMessage();
         tag.dispose();
         $('.acResults').hide();//a hack to hide the autocompleter
         me.fixHeight();
     };
 };
 
-TagEditor.prototype.addTag = function(tag_name) {
-    var tag_name = tag_name.replace(/\s+/, ' ').toLowerCase();
+TagEditor.prototype.cleanTag = function(tag_name) {
+    tag_name = $.trim(tag_name);
+    tag_name = tag_name.replace(/\s+/, ' ');
+
+    var force_lowercase = this._settings['force_lowercase_tags'];
+    if (force_lowercase) {
+        tag_name = tag_name.toLowerCase();
+    }
+
     if ($.inArray(tag_name, this.getSelectedTags()) !== -1) {
-        return;
+        throw interpolate(
+            gettext('tag "%s" was already added, no need to repeat'),
+            [tag_name]
+        );
+    }
+
+    var tag_regex = new RegExp(this._settings['tag_regex']);
+    if (tag_regex.test(tag_name) === false) {
+        throw this._settings['messages']['wrong_chars']
     }
+
+    var max_tags = this._settings['max_tags_per_post'];
+    if (this.getSelectedTags().length + 1 > max_tags) {//count current
+        throw interpolate(
+            ngettext(
+                'a maximum of %s tag is allowed',
+                'a maximum of %s tags are allowed',
+                max_tags
+            ),
+            [max_tags]
+        );
+    }
+
+    var max_length = this._settings['max_tag_length'];
+    if (tag_name.length > max_length) {
+        throw interpolate(
+            ngettext(
+                'must be shorter than %(max_chars)s character',
+                'must be shorter than %(max_chars)s characters',
+                max_length
+            ),
+            {'max_chars': max_length },
+            true
+        );
+    }
+    if (this._settings['force_lowercase_tags']) {
+        return tag_name.toLowerCase();
+    } else {
+        return tag_name;
+    }
+};
+
+TagEditor.prototype.addTag = function(tag_name) {
     var tag = new Tag();
     tag.setName(tag_name);
     tag.setDeletable(true);
@@ -2441,12 +2491,33 @@ TagEditor.prototype.addTag = function(tag_name) {
     this.addSelectedTag(tag_name);
 };
 
+TagEditor.prototype.clearErrorMessage = function() {
+    this._error_alert.html('');
+    //this._error_alert.fadeOut();
+    this._element.css('margin-top', '18px');//todo: the margin thing is a hack
+};
+
+TagEditor.prototype.setErrorMessage = function(text) {
+    var old_text = this._error_alert.html();
+    this._error_alert.html(text);
+    if (old_text == '') {
+        this._error_alert.hide();
+        this._error_alert.fadeIn(100);
+    }
+    this._element.css('margin-top', '0');//todo: remove this hack
+};
+
 TagEditor.prototype.getAddTagHandler = function() {
     var me = this;
     return function(tag_name) {
-        me.addTag(tag_name);
-        me.clearNewTagInput();
-        me.fixHeight();
+        try {
+            var clean_tag_name = me.cleanTag($.trim(tag_name));
+            me.addTag(clean_tag_name);
+            me.clearNewTagInput();
+            me.fixHeight();
+        } catch (error) {
+            me.setErrorMessage(error);
+        }
     };
 };
 
@@ -2481,9 +2552,12 @@ TagEditor.prototype.hasHotBackspace = function() {
 
 TagEditor.prototype.completeTagInput = function() {
     var tag_name = $.trim(this._visible_tags_input.val());
-    if (tag_name.length > 0) {
+    try {
+        tag_name = this.cleanTag(tag_name);
         this.addTag(tag_name);
         this.clearNewTagInput();
+    } catch (error) {
+        this.setErrorMessage(error);
     }
 };
 
@@ -2508,6 +2582,10 @@ TagEditor.prototype.fixHeight = function() {
     this.saveHeight();
 };
 
+TagEditor.prototype.closeAutoCompleter = function() {
+    this._autocompleter.finish();
+};
+
 TagEditor.prototype.getTagInputKeyHandler = function() {
     var new_tags = this._visible_tags_input;
     var me = this;
@@ -2518,20 +2596,48 @@ TagEditor.prototype.getTagInputKeyHandler = function() {
         me.saveHeight();
         var key = e.which || e.keyCode;
         var text = me.getRawNewTagValue();
-        //space 32, backspace 8, enter 13
+
+        //space 32, enter 13
         if (key == 32 || key == 13) {
             var tag_name = $.trim(text);
             if (tag_name.length > 0) {
                 me.completeTagInput();
             }
-        } else if (key == 8 && text.length == 0) {
+            me.fixHeight();
+            return false;
+        }
+
+        if (text == '') {
+            me.clearErrorMessage();
+            me.closeAutoCompleter();
+        } else {
+            try {
+                /* do-nothing validation here
+                 * just to report any errors while 
+                 * the user is typing */
+                me.cleanTag(text);
+                me.clearErrorMessage();
+            } catch (error) {
+                me.setErrorMessage(error);
+            }
+        }
+        
+        //8 is backspace
+        if (key == 8 && text.length == 0) {
             if (me.hasHotBackspace() === true) {
                 me.editLastTag();
+                me.setHotBackspace(false);
             } else {
                 me.setHotBackspace(true);
             }
         }
 
+        //27 is escape
+        if (key == 27) {
+            me.clearNewTagInput();
+            me.clearErrorMessage();
+        }
+
         if (key !== 8) {
             me.setHotBackspace(false);
         }
@@ -2544,6 +2650,7 @@ TagEditor.prototype.decorate = function(element) {
     this._element = element;
     this._hidden_tags_input = element.find('input[name="tags"]');//this one is hidden
     this._tags_container = element.find('ul.tags');
+    this._error_alert = $('.tag-editor-error-alert');
 
     var me = this;
     this._tags_container.children().each(function(idx, elem){
@@ -2572,6 +2679,7 @@ TagEditor.prototype.decorate = function(element) {
         delay: 10
     });
     tagsAc.decorate(visible_tags_input);
+    this._autocompleter = tagsAc;
     visible_tags_input.keyup(this.getTagInputKeyHandler());
 
     element.click(function(e) {
diff --git a/askbot/skins/default/media/style/style.less b/askbot/skins/default/media/style/style.less
index 193d072c..5b33e044 100644
--- a/askbot/skins/default/media/style/style.less
+++ b/askbot/skins/default/media/style/style.less
@@ -9,7 +9,7 @@ body {
     line-height: 150%;
     margin: 0;
     padding: 0;
-    color: #000;
+    color: #666;
     font-family:@body-font;
 }
 
@@ -1351,7 +1351,7 @@ ul#related-tags li {
     margin-bottom: 5px;
 }
 
-.ask-page {
+.ask-page, .question-page {
     .title-desc, .tags-desc {
         color: @info-text;
         font-style: italic;
@@ -3526,8 +3526,14 @@ textarea.tipped-input {
     }
 }
 
-.question-page .category-selector ul.select-box {
-    width: 217px;
+.question-page {
+    .category-selector ul.select-box {
+        width: 217px;
+    }
+    .tag-editor {
+        width: 660px;
+        margin-left: 0;
+    }
 }
 
 /* tag editor */
@@ -3539,6 +3545,7 @@ textarea.tipped-input {
         margin: 0;
         li {
             margin-top: 8px;
+            height: 13px;
         }
     }
     input.new-tags-input {
diff --git a/askbot/skins/default/templates/meta/editor_data.html b/askbot/skins/default/templates/meta/editor_data.html
index 2363281c..25c47739 100644
--- a/askbot/skins/default/templates/meta/editor_data.html
+++ b/askbot/skins/default/templates/meta/editor_data.html
@@ -9,7 +9,8 @@
     askbot['messages']['maxTagsPerPost'] = '{% trans tag_count = settings.MAX_TAGS_PER_POST %}please use {{tag_count}} tag{% pluralize %}please use {{tag_count}} tags or less{% endtrans %}';
     askbot['messages']['tagLimits'] = '{% trans tag_count=settings.MAX_TAGS_PER_POST, max_chars=settings.MAX_TAG_LENGTH %}please use up to {{tag_count}} tags, less than {{max_chars}} characters each{% endtrans %}'; 
     askbot['urls']['upload'] = '{% url "upload" %}';
-    askbot['settings']['minTitleLength'] = {{settings.MIN_TITLE_LENGTH}}
-    askbot['settings']['minQuestionBodyLength'] = {{settings.MIN_QUESTION_BODY_LENGTH}}
-    askbot['settings']['minAnswerBodyLength'] = {{settings.MIN_ANSWER_BODY_LENGTH}}
+    askbot['settings']['minTitleLength'] = {{settings.MIN_TITLE_LENGTH}};
+    askbot['settings']['minQuestionBodyLength'] = {{settings.MIN_QUESTION_BODY_LENGTH}};
+    askbot['settings']['minAnswerBodyLength'] = {{settings.MIN_ANSWER_BODY_LENGTH}};
+    askbot['settings']['tag_editor'] = '{{ tag_editor_settings|escapejs }}';
 </script>
diff --git a/askbot/skins/default/templates/widgets/tag_editor.html b/askbot/skins/default/templates/widgets/tag_editor.html
index d6befb02..53a73130 100644
--- a/askbot/skins/default/templates/widgets/tag_editor.html
+++ b/askbot/skins/default/templates/widgets/tag_editor.html
@@ -1,5 +1,7 @@
 {% import "macros.html" as macros %}
-<div class="tag-editor">
+<p style="margin:-18px 0 0 42px; color: brown; font-size: 13px; font-style: italic"
+    class="tag-editor-error-alert"></p>
+<div class="tag-editor" style="margin-top: 18px">{# there's a hack with the margin in js as well #}
     {{ macros.tag_list_widget(
                         tag_names,
                         deletable = True,
@@ -17,7 +19,6 @@
         type="hidden"
         value="{% if tag_names %}{{tag_names|join(' ')}}{% endif %}"
     />
-    <span class="form-error" style="display:block;"></span>
     {# 
         tag input is hidden because we want to edit the list visually 
         we also want to eventually allow multiword tags, which are
diff --git a/askbot/views/context.py b/askbot/views/context.py
new file mode 100644
index 00000000..12b77bd9
--- /dev/null
+++ b/askbot/views/context.py
@@ -0,0 +1,22 @@
+"""functions, preparing parts of context for
+the templates in the various views"""
+from django.utils import simplejson
+from django.utils.translation import ugettext as _
+from askbot.conf import settings as askbot_settings
+from askbot import const
+from askbot.const import message_keys as msg
+
+def get_for_tag_editor():
+    #data for the tag editor
+    data = {
+        'tag_regex': const.TAG_REGEX,
+        'tags_are_required': askbot_settings.TAGS_ARE_REQUIRED,
+        'max_tags_per_post': askbot_settings.MAX_TAGS_PER_POST,
+        'max_tag_length': askbot_settings.MAX_TAG_LENGTH,
+        'force_lowercase_tags': askbot_settings.FORCE_LOWERCASE_TAGS,
+        'messages': {
+            'required': _(msg.TAGS_ARE_REQUIRED_MESSAGE),
+            'wrong_chars': _(msg.TAG_WRONG_CHARS_MESSAGE)
+        }
+    }
+    return {'tag_editor_settings': simplejson.dumps(data)}
diff --git a/askbot/views/readers.py b/askbot/views/readers.py
index ea67a05d..c4778d7a 100644
--- a/askbot/views/readers.py
+++ b/askbot/views/readers.py
@@ -40,6 +40,7 @@ from askbot.templatetags import extra_tags
 import askbot.conf
 from askbot.conf import settings as askbot_settings
 from askbot.skins.loaders import render_into_skin, get_template #jinja2 template loading enviroment
+from askbot.views import context
 
 # used in index page
 #todo: - take these out of const or settings
@@ -554,6 +555,8 @@ def question(request, id):#refactor - long subroutine. display question body, an
         'show_comment_position': show_comment_position,
     }
 
+    data.update(context.get_for_tag_editor())
+
     return render_into_skin('question.html', data, request)
 
 def revisions(request, id, post_type = None):
diff --git a/askbot/views/writers.py b/askbot/views/writers.py
index 488d59d9..41391887 100644
--- a/askbot/views/writers.py
+++ b/askbot/views/writers.py
@@ -33,6 +33,7 @@ from askbot.utils.functions import diff_date
 from askbot.utils import url_utils
 from askbot.utils.file_utils import store_file
 from askbot.utils import category_tree
+from askbot.views import context
 from askbot.templatetags import extra_filters_jinja as template_filters
 from askbot.importers.stackexchange import management as stackexchange#todo: may change
 
@@ -270,6 +271,7 @@ def ask(request):#view used to ask a new question
         'category_tree_data': category_tree.get_data(),
         'tag_names': list()#need to keep context in sync with edit_question for tag editor
     }
+    data.update(context.get_for_tag_editor())
     return render_into_skin('ask.html', data, request)
 
 @login_required
@@ -408,6 +410,7 @@ def edit_question(request, id):
             'tag_names': question.thread.get_tag_names(),
             'category_tree_data': category_tree.get_data()
         }
+        data.update(context.get_for_tag_editor())
         return render_into_skin('question_edit.html', data, request)
 
     except exceptions.PermissionDenied, e: