From 5a8b158900f8f05024839859c37f4ef270dae911 Mon Sep 17 00:00:00 2001
From: muhammad-ammar <mammar@gmail.com>
Date: Tue, 30 Jun 2015 16:11:31 +0500
Subject: [PATCH] Visual change to new Hints and Feedback problem types

TNL-2619
---
 .../component_settings_editor_helpers.py      |  6 +-
 .../contentstore/views/component.py           | 14 +++--
 cms/static/js/models/component_template.js    |  5 ++
 cms/static/sass/elements/_modules.scss        | 15 +++--
 ...d-xblock-component-menu-problem.underscore | 19 +-----
 .../problem/checkboxes_response_hint.yaml     |  2 +-
 .../problem/multiplechoice_hint.yaml          |  2 +-
 .../problem/numericalresponse_hint.yaml       |  2 +-
 .../problem/optionresponse_hint.yaml          |  2 +-
 .../problem/string_response_hint.yaml         |  2 +-
 .../test/acceptance/pages/studio/container.py | 30 ++++++++++
 .../tests/studio/test_studio_container.py     | 59 +++++++++++++++++++
 12 files changed, 121 insertions(+), 37 deletions(-)

diff --git a/cms/djangoapps/contentstore/features/component_settings_editor_helpers.py b/cms/djangoapps/contentstore/features/component_settings_editor_helpers.py
index f295a3517d..743f54f829 100644
--- a/cms/djangoapps/contentstore/features/component_settings_editor_helpers.py
+++ b/cms/djangoapps/contentstore/features/component_settings_editor_helpers.py
@@ -59,12 +59,12 @@ def click_new_component_button(step, component_button_css):
 
 
 def _click_advanced():
-    css = 'ul.problem-type-tabs a[href="#tab3"]'
+    css = 'ul.problem-type-tabs a[href="#tab2"]'
     world.css_click(css)
 
     # Wait for the advanced tab items to be displayed
-    tab3_css = 'div.ui-tabs-panel#tab3'
-    world.wait_for_visible(tab3_css)
+    tab2_css = 'div.ui-tabs-panel#tab2'
+    world.wait_for_visible(tab2_css)
 
 
 def _find_matching_link(category, component_type):
diff --git a/cms/djangoapps/contentstore/views/component.py b/cms/djangoapps/contentstore/views/component.py
index 391a62d6b7..994af7c1db 100644
--- a/cms/djangoapps/contentstore/views/component.py
+++ b/cms/djangoapps/contentstore/views/component.py
@@ -227,7 +227,7 @@ def get_component_templates(courselike, library=False):
     """
     Returns the applicable component templates that can be used by the specified course or library.
     """
-    def create_template_dict(name, cat, boilerplate_name=None, tab="common"):
+    def create_template_dict(name, cat, boilerplate_name=None, tab="common", hinted=False):
         """
         Creates a component template dict.
 
@@ -235,13 +235,15 @@ def get_component_templates(courselike, library=False):
             display_name: the user-visible name of the component
             category: the type of component (problem, html, etc.)
             boilerplate_name: name of boilerplate for filling in default values. May be None.
-            tab: common(default)/advanced/hint, which tab it goes in
+            hinted: True if hinted problem else False
+            tab: common(default)/advanced, which tab it goes in
 
         """
         return {
             "display_name": name,
             "category": cat,
             "boilerplate_name": boilerplate_name,
+            "hinted": hinted,
             "tab": tab
         }
 
@@ -277,20 +279,20 @@ def get_component_templates(courselike, library=False):
             for template in component_class.templates():
                 filter_templates = getattr(component_class, 'filter_templates', None)
                 if not filter_templates or filter_templates(template, courselike):
-                    # Tab can be 'common' 'advanced' 'hint'
+                    # Tab can be 'common' 'advanced'
                     # Default setting is common/advanced depending on the presence of markdown
                     tab = 'common'
                     if template['metadata'].get('markdown') is None:
                         tab = 'advanced'
-                    # Then the problem can override that with a tab: attribute (note: not nested in metadata)
-                    tab = template.get('tab', tab)
+                    hinted = template.get('hinted', False)
 
                     templates_for_category.append(
                         create_template_dict(
                             _(template['metadata'].get('display_name')),    # pylint: disable=translation-of-non-string
                             category,
                             template.get('template_id'),
-                            tab
+                            tab,
+                            hinted,
                         )
                     )
 
diff --git a/cms/static/js/models/component_template.js b/cms/static/js/models/component_template.js
index 050ef2e0b1..3b55f559dc 100644
--- a/cms/static/js/models/component_template.js
+++ b/cms/static/js/models/component_template.js
@@ -32,6 +32,11 @@ define(["backbone"], function (Backbone) {
                     return -1;
                 } else if (isPrimaryBlankTemplate(b)) {
                     return 1;
+                // Hinted problems should be shown at the end
+                } else if (a.hinted && !b.hinted) {
+                    return 1;
+                } else if (!a.hinted && b.hinted) {
+                    return -1;
                 } else if (a.display_name > b.display_name) {
                     return 1;
                 } else if (a.display_name < b.display_name) {
diff --git a/cms/static/sass/elements/_modules.scss b/cms/static/sass/elements/_modules.scss
index 889fbe8879..92ad1f86b6 100644
--- a/cms/static/sass/elements/_modules.scss
+++ b/cms/static/sass/elements/_modules.scss
@@ -124,11 +124,11 @@
       // green button
       .add-xblock-component-button {
         @extend %t-action3;
+        @include margin-right($baseline*0.75);
         position: relative;
         display: inline-block;
         width: ($baseline*5);
         height: ($baseline*5);
-        margin-right: ($baseline*0.75);
         margin-bottom: ($baseline/2);
         border: 1px solid $green-d2;
         border-radius: ($baseline/4);
@@ -164,7 +164,7 @@
 
     .cancel-button {
       @include white-button;
-      margin: $baseline 0 ($baseline/2) ($baseline/2);
+      @include margin($baseline, 0, ($baseline/2), ($baseline/2));
     }
 
     .problem-type-tabs {
@@ -225,13 +225,13 @@
     box-shadow: 0 1px 0 rgba(255, 255, 255, 0.2) inset, 0 -1px 0 $shadow inset;
 
     li:first-child {
-      margin-left: $baseline;
+      @include margin-left($baseline);
     }
 
     li {
       @include linear-gradient(top, rgba(255, 255, 255, .4), rgba(255, 255, 255, 0));
       opacity: 0.8;
-      float: left;
+      @include float(left);
       display: inline-block;
       width: auto;
       box-shadow: 0 1px 0 rgba(255, 255, 255, 0.2) inset, 0 -1px 0 $shadow inset;
@@ -248,6 +248,11 @@
         border: 0px;
         opacity: 1.0;
       }
+
+      // reset to remove jquery-ui float 
+      a.link-tab {
+        float: none;
+      }
     }
 
     a {
@@ -286,7 +291,7 @@ $outline-indent-width: $baseline;
       .icon {
         display: inline-block;
         vertical-align: middle;
-        margin-right: ($baseline/2);
+        @include margin-right($baseline/2);
       }
     }
   }
diff --git a/cms/templates/js/add-xblock-component-menu-problem.underscore b/cms/templates/js/add-xblock-component-menu-problem.underscore
index 301064935c..85bc58776b 100644
--- a/cms/templates/js/add-xblock-component-menu-problem.underscore
+++ b/cms/templates/js/add-xblock-component-menu-problem.underscore
@@ -4,10 +4,7 @@
             <a class="link-tab" href="#tab1"><%= gettext("Common Problem Types") %></a>
         </li>
         <li>
-            <a class="link-tab" href="#tab2"><%= gettext("Common Problems with Hints and Feedback") %></a>
-        </li>
-        <li>
-            <a class="link-tab" href="#tab3"><%= gettext("Advanced") %></a>
+            <a class="link-tab" href="#tab2"><%= gettext("Advanced") %></a>
         </li>
     </ul>
     <div class="tab current" id="tab1">
@@ -33,20 +30,6 @@
         </ul>
     </div>
     <div class="tab" id="tab2">
-        <ul class="new-component-template">
-            <% for (var i = 0; i < templates.length; i++) { %>
-                <% if (templates[i].tab == "hint") { %>
-                <li class="editor-manual">
-                    <a href="#" data-category="<%= templates[i].category %>"
-                       data-boilerplate="<%= templates[i].boilerplate_name %>">
-                        <span class="name"><%= templates[i].display_name %></span>
-                    </a>
-                </li>
-                <% } %>
-            <% } %>
-        </ul>
-    </div>
-    <div class="tab" id="tab3">
         <ul class="new-component-template">
             <% for (var i = 0; i < templates.length; i++) { %>
                 <% if (templates[i].tab == "advanced") { %>
diff --git a/common/lib/xmodule/xmodule/templates/problem/checkboxes_response_hint.yaml b/common/lib/xmodule/xmodule/templates/problem/checkboxes_response_hint.yaml
index b0882e6b3b..7d1f78c006 100644
--- a/common/lib/xmodule/xmodule/templates/problem/checkboxes_response_hint.yaml
+++ b/common/lib/xmodule/xmodule/templates/problem/checkboxes_response_hint.yaml
@@ -27,7 +27,7 @@ metadata:
       ||A fruit is the fertilized ovary from a flower.||
       ||A fruit contains seeds of the plant.||
 
-tab: hint
+hinted: true
 data: |
       <problem>
       
diff --git a/common/lib/xmodule/xmodule/templates/problem/multiplechoice_hint.yaml b/common/lib/xmodule/xmodule/templates/problem/multiplechoice_hint.yaml
index acace4cc9f..32c0fbb1c7 100644
--- a/common/lib/xmodule/xmodule/templates/problem/multiplechoice_hint.yaml
+++ b/common/lib/xmodule/xmodule/templates/problem/multiplechoice_hint.yaml
@@ -20,7 +20,7 @@ metadata:
        ||A fruit is the fertilized ovary from a flower.||
        ||A fruit contains seeds of the plant.||
 
-tab: hint
+hinted: true
 data: |
    <problem>
      
diff --git a/common/lib/xmodule/xmodule/templates/problem/numericalresponse_hint.yaml b/common/lib/xmodule/xmodule/templates/problem/numericalresponse_hint.yaml
index 0f00b7760b..b87732922c 100644
--- a/common/lib/xmodule/xmodule/templates/problem/numericalresponse_hint.yaml
+++ b/common/lib/xmodule/xmodule/templates/problem/numericalresponse_hint.yaml
@@ -24,7 +24,7 @@ metadata:
         The mean is calculated by summing the set of numbers and dividing by n. In this case: (1 + 5 + 6 + 3 + 5) / 5 = 20 / 5 = 4.
         [explanation]
 
-tab: hint
+hinted: true
 data: |
     <problem>
     
diff --git a/common/lib/xmodule/xmodule/templates/problem/optionresponse_hint.yaml b/common/lib/xmodule/xmodule/templates/problem/optionresponse_hint.yaml
index c7674648f6..139745f69f 100644
--- a/common/lib/xmodule/xmodule/templates/problem/optionresponse_hint.yaml
+++ b/common/lib/xmodule/xmodule/templates/problem/optionresponse_hint.yaml
@@ -23,7 +23,7 @@ metadata:
         ||A fruit is the fertilized ovary from a flower.||
         ||A fruit contains seeds of the plant.||
 
-tab: hint
+hinted: true
 data: |
     <problem>
 
diff --git a/common/lib/xmodule/xmodule/templates/problem/string_response_hint.yaml b/common/lib/xmodule/xmodule/templates/problem/string_response_hint.yaml
index bcd6bf6813..bbc6d5006f 100644
--- a/common/lib/xmodule/xmodule/templates/problem/string_response_hint.yaml
+++ b/common/lib/xmodule/xmodule/templates/problem/string_response_hint.yaml
@@ -23,7 +23,7 @@ metadata:
         ||Consider the square miles, not population.||
         ||Consider all 50 states, not just the continental United States.||
 
-tab: hint
+hinted: true
 data: |
   <problem>
 
diff --git a/common/test/acceptance/pages/studio/container.py b/common/test/acceptance/pages/studio/container.py
index a33b0a2d19..2aa5ac2310 100644
--- a/common/test/acceptance/pages/studio/container.py
+++ b/common/test/acceptance/pages/studio/container.py
@@ -281,6 +281,36 @@ class ContainerPage(PageObject):
         """
         return "is-editing" in self.q(css=self.NAME_FIELD_WRAPPER_SELECTOR).first.attrs("class")[0]
 
+    def get_category_tab_names(self, category_type):
+        """
+        Returns list of tab name in a category.
+
+        Arguments:
+            category_type (str): category type
+
+        Returns:
+            list
+        """
+        self.q(css='.add-xblock-component-button[data-type={}]'.format(category_type)).first.click()
+        return self.q(css='.{}-type-tabs>li>a'.format(category_type)).text
+
+    def get_category_tab_components(self, category_type, tab_index):
+        """
+        Return list of component names in a tab in a category.
+
+        Arguments:
+            category_type (str): category type
+            tab_index (int): tab index in a category
+
+        Returns:
+            list
+        """
+        css = '#tab{tab_index} a[data-category={category_type}] span'.format(
+            tab_index=tab_index,
+            category_type=category_type
+        )
+        return self.q(css=css).html
+
 
 class XBlockWrapper(PageObject):
     """
diff --git a/common/test/acceptance/tests/studio/test_studio_container.py b/common/test/acceptance/tests/studio/test_studio_container.py
index 95766c5b9c..2eebe98730 100644
--- a/common/test/acceptance/tests/studio/test_studio_container.py
+++ b/common/test/acceptance/tests/studio/test_studio_container.py
@@ -8,6 +8,7 @@ from unittest import skip
 
 from ...fixtures.course import XBlockFixtureDesc
 from ...pages.studio.component_editor import ComponentEditorView, ComponentVisibilityEditorView
+from ...pages.studio.container import ContainerPage
 from ...pages.studio.html_component_editor import HtmlComponentEditorView
 from ...pages.studio.utils import add_discussion, drag
 from ...pages.lms.courseware import CoursewarePage
@@ -1056,3 +1057,61 @@ class DisplayNameTest(ContainerBase):
         title_on_unit_page = test_block.name
         container = test_block.go_to_container()
         self.assertEqual(container.name, title_on_unit_page)
+
+
+class ProblemCategoryTabsTest(ContainerBase):
+    """
+    Test to verify tabs in problem category.
+    """
+    def setUp(self, is_staff=True):
+        super(ProblemCategoryTabsTest, self).setUp(is_staff=is_staff)
+
+    def populate_course_fixture(self, course_fixture):
+        """
+        Sets up course structure.
+        """
+        course_fixture.add_children(
+            XBlockFixtureDesc('chapter', 'Test Section').add_children(
+                XBlockFixtureDesc('sequential', 'Test Subsection').add_children(
+                    XBlockFixtureDesc('vertical', 'Test Unit')
+                )
+            )
+        )
+
+    def test_correct_tabs_present(self):
+        """
+        Scenario: Verify that correct tabs are present in problem category.
+
+        Given I am a staff user
+        When I go to unit page
+        Then I only see `Common Problem Types` and `Advanced` tabs in `problem` category
+        """
+        self.go_to_unit_page()
+        page = ContainerPage(self.browser, None)
+        self.assertEqual(page.get_category_tab_names('problem'), ['Common Problem Types', 'Advanced'])
+
+    def test_common_problem_types_tab(self):
+        """
+        Scenario: Verify that correct components are present in Common Problem Types tab.
+
+        Given I am a staff user
+        When I go to unit page
+        Then I see correct components under `Common Problem Types` tab in `problem` category
+        """
+        self.go_to_unit_page()
+        page = ContainerPage(self.browser, None)
+
+        expected_components = [
+            "Blank Common Problem",
+            "Checkboxes",
+            "Dropdown",
+            "Multiple Choice",
+            "Numerical Input",
+            "Text Input",
+            "Checkboxes with Hints and Feedback",
+            "Dropdown with Hints and Feedback",
+            "Multiple Choice with Hints and Feedback",
+            "Numerical Input with Hints and Feedback",
+            "Text Input with Hints and Feedback",
+        ]
+        self.assertEqual(page.get_category_tab_components('problem', 1), expected_components)