[frontend/course_admin] move tags page to settings page (#841)

inginious/frontend/flask/mapping.py

@@ -40,7 +40,6 @@
 from inginious.frontend.pages.course_admin.submission import SubmissionPage
 from inginious.frontend.pages.course_admin.submissions import CourseSubmissionsPage
 from inginious.frontend.pages.course_admin.task_list import CourseTaskListPage
-from inginious.frontend.pages.course_admin.tags import CourseTagsPage
 from inginious.frontend.pages.course_admin.audience_edit import CourseEditAudience
 from inginious.frontend.pages.course_admin.task_edit import CourseEditTask
 from inginious.frontend.pages.course_admin.task_edit_file import CourseTaskFiles
@@ -118,8 +117,6 @@ def init_flask_mapping(flask_app):
                            view_func=CourseSubmissionsPage.as_view('coursesubmissionspage'))
     flask_app.add_url_rule('/<cookieless:sessionid>admin/<courseid>/tasks',
                            view_func=CourseTaskListPage.as_view('coursetasklistpage'))
-    flask_app.add_url_rule('/<cookieless:sessionid>admin/<courseid>/tags',
-                           view_func=CourseTagsPage.as_view('coursetagspage'))
     flask_app.add_url_rule('/<cookieless:sessionid>admin/<courseid>/edit/audience/<audienceid>',
                            view_func=CourseEditAudience.as_view('courseditaudience'))
     flask_app.add_url_rule('/<cookieless:sessionid>admin/<courseid>/edit/task/<taskid>',

inginious/frontend/pages/course_admin/settings.py

@@ -6,6 +6,7 @@
 import re
 import flask
 
+from inginious.common.base import dict_from_prefix, id_checker
 from inginious.frontend.accessible_time import AccessibleTime
 from inginious.frontend.pages.course_admin.utils import INGIniousAdminPage
 
@@ -24,74 +25,77 @@ def POST_AUTH(self, courseid):  # pylint: disable=arguments-differ
 
         errors = []
         course_content = {}
+
+        data = flask.request.form
+        course_content = self.course_factory.get_course_descriptor_content(courseid)
+        course_content['name'] = data['name']
+        if course_content['name'] == "":
+            errors.append(_('Invalid name'))
+        course_content['description'] = data['description']
+        course_content['admins'] = list(map(str.strip, data['admins'].split(','))) if data['admins'].strip() else []
+        if not self.user_manager.user_is_superadmin() and self.user_manager.session_username() not in course_content['admins']:
+            errors.append(_('You cannot remove yourself from the administrators of this course'))
+        course_content['tutors'] = list(map(str.strip, data['tutors'].split(','))) if data['tutors'].strip() else []
+        if len(course_content['tutors']) == 1 and course_content['tutors'][0].strip() == "":
+            course_content['tutors'] = []
+
+        course_content['groups_student_choice'] = True if data["groups_student_choice"] == "true" else False
+
+        if data["accessible"] == "custom":
+            course_content['accessible'] = "{}/{}".format(data["accessible_start"], data["accessible_end"])
+        elif data["accessible"] == "true":
+            course_content['accessible'] = True
+        else:
+            course_content['accessible'] = False
+
+        try:
+            AccessibleTime(course_content['accessible'])
+        except:
+            errors.append(_('Invalid accessibility dates'))
+
+        course_content['allow_unregister'] = True if data["allow_unregister"] == "true" else False
+        course_content['allow_preview'] = True if data["allow_preview"] == "true" else False
+
+        if data["registration"] == "custom":
+            course_content['registration'] = "{}/{}".format(data["registration_start"], data["registration_end"])
+        elif data["registration"] == "true":
+            course_content['registration'] = True
+        else:
+            course_content['registration'] = False
+
         try:
-            data = flask.request.form
-            course_content = self.course_factory.get_course_descriptor_content(courseid)
-            course_content['name'] = data['name']
-            if course_content['name'] == "":
-                errors.append(_('Invalid name'))
-            course_content['description'] = data['description']
-            course_content['admins'] = list(map(str.strip, data['admins'].split(','))) if data['admins'].strip() else []
-            if not self.user_manager.user_is_superadmin() and self.user_manager.session_username() not in course_content['admins']:
-                errors.append(_('You cannot remove yourself from the administrators of this course'))
-            course_content['tutors'] = list(map(str.strip, data['tutors'].split(','))) if data['tutors'].strip() else []
-            if len(course_content['tutors']) == 1 and course_content['tutors'][0].strip() == "":
-                course_content['tutors'] = []
-
-            course_content['groups_student_choice'] = True if data["groups_student_choice"] == "true" else False
-
-            if data["accessible"] == "custom":
-                course_content['accessible'] = "{}/{}".format(data["accessible_start"], data["accessible_end"])
-            elif data["accessible"] == "true":
-                course_content['accessible'] = True
-            else:
-                course_content['accessible'] = False
-
-            try:
-                AccessibleTime(course_content['accessible'])
-            except:
-                errors.append(_('Invalid accessibility dates'))
-
-            course_content['allow_unregister'] = True if data["allow_unregister"] == "true" else False
-            course_content['allow_preview'] = True if data["allow_preview"] == "true" else False
-
-            if data["registration"] == "custom":
-                course_content['registration'] = "{}/{}".format(data["registration_start"], data["registration_end"])
-            elif data["registration"] == "true":
-                course_content['registration'] = True
-            else:
-                course_content['registration'] = False
-
-            try:
-                AccessibleTime(course_content['registration'])
-            except:
-                errors.append(_('Invalid registration dates'))
-
-            course_content['registration_password'] = data['registration_password']
-            if course_content['registration_password'] == "":
-                course_content['registration_password'] = None
-
-            course_content['registration_ac'] = data['registration_ac']
-            if course_content['registration_ac'] not in ["None", "username", "binding", "email"]:
-                errors.append(_('Invalid ACL value'))
-            if course_content['registration_ac'] == "None":
-                course_content['registration_ac'] = None
-
-            course_content['registration_ac_accept'] = True if data['registration_ac_accept'] == "true" else False
-            course_content['registration_ac_list'] = [line.strip() for line in data['registration_ac_list'].splitlines()]
-
-
-            course_content['is_lti'] = 'lti' in data and data['lti'] == "true"
-            course_content['lti_url'] = data.get("lti_url", "")
-            course_content['lti_keys'] = dict([x.split(":") for x in data['lti_keys'].splitlines() if x])
-
-            for lti_key in course_content['lti_keys'].keys():
-                if not re.match("^[a-zA-Z0-9]*$", lti_key):
-                    errors.append(_("LTI keys must be alphanumerical."))
-
-            course_content['lti_send_back_grade'] = 'lti_send_back_grade' in data and data['lti_send_back_grade'] == "true"
+            AccessibleTime(course_content['registration'])
         except:
-            errors.append(_('User returned an invalid form.'))
+            errors.append(_('Invalid registration dates'))
+
+        course_content['registration_password'] = data['registration_password']
+        if course_content['registration_password'] == "":
+            course_content['registration_password'] = None
+
+        course_content['registration_ac'] = data['registration_ac']
+        if course_content['registration_ac'] not in ["None", "username", "binding", "email"]:
+            errors.append(_('Invalid ACL value'))
+        if course_content['registration_ac'] == "None":
+            course_content['registration_ac'] = None
+
+        course_content['registration_ac_accept'] = True if data['registration_ac_accept'] == "true" else False
+        course_content['registration_ac_list'] = [line.strip() for line in data['registration_ac_list'].splitlines()]
+
+
+        course_content['is_lti'] = 'lti' in data and data['lti'] == "true"
+        course_content['lti_url'] = data.get("lti_url", "")
+        course_content['lti_keys'] = dict([x.split(":") for x in data['lti_keys'].splitlines() if x])
+
+        for lti_key in course_content['lti_keys'].keys():
+            if not re.match("^[a-zA-Z0-9]*$", lti_key):
+                errors.append(_("LTI keys must be alphanumerical."))
+
+        course_content['lti_send_back_grade'] = 'lti_send_back_grade' in data and data['lti_send_back_grade'] == "true"
+
+        tag_error = self.define_tags(course, data, course_content)
+        if tag_error is not None:
+            errors.append(tag_error)
+
 
         if len(errors) == 0:
             self.course_factory.update_course_descriptor_content(courseid, course_content)
@@ -103,3 +107,40 @@ def POST_AUTH(self, courseid):  # pylint: disable=arguments-differ
     def page(self, course, errors=None, saved=False):
         """ Get all data and display the page """
         return self.template_helper.render("course_admin/settings.html", course=course, errors=errors, saved=saved)
+
+    def define_tags(self, course, data, course_content):
+        tags = self.prepare_datas(data, "tags")
+        if type(tags) is not dict:
+            # prepare_datas returned an error
+            return tags
+
+        # Repair tags
+        for key, tag in tags.items():
+            # Since unchecked checkboxes are not present here, we manually add them to avoid later errors
+            tag["visible"] = "visible" in tag
+            tag["type"] = int(tag["type"])
+
+            if (tag["id"] == "" and tag["type"] != 2) or tag["name"] == "":
+                return _("Some tag fields were missing.")
+
+            if not id_checker(tag["id"]):
+                return _("Invalid tag id: {}").format(tag["id"])
+
+            del tag["id"]
+
+        course_content["tags"] = tags
+        self.course_factory.update_course_descriptor_content(course.get_id(), course_content)
+
+    def prepare_datas(self, data, prefix: str):
+        # prepare dict
+        datas = dict_from_prefix(prefix, data)
+        if datas is None:
+            datas = {}
+
+        items_id = [item["id"] for key, item in datas.items() if item["id"]]
+
+        if len(items_id) != len(set(items_id)):
+            return _("Some datas have the same id! The id must be unique.")
+
+        return {field["id"]: field for item, field in datas.items() if field["id"]}
+

inginious/frontend/pages/course_admin/utils.py

@@ -349,8 +349,7 @@ def get_menu(course, current, renderer, plugin_manager, user_manager):
     if user_manager.has_admin_rights_on_course(course):
         default_entries += [("tasks", "<i class='fa fa-tasks fa-fw'></i>&nbsp; " + _("Tasks"))]
 
-    default_entries += [("tags", "<i class='fa fa-tags fa-fw'></i>&nbsp;" + _("Tags")),
-                        ("submissions", "<i class='fa fa-file-code-o fa-fw'></i>&nbsp; " + _("Submissions"))]
+    default_entries += [("submissions", "<i class='fa fa-file-code-o fa-fw'></i>&nbsp; " + _("Submissions"))]
 
     if user_manager.has_admin_rights_on_course(course):
         default_entries += [("danger", "<i class='fa fa-bomb fa-fw'></i>&nbsp; " + _("Danger zone"))]

inginious/frontend/static/js/studio.js

@@ -634,17 +634,17 @@ function studio_get_feedback(sid)
  * Functions for tags edition. Use in tags.html
  */
 
-function studio_expand_tag_description(elem){
+function studio_expand_description(elem){
     elem.rows = 5;
 }
-function studio_expand_tag_description_not(elem){
+function studio_expand_description_not(elem){
     elem.rows = 1;
 }
 // Add a new line to the tag table
-function studio_add_tag_line(line) {
+function studio_add_table_line(line,target,target_blank_row) {
 
-    var new_row = $("#NEW").clone();
-    var new_id = 1 + parseInt($('#table tr:last').attr('id'));
+    var new_row = $("#"+target_blank_row).clone();
+    var new_id = 1 + parseInt($('#'+target+' tr:last').attr('id'));
     if (isNaN(new_id))
         new_id = 0
 
@@ -673,7 +673,7 @@ function studio_add_tag_line(line) {
     modified_row = modified_row.replace("type_replace_"+type, 'selected="selected"');
     modified_row = modified_row.replace("id_stop", "");
 
-    $('#table').find('tbody').append("<tr id="+new_id+">" + modified_row + "</tr>");
+    $('#'+target).find('tbody').append("<tr id="+new_id+">" + modified_row + "</tr>");
     new_row.show();
 }
 

inginious/frontend/templates/course_admin/settings.html

@@ -45,6 +45,9 @@ <h2>{{_("Course settings")}}</h2>
               <li class="nav-item" id="lti-nav-item">
                 <a class="nav-link" data-toggle="tab" href="#tab_lti">{{ _("LTI") }}</a>
               </li>
+              <li class="nav-item">
+                <a class="nav-link" data-toggle="tab" href="#tags_tab">{{ _("Tags") }}</a>
+              </li>
             </ul>
         </div>
 
@@ -364,9 +367,72 @@ <h2>{{_("Course settings")}}</h2>
                 </div>
             </div>
           </div>
+          <div class="tab-pane fade" id="tags_tab">
+            {% if error %}
+                <div class="alert alert-danger" role="alert">
+                    {{error}}
+                    <button type="button" class="close" data-dismiss="alert" aria-label="Close">
+                        <span aria-hidden="true">&times;</span>
+                    </button>
+                </div>
+            {% endif %}
+            <div class="btn-group btn-group-sm pull-right">
+                <button type="button" class="mt-3 btn btn-info btn-block" onclick="studio_add_table_line(-1,'table','NEW')"><i class="fa fa-plus fa-lg"></i> {{ _("New tag") }}</button>
+            </div>
+            <table id="table" class="table">
+                <thead class="table-borderless">
+                    <tr>
+                        <th>{{_("id")}}</th>
+                        <th>{{_("name")}}</th>
+                        <th>{{_("description")}}</th>
+                        <th>{{_("show to students")}}</th>
+                        <th>{{_("type ")}}</th>
+                    </tr>
+                </thead>
+                <tbody>
+
+                <!-- An empty row so that we can copy it to add new rows. All 'disabled' are important to not send this row to INGInious when saving the task-->
+                <tr id="NEW" style="display:none;">
+                    <td><input disabled type="text" ID_STOP id="id_NEW" class="form-control" name="tags[NEW][id]" value="ID_REPLACE"></td>
+                    <td><input disabled type="text" class="form-control" name="tags[NEW][name]" value="NAME_REPLACE"/></td>
+                    <td><textarea disabled class="form-control" onfocus="studio_expand_tag_description(this);" onblur="studio_expand_tag_description_not(this);" name="tags[NEW][description]" rows="1" style="resize:none">DESCRIPTION_REPLACE</textarea></td>
+                    <td><input disabled type="checkbox" class="form-control" name="tags[NEW][visible]" VISIBLE_REPLACE)/></td>
+                    <td>
+                        <select disabled  class="form-control" name="tags[NEW][type]">
+                            <option value="0" TYPE_REPLACE_0>{{_("Skill")}}</option>
+                            <option value="1" TYPE_REPLACE_1>{{_("Misconception")}}</option>
+                            <option value="2" TYPE_REPLACE_2>{{_("Category")}}</option>
+                        </select>
+                    </td>
+                </tr>
+
+                {% for key, tag in course.get_descriptor().get("tags", {}).items() %}
+                    {% set name = tag["name"] if "name" in tag else _("Unknown name") %}
+                    {% set description = tag["description"] if "description" in tag else "" %}
+                    {% set type = tag["type"] if "type" in tag else 0 %}
+                    {% set visible = tag["visible"] if "visible" in tag else False %}
+                    <tr id="{{loop.index}}">
+                        <td><input type="text" id="id_{{loop.index}}" class="form-control" name="tags[{{loop.index}}][id]" value="{{ key }}"/></td>
+                        <td><input type="text" class="form-control" name="tags[{{loop.index}}][name]" value="{{name}}"/></td>
+                        <td><textarea class="form-control" onfocus="studio_expand_description(this);" onblur="studio_expand_description_not(this);" name="tags[{{loop.index}}][description]" rows="1" style="resize:none">{{description}}</textarea></td>
+                        <td><input type="checkbox" class="form-control" name="tags[{{loop.index}}][visible]" {% if visible %} checked="checked" {% endif %}/></td>
+                        <td>
+                            <select class="form-control" name="tags[{{loop.index}}][type]">
+                                <option value="0" {% if type|int == 0 %} selected="selected" {% endif %}>{{_("Skill")}}</option>
+                                <option value="1" {% if type|int == 1 %} selected="selected" {% endif %}>{{_("Misconception")}}</option>
+                                <option value="2" {% if type|int == 2 %} selected="selected" {% endif %}>{{_("Category")}}</option>
+                            </select>
+                        </td>
+                    </tr>
+                {% endfor %}
+                </tbody>
+            </table>
+
+      </div>
         </div>
-        <button class="btn btn-primary btn-block" type="submit"><i class="fa fa-download"></i> {{ _("Save changes") }}</button>
     </div>
+    <button class="btn btn-primary btn-block mt-3" type="submit"><i class="fa fa-download"></i> {{ _("Save changes") }}</button>
+
 </form>
 
 <script type="text/javascript">