improved tag validation messages and fixed bug where tags were lost in the input box on edit

8 files changed, 63 insertions, 11 deletions

diff --git a/askbot/forms.py b/askbot/forms.py
index 8548683f..0659e338 100644
--- a/askbot/forms.py
+++ b/askbot/forms.py
@@ -365,6 +365,10 @@ class TagNamesField(forms.CharField):
             attrs={'size': 50, 'autocomplete': 'off'}
         )
         self.max_length = 255
+        self.error_messages['max_length'] = _(
+                            'We ran out of space for recording the tags. '
+                            'Please shorten or delete some of them.'
+                        )
         self.label = _('tags')
         self.help_text = ungettext_lazy(
             'Tags are short keywords, with no spaces within. '
@@ -412,6 +416,11 @@ class TagNamesField(forms.CharField):
             if cleaned_tag not in cleaned_entered_tags:
                 cleaned_entered_tags.append(clean_tag(tag))
 
+        result = u' '.join(cleaned_entered_tags)
+
+        if len(result) > 125:#magic number!, the same as max_length in db
+            raise forms.ValidationError(self.error_messages['max_length'])
+
         return u' '.join(cleaned_entered_tags)
 
 
diff --git a/askbot/media/js/post.js b/askbot/media/js/post.js
index 4c0dfa67..483973ec 100644
--- a/askbot/media/js/post.js
+++ b/askbot/media/js/post.js
@@ -1241,7 +1241,6 @@ var questionRetagger = function(){
         var div = $('<form method="post"></form>');
         tagInput = $('<input id="retag_tags" type="text" autocomplete="off" name="tags" size="30"/>');
         //var tagLabel = $('<label for="retag_tags" class="error"></label>');
-        tagInput.val(old_tags_string);
         //populate input
         var tagAc = new AutoCompleter({
             url: askbot['urls']['get_tag_list'],
@@ -1253,6 +1252,7 @@ var questionRetagger = function(){
             delay: 10
         });
         tagAc.decorate(tagInput);
+        tagInput.val(old_tags_string);
         div.append(tagInput);
         //div.append(tagLabel);
         setupInputEventHandlers(tagInput);
diff --git a/askbot/models/post.py b/askbot/models/post.py
index e15516eb..72b1b680 100644
--- a/askbot/models/post.py
+++ b/askbot/models/post.py
@@ -1944,11 +1944,11 @@ class PostRevision(models.Model):
     )
 
     post = models.ForeignKey('askbot.Post', related_name='revisions', null=True, blank=True)
-    revision   = models.PositiveIntegerField()
-    author     = models.ForeignKey('auth.User', related_name='%(class)ss')
+    revision = models.PositiveIntegerField()
+    author = models.ForeignKey('auth.User', related_name='%(class)ss')
     revised_at = models.DateTimeField()
-    summary    = models.CharField(max_length=300, blank=True)
-    text       = models.TextField()
+    summary = models.CharField(max_length=300, blank=True)
+    text = models.TextField()
 
     approved = models.BooleanField(default=False, db_index=True)
     approved_by = models.ForeignKey(User, null = True, blank = True)
@@ -1958,8 +1958,8 @@ class PostRevision(models.Model):
     email_address = models.EmailField(null = True, blank = True)
 
     # Question-specific fields
-    title      = models.CharField(max_length=300, blank=True, default='')
-    tagnames   = models.CharField(max_length=125, blank=True, default='')
+    title = models.CharField(max_length=300, blank=True, default='')
+    tagnames = models.CharField(max_length=125, blank=True, default='')
     is_anonymous = models.BooleanField(default=False)
 
     objects = PostRevisionManager()
@@ -2102,10 +2102,10 @@ class PostRevision(models.Model):
         # Determine the revision number, if not set
         if not self.revision:
             # TODO: Maybe use Max() aggregation? Or `revisions.count() + 1`
-            self.revision = self.parent().revisions.values_list('revision', flat=True)[0] + 1
-
+            self.revision = self.parent().revisions.values_list(
+                                                'revision', flat=True
+                                            )[0] + 1
         self.full_clean()
-
         super(PostRevision, self).save(**kwargs)
 
     def get_absolute_url(self):
diff --git a/askbot/startup_procedures.py b/askbot/startup_procedures.py
index bfce701b..c6ab75a6 100644
--- a/askbot/startup_procedures.py
+++ b/askbot/startup_procedures.py
@@ -313,6 +313,19 @@ class SettingsTester(object):
                 '\n\n* '.join(self.messages)
             )
 
+
+def test_new_skins():
+    """tests that there are no directories in the `askbot/skins`
+    because we've moved skin files a few levels up"""
+    askbot_root = askbot.get_install_directory()
+    for item in os.listdir(os.path.join(askbot_root, 'skins')):
+        item_path = os.path.join(askbot_root, 'skins', item)
+        if os.path.isdir(item_path):
+            raise AskbotConfigError(
+                ('Time to move skin files from %s.\n'
+                'Now we have `askbot/templates` and `askbot/media`') % item_path
+            )
+
 def test_staticfiles():
     """tests configuration of the staticfiles app"""
     errors = list()
@@ -623,6 +636,7 @@ def run_startup_tests():
     #test_csrf_cookie_domain()
     test_tinymce()
     test_staticfiles()
+    test_new_skins()
     test_longerusername()
     test_avatar()
     settings_tester = SettingsTester({
diff --git a/askbot/tests/db_api_tests.py b/askbot/tests/db_api_tests.py
index fa166ba7..ec4210e8 100644
--- a/askbot/tests/db_api_tests.py
+++ b/askbot/tests/db_api_tests.py
@@ -10,6 +10,7 @@ from django.conf import settings
 from django.contrib.auth.models import AnonymousUser
 from django import forms
 from askbot.tests.utils import AskbotTestCase
+from askbot.tests.utils import with_settings
 from askbot import models
 from askbot import const
 from askbot.conf import settings as askbot_settings
@@ -164,6 +165,19 @@ class DBApiTests(AskbotTestCase):
 
         count = models.Tag.objects.filter(name='one-tag').count()
         self.assertEquals(count, 0)
+    
+    @with_settings({
+        'MAX_TAG_LENGTH': 200,
+        'MAX_TAGS_PER_POST': 50
+    })
+    def test_retag_tags_too_long_raises(self):
+        tags = "aoaoesuouooeueooeuoaeuoeou aostoeuoaethoeastn oasoeoa nuhoasut oaeeots aoshootuheotuoehao asaoetoeatuoasu o  aoeuethut aoaoe uou uoetu uouuou ao aouosutoeh"
+        question = self.post_question(user=self.user)
+        self.assertRaises(
+            exceptions.ValidationError, 
+            self.user.retag_question,
+            question=question, tags=tags
+        )
 
     def test_search_with_apostrophe_works(self):
         self.post_question(
diff --git a/askbot/utils/decorators.py b/askbot/utils/decorators.py
index 0042df8f..da1cd7db 100644
--- a/askbot/utils/decorators.py
+++ b/askbot/utils/decorators.py
@@ -87,6 +87,7 @@ def ajax_only(view_func):
             if data is None:
                 data = {}
         except Exception, e:
+            #todo: also check field called "message"
             if hasattr(e, 'messages'):
                 if len(e.messages) > 1:
                     message = u'<ul>' + \
diff --git a/askbot/utils/forms.py b/askbot/utils/forms.py
index 6c3af066..6f57f71f 100644
--- a/askbot/utils/forms.py
+++ b/askbot/utils/forms.py
@@ -47,6 +47,18 @@ def get_db_object_or_404(params):
         #need catch-all b/c of the nature of the function
         raise Http404
 
+def format_errors(error_list):
+    """If there is only one error - returns a string
+    corresponding to that error, to remove the <ul> tag.
+
+    If there is > 1 error - then convert the error_list into
+    a string.
+    """
+    if len(error_list) == 1:
+        return unicode(error_list[0])
+    else:
+        return unicode(error_list)
+
 class StrippedNonEmptyCharField(forms.CharField):
     def clean(self, value):
         value = value.strip()
diff --git a/askbot/views/writers.py b/askbot/views/writers.py
index 85a92d23..4024b4b0 100644
--- a/askbot/views/writers.py
+++ b/askbot/views/writers.py
@@ -31,6 +31,7 @@ from askbot import models
 from askbot.conf import settings as askbot_settings
 from askbot.skins.loaders import render_into_skin
 from askbot.utils import decorators
+from askbot.utils.forms import format_errors
 from askbot.utils.functions import diff_date
 from askbot.utils import url_utils
 from askbot.utils.file_utils import store_file
@@ -314,6 +315,7 @@ def retag_question(request, id):
         request.user.assert_can_retag_question(question)
         if request.method == 'POST':
             form = forms.RetagQuestionForm(question, request.POST)
+
             if form.is_valid():
                 if form.has_changed():
                     request.user.retag_question(question=question, tags=form.cleaned_data['tags'])
@@ -334,7 +336,7 @@ def retag_question(request, id):
                     return HttpResponseRedirect(question.get_absolute_url())
             elif request.is_ajax():
                 response_data = {
-                    'message': unicode(form.errors['tags']),
+                    'message': format_errors(form.errors['tags']),
                     'success': False
                 }
                 data = simplejson.dumps(response_data)