EDUCATOR-1787 Allow hyphen character in course number

AUTHORS

 Calen Pennington <cale@edx.org>
 Clinton Blackburn <cblackburn@edx.org>
 Bill DeRusha <bill@edx.org>
+Rabia Iftikhar <rabiaiftikhar2392@gmail.com>

course_discovery/apps/course_metadata/migrations/0070_auto_20171127_1057.py

+# -*- coding: utf-8 -*-
+# Generated by Django 1.11.3 on 2017-11-27 10:57
+from __future__ import unicode_literals
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('course_metadata', '0069_courseentitlement_expires'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='organization',
+            name='key',
+            field=models.CharField(help_text="Please do not use any spaces or special characters other than period, underscore or hyphen. This key will be used in the course's course key.", max_length=255),
+        ),
+    ]

course_discovery/apps/course_metadata/models.py

 import datetime
 import itertools
 import logging
-import re
 from collections import defaultdict
 from urllib.parse import urljoin
 from uuid import uuid4
@@ -33,6 +32,8 @@ from course_discovery.apps.course_metadata.publishers import (
 from course_discovery.apps.course_metadata.query import CourseQuerySet, CourseRunQuerySet, ProgramQuerySet
 from course_discovery.apps.course_metadata.utils import UploadToFieldNamePath, clean_query, custom_render_variations
 from course_discovery.apps.ietf_language_tags.models import LanguageTag
+from course_discovery.apps.publisher.utils import VALID_CHARS_IN_COURSE_NUM_AND_ORG_KEY
+
 
 logger = logging.getLogger(__name__)
 
@@ -176,7 +177,9 @@ class Organization(TimeStampedModel):
     """ Organization model. """
     partner = models.ForeignKey(Partner, null=True, blank=False)
     uuid = models.UUIDField(blank=False, null=False, default=uuid4, editable=False, verbose_name=_('UUID'))
-    key = models.CharField(max_length=255, help_text=_('Only ascii characters allowed (a-zA-Z0-9)'))
+    key = models.CharField(max_length=255, help_text=_('Please do not use any spaces or special characters other '
+                                                       'than period, underscore or hyphen. This key will be used '
+                                                       'in the course\'s course key.'))
     name = models.CharField(max_length=255)
     marketing_url_path = models.CharField(max_length=255, null=True, blank=True)
     description = models.TextField(null=True, blank=True)
@@ -194,8 +197,9 @@ class Organization(TimeStampedModel):
     )
 
     def clean(self):
-        if not re.match("^[a-zA-Z0-9_-]*$", self.key):
-            raise ValidationError(_('Please do not use any spaces or special characters in the key field'))
+        if not VALID_CHARS_IN_COURSE_NUM_AND_ORG_KEY.match(self.key):
+            raise ValidationError(_('Please do not use any spaces or special characters other than period, '
+                                    'underscore or hyphen in the key field.'))
 
     class Meta:
         unique_together = (

course_discovery/apps/course_metadata/tests/test_models.py

@@ -366,27 +366,25 @@ class OrganizationTests(TestCase):
         self.organization = factories.OrganizationFactory()
 
     @ddt.data(
-        "key with space",
-        "key[with,special",
-        "keyó"
+        [" ", ",", "@", "(", "!", "#", "$", "%", "^", "&", "*", "+", "=", "{", "[", "ó"]
     )
-    def test_clean_error(self, key):
+    def test_clean_error(self, invalid_char_list):
         """
         Verify that the clean method raises validation error if key consists of special characters
         """
-        self.organization.key = key
+        for char in invalid_char_list:
+            self.organization.key = 'key{}'.format(char)
             self.assertRaises(ValidationError, self.organization.clean)
 
     @ddt.data(
-        "keywithoutspace",
-        "correctkey",
-        "correct_key"
+        ["keywithoutspace", "correct-key", "correct_key", "correct.key"]
     )
-    def test_clean_success(self, key):
+    def test_clean_success(self, valid_key_list):
         """
         Verify that the clean method returns None if key is valid
         """
-        self.organization.key = key
+        for valid_key in valid_key_list:
+            self.organization.key = valid_key
             self.assertEqual(self.organization.clean(), None)
 
     def test_str(self):

course_discovery/apps/publisher/forms.py

 import html
 import logging
-import re
 
 import waffle
 from dal import autocomplete
@@ -16,9 +15,10 @@ from course_discovery.apps.course_metadata.choices import CourseRunPacing
 from course_discovery.apps.course_metadata.models import LevelType, Organization, Person, Subject
 from course_discovery.apps.ietf_language_tags.models import LanguageTag
 from course_discovery.apps.publisher.mixins import LanguageModelSelect2Multiple, get_user_organizations
-from course_discovery.apps.publisher.models import (Course, CourseRun, CourseUserRole, OrganizationExtension,
-                                                    OrganizationUserRole, PublisherUser, Seat, User)
-from course_discovery.apps.publisher.utils import is_internal_user
+from course_discovery.apps.publisher.models import (
+    Course, CourseRun, CourseUserRole, OrganizationExtension, OrganizationUserRole, PublisherUser, Seat, User
+)
+from course_discovery.apps.publisher.utils import VALID_CHARS_IN_COURSE_NUM_AND_ORG_KEY, is_internal_user
 from course_discovery.apps.publisher.validators import validate_text_count
 
 logger = logging.getLogger(__name__)
@@ -96,7 +96,7 @@ class CourseForm(BaseForm):
         label=_('Organization Course Admin'),
     )
 
-    subjects = Subject.objects.all().order_by("translations__name")
+    subjects = Subject.objects.all().order_by('translations__name')
     primary_subject = forms.ModelChoiceField(
         queryset=subjects,
         label=_('Primary'),
@@ -178,22 +178,23 @@ class CourseForm(BaseForm):
         Convert all named and numeric character references in the string
         to the corresponding unicode characters
         """
-        return html.unescape(self.cleaned_data.get("title"))
+        return html.unescape(self.cleaned_data.get('title'))
 
     def clean_number(self):
         """
-        Validate that number doesn't consist of any special characters
+        Validate that number doesn't consist of any special characters other than period, underscore or hyphen
         """
-        number = self.cleaned_data.get("number")
-        if not re.match("^[a-zA-Z0-9_.]*$", number):
-            raise ValidationError(_('Please do not use any spaces or special characters.'))
+        number = self.cleaned_data.get('number')
+        if not VALID_CHARS_IN_COURSE_NUM_AND_ORG_KEY.match(number):
+            raise ValidationError(_('Please do not use any spaces or special characters other than period, '
+                                    'underscore or hyphen.'))
         return number
 
     def clean(self):
         cleaned_data = self.cleaned_data
-        organization = cleaned_data.get("organization")
-        title = cleaned_data.get("title")
-        number = cleaned_data.get("number")
+        organization = cleaned_data.get('organization')
+        title = cleaned_data.get('title')
+        number = cleaned_data.get('number')
         instance = getattr(self, 'instance', None)
         if not instance.pk:
             if Course.objects.filter(title=title, organizations__in=[organization]).exists():
@@ -306,7 +307,7 @@ class CourseRunForm(BaseForm):
             try:
                 CourseKey.from_string(lms_course_id)
             except InvalidKeyError:
-                raise ValidationError("Invalid course key.")
+                raise ValidationError('Invalid course key.')
 
             return lms_course_id
 
@@ -320,16 +321,16 @@ class CourseRunForm(BaseForm):
 
     def clean(self):
         cleaned_data = self.cleaned_data
-        min_effort = cleaned_data.get("min_effort")
-        max_effort = cleaned_data.get("max_effort")
-        start = cleaned_data.get("start")
-        end = cleaned_data.get("end")
-        is_xseries = cleaned_data.get("is_xseries")
-        xseries_name = cleaned_data.get("xseries_name")
-        is_micromasters = cleaned_data.get("is_micromasters")
-        micromasters_name = cleaned_data.get("micromasters_name")
-        is_professional_certificate = cleaned_data.get("is_professional_certificate")
-        professional_certificate_name = cleaned_data.get("professional_certificate_name")
+        min_effort = cleaned_data.get('min_effort')
+        max_effort = cleaned_data.get('max_effort')
+        start = cleaned_data.get('start')
+        end = cleaned_data.get('end')
+        is_xseries = cleaned_data.get('is_xseries')
+        xseries_name = cleaned_data.get('xseries_name')
+        is_micromasters = cleaned_data.get('is_micromasters')
+        micromasters_name = cleaned_data.get('micromasters_name')
+        is_professional_certificate = cleaned_data.get('is_professional_certificate')
+        professional_certificate_name = cleaned_data.get('professional_certificate_name')
         if start and end and start > end:
             raise ValidationError({'start': _('Start date cannot be after the End date')})
         if min_effort and max_effort and min_effort > max_effort:
@@ -521,7 +522,7 @@ class CourseRunAdminForm(forms.ModelForm):
             try:
                 CourseKey.from_string(lms_course_id)
             except InvalidKeyError:
-                raise ValidationError(_("Invalid course key."))
+                raise ValidationError(_('Invalid course key.'))
 
             return lms_course_id
 

course_discovery/apps/publisher/templates/publisher/add_course_form.html

@@ -107,7 +107,7 @@
                         </div>
                         <div id="tab-practices" class="content active">
                             <ul>
-                                <li>{% trans "Maximum 50 characters. Characters can be letters, numbers, or periods." %}</li>
+                                <li>{% trans "Maximum 50 characters. Characters can be letters, numbers, periods, underscores or hyphens." %}</li>
                                 <li>{% trans "If a course consists of several modules, the course number can have an ending such as .1x or .2x." %}</li>
                             </ul>
                         </div>

course_discovery/apps/publisher/templates/publisher/course_edit_form.html

@@ -131,7 +131,7 @@
                                         </div>
                                         <div id="tab-practices" class="content active">
                                             <ul>
-                                                <li>{% trans "Maximum 10 characters. Characters can be letters, numbers, or periods." %}</li>
+                                                <li>{% trans "Maximum 50 characters. Characters can be letters, numbers, periods, underscores or hyphens." %}</li>
                                                 <li>{% trans "If a course consists of several modules, the course number can have an ending such as .1x or .2x." %}</li>
                                             </ul>
                                         </div>
@@ -145,6 +145,11 @@
                                     <div class="col col-6">
                                         <label class="field-label ">{{ form.number.label_tag }} <span class="required">*</span></label>
                                         {{ form.number }}
+                                        {% if form.number.errors %}
+                                            <div class="field-message-content">
+                                                {{ form.number.errors|escape }}
+                                            </div>
+                                        {% endif %}
                                     </div>
                                 </div>
                             </fieldset>

course_discovery/apps/publisher/tests/test_forms.py

 from datetime import datetime, timedelta
 
+import ddt
 import pytest
 from django.core.exceptions import ValidationError
 from django.test import TestCase
@@ -166,6 +167,7 @@ class PublisherCourseRunEditFormTests(TestCase):
         self.assertEqual(run_form.clean(), run_form.cleaned_data)
 
 
+@ddt.ddt
 class PublisherCustomCourseFormTests(TestCase):
     """
     Tests for publisher 'CourseForm'
@@ -210,7 +212,7 @@ class PublisherCustomCourseFormTests(TestCase):
         course_form.cleaned_data['title'] = "Test2"
         self.assertEqual(course_form.clean(), course_form.cleaned_data)
 
-    def test_duplicate_number(self):
+    def test_duplicate_course_number(self):
         """
         Verify that clean raises 'ValidationError' if the course number is a duplicate of another course number
         within the same organization
@@ -223,21 +225,32 @@ class PublisherCustomCourseFormTests(TestCase):
         course_form.cleaned_data['number'] = "123a"
         self.assertEqual(course_form.clean(), course_form.cleaned_data)
 
-    def test_invalid_number(self):
+    @ddt.data(
+        [" ", ",", "@", "(", "!", "#", "$", "%", "^", "&", "*", "+", "=", "{", "[", "ó"]
+    )
+    def test_invalid_course_number(self, invalid_char_list):
         """
         Verify that clean_number raises 'ValidationError' if the course number consists of special characters
-        or spaces
+        or spaces other than underscore,hyphen or period
         """
         course_form = CourseForm()
-        course_form.cleaned_data = {'number': '123 a'}
+        for invalid_char in invalid_char_list:
+            course_form.cleaned_data = {'number': 'course_num{}'.format(invalid_char)}
             with self.assertRaises(ValidationError):
                 course_form.clean_number()
 
-        course_form.cleaned_data['number'] = "123.a"
-        self.assertEqual(course_form.clean_number(), "123.a")
-
-        course_form.cleaned_data['number'] = "123a"
-        self.assertEqual(course_form.clean_number(), "123a")
+    @ddt.data(
+        ["123a", "123_a", "123.a", "123-a", "XYZ123"]
+    )
+    def test_valid_course_number(self, valid_number_list):
+        """
+        Verify that clean_number allows alphanumeric(a-zA-Z0-9) characters, period, underscore and hyphen
+        in course number
+        """
+        course_form = CourseForm()
+        for valid_number in valid_number_list:
+            course_form.cleaned_data = {'number': valid_number}
+            self.assertEqual(course_form.clean_number(), valid_number)
 
     def test_course_title_formatting(self):
         """

course_discovery/apps/publisher/utils.py

 """ Publisher Utils."""
+import re
 from dateutil import parser
 
 from course_discovery.apps.core.models import User
 from course_discovery.apps.publisher.constants import (ADMIN_GROUP_NAME, INTERNAL_USER_GROUP_NAME,
                                                        PROJECT_COORDINATOR_GROUP_NAME)
 
+VALID_CHARS_IN_COURSE_NUM_AND_ORG_KEY = re.compile(r'^[a-zA-Z0-9._-]*$')
+
 
 def is_email_notification_enabled(user):
     """ Check email notification is enabled for the user.

course_discovery/conf/locale/en/LC_MESSAGES/django.po

@@ -7,7 +7,7 @@ msgid ""
 msgstr ""
 "Project-Id-Version: PACKAGE VERSION\n"
 "Report-Msgid-Bugs-To: \n"
-"POT-Creation-Date: 2017-11-15 14:00+0000\n"
+"POT-Creation-Date: 2017-11-28 09:26+0000\n"
 "PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n"
 "Last-Translator: FULL NAME <EMAIL@ADDRESS>\n"
 "Language-Team: LANGUAGE <LL@li.org>\n"
@@ -294,7 +294,9 @@ msgid "Subject model translations"
 msgstr ""
 
 #: apps/course_metadata/models.py
-msgid "Only ascii characters allowed (a-zA-Z0-9)"
+msgid ""
+"Please do not use any spaces or special characters other than period, "
+"underscore or hyphen. This key will be used in the course's course key."
 msgstr ""
 
 #: apps/course_metadata/models.py
@@ -310,7 +312,9 @@ msgid ""
 msgstr ""
 
 #: apps/course_metadata/models.py
-msgid "Please do not use any spaces or special characters in the key field"
+msgid ""
+"Please do not use any spaces or special characters other than period, "
+"underscore or hyphen in the key field."
 msgstr ""
 
 #: apps/course_metadata/models.py
@@ -698,7 +702,9 @@ msgid "Syllabus"
 msgstr ""
 
 #: apps/publisher/forms.py
-msgid "Please do not use any spaces or special characters."
+msgid ""
+"Please do not use any spaces or special characters other than period, "
+"underscore or hyphen."
 msgstr ""
 
 #: apps/publisher/forms.py
@@ -1332,7 +1338,10 @@ msgid "COURSE NUMBER"
 msgstr ""
 
 #: apps/publisher/templates/publisher/add_course_form.html
-msgid "Maximum 50 characters. Characters can be letters, numbers, or periods."
+#: apps/publisher/templates/publisher/course_edit_form.html
+msgid ""
+"Maximum 50 characters. Characters can be letters, numbers, periods, "
+"underscores or hyphens."
 msgstr ""
 
 #: apps/publisher/templates/publisher/add_course_form.html
@@ -1597,10 +1606,6 @@ msgid "255 character limit, including spaces."
 msgstr ""
 
 #: apps/publisher/templates/publisher/course_edit_form.html
-msgid "Maximum 10 characters. Characters can be letters, numbers, or periods."
-msgstr ""
-
-#: apps/publisher/templates/publisher/course_edit_form.html
 msgid "BIO1.1x, BIO1.2x"
 msgstr ""
 

course_discovery/conf/locale/en/LC_MESSAGES/djangojs.po

@@ -7,7 +7,7 @@ msgid ""
 msgstr ""
 "Project-Id-Version: PACKAGE VERSION\n"
 "Report-Msgid-Bugs-To: \n"
-"POT-Creation-Date: 2017-11-15 14:00+0000\n"
+"POT-Creation-Date: 2017-11-28 09:26+0000\n"
 "PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n"
 "Last-Translator: FULL NAME <EMAIL@ADDRESS>\n"
 "Language-Team: LANGUAGE <LL@li.org>\n"

course_discovery/conf/locale/eo/LC_MESSAGES/django.po

@@ -7,7 +7,7 @@ msgid ""
 msgstr ""
 "Project-Id-Version: PACKAGE VERSION\n"
 "Report-Msgid-Bugs-To: \n"
-"POT-Creation-Date: 2017-11-15 14:00+0000\n"
+"POT-Creation-Date: 2017-11-28 09:26+0000\n"
 "PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n"
 "Last-Translator: FULL NAME <EMAIL@ADDRESS>\n"
 "Language-Team: LANGUAGE <LL@li.org>\n"
@@ -347,10 +347,18 @@ msgid "Subject model translations"
 msgstr "Süßjéçt mödél tränslätïöns Ⱡ'σяєм ιρѕυм ∂σłσя ѕιт αмєт, ¢σηѕ#"
 
 #: apps/course_metadata/models.py
-msgid "Only ascii characters allowed (a-zA-Z0-9)"
+msgid ""
+"Please do not use any spaces or special characters other than period, "
+"underscore or hyphen. This key will be used in the course's course key."
 msgstr ""
-"Önlý äsçïï çhäräçtérs ällöwéd (ä-zÀ-Z0-9) Ⱡ'σяєм ιρѕυм ∂σłσя ѕιт αмєт, "
-"¢σηѕє¢тєтυя #"
+"Pléäsé dö nöt üsé äný späçés ör spéçïäl çhäräçtérs öthér thän pérïöd, "
+"ündérsçöré ör hýphén. Thïs kéý wïll ßé üséd ïn thé çöürsé's çöürsé kéý. "
+"Ⱡ'σяєм ιρѕυм ∂σłσя ѕιт αмєт, ¢σηѕє¢тєтυя α∂ιριѕι¢ιηg єłιт, ѕє∂ ∂σ єιυѕмσ∂ "
+"тємρσя ιη¢ι∂ι∂υηт υт łαвσяє єт ∂σłσяє мαgηα αłιqυα. υт єηιм α∂ мιηιм νєηιαм,"
+" qυιѕ ησѕтяυ∂ єχєя¢ιтαтιση υłłαм¢σ łαвσяιѕ ηιѕι υт αłιqυιρ єχ єα ¢σммσ∂σ "
+"¢σηѕєqυαт. ∂υιѕ αυтє ιяυяє ∂σłσя ιη яєρяєнєη∂єяιт ιη νσłυρтαтє νєłιт єѕѕє "
+"¢ιłłυм ∂σłσяє єυ ƒυgιαт ηυłłα ραяιαтυя. єχ¢єρтєυя ѕιηт σ¢¢αє¢αт ¢υρι∂αтαт "
+"ηση ρяσι∂єηт, ѕυηт ιη ¢υłρα qυι σƒƒι¢ια ∂єѕєяυηт мσłłιт αηιм ι∂ єѕт #"
 
 #: apps/course_metadata/models.py
 msgid ""
@@ -369,10 +377,12 @@ msgstr ""
 "täg nämé. Ⱡ'σяєм ιρѕυм ∂σłσя ѕιт αмєт, ¢σηѕє¢т#"
 
 #: apps/course_metadata/models.py
-msgid "Please do not use any spaces or special characters in the key field"
+msgid ""
+"Please do not use any spaces or special characters other than period, "
+"underscore or hyphen in the key field."
 msgstr ""
-"Pléäsé dö nöt üsé äný späçés ör spéçïäl çhäräçtérs ïn thé kéý fïéld Ⱡ'σяєм "
-"ιρѕυм ∂σłσя ѕιт αмєт, ¢σηѕє¢тєтυя #"
+"Pléäsé dö nöt üsé äný späçés ör spéçïäl çhäräçtérs öthér thän pérïöd, "
+"ündérsçöré ör hýphén ïn thé kéý fïéld. Ⱡ'σяєм ιρѕυм ∂σłσя ѕι#"
 
 #: apps/course_metadata/models.py
 msgid "People"
@@ -841,10 +851,12 @@ msgid "Syllabus"
 msgstr "Sýlläßüs Ⱡ'σяєм ιρѕυм ∂#"
 
 #: apps/publisher/forms.py
-msgid "Please do not use any spaces or special characters."
+msgid ""
+"Please do not use any spaces or special characters other than period, "
+"underscore or hyphen."
 msgstr ""
-"Pléäsé dö nöt üsé äný späçés ör spéçïäl çhäräçtérs. Ⱡ'σяєм ιρѕυм ∂σłσя ѕιт "
-"αмєт, ¢σηѕє¢тєтυя α#"
+"Pléäsé dö nöt üsé äný späçés ör spéçïäl çhäräçtérs öthér thän pérïöd, "
+"ündérsçöré ör hýphén. Ⱡ'σяєм ιρѕυм ∂σłσя ѕιт αмєт, ¢ση#"
 
 #: apps/publisher/forms.py
 msgid "This course title already exists"
@@ -1534,10 +1546,13 @@ msgid "COURSE NUMBER"
 msgstr "ÇÖÛRSÉ NÛMBÉR Ⱡ'σяєм ιρѕυм ∂σłσя ѕι#"
 
 #: apps/publisher/templates/publisher/add_course_form.html
-msgid "Maximum 50 characters. Characters can be letters, numbers, or periods."
+#: apps/publisher/templates/publisher/course_edit_form.html
+msgid ""
+"Maximum 50 characters. Characters can be letters, numbers, periods, "
+"underscores or hyphens."
 msgstr ""
-"Mäxïmüm 50 çhäräçtérs. Çhäräçtérs çän ßé léttérs, nümßérs, ör pérïöds. "
-"Ⱡ'σяєм ιρѕυм ∂σłσя ѕιт αмєт, ¢σηѕє¢тєтυя #"
+"Mäxïmüm 50 çhäräçtérs. Çhäräçtérs çän ßé léttérs, nümßérs, pérïöds, "
+"ündérsçörés ör hýphéns. Ⱡ'σяєм ιρѕυм ∂σłσя ѕιт αмєт, ¢ση#"
 
 #: apps/publisher/templates/publisher/add_course_form.html
 #: apps/publisher/templates/publisher/course_edit_form.html
@@ -1865,12 +1880,6 @@ msgstr ""
 "¢σηѕє¢тєтυя#"
 
 #: apps/publisher/templates/publisher/course_edit_form.html
-msgid "Maximum 10 characters. Characters can be letters, numbers, or periods."
-msgstr ""
-"Mäxïmüm 10 çhäräçtérs. Çhäräçtérs çän ßé léttérs, nümßérs, ör pérïöds. "
-"Ⱡ'σяєм ιρѕυм ∂σłσя ѕιт αмєт, ¢σηѕє¢тєтυя #"
-
-#: apps/publisher/templates/publisher/course_edit_form.html
 msgid "BIO1.1x, BIO1.2x"
 msgstr "BÌÖ1.1x, BÌÖ1.2x Ⱡ'σяєм ιρѕυм ∂σłσя ѕιт αм#"
 

course_discovery/conf/locale/eo/LC_MESSAGES/djangojs.po

@@ -7,7 +7,7 @@ msgid ""
 msgstr ""
 "Project-Id-Version: PACKAGE VERSION\n"
 "Report-Msgid-Bugs-To: \n"
-"POT-Creation-Date: 2017-11-15 14:00+0000\n"
+"POT-Creation-Date: 2017-11-28 09:26+0000\n"
 "PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n"
 "Last-Translator: FULL NAME <EMAIL@ADDRESS>\n"
 "Language-Team: LANGUAGE <LL@li.org>\n"