PR Part 3

.github/workflows/test.yml

@@ -0,0 +1,27 @@
+name: feedbackxblock
+
+on:
+  push:
+    branches: [master, main]
+  pull_request:
+
+jobs:
+  test:
+    runs-on: ubuntu-18.04
+    strategy:
+      matrix:
+        python-version: [3.5, 3.8]
+        tox-env:
+          - quality
+          - py
+    steps:
+     - uses: actions/checkout@v2
+     - name: Set up Python ${{ matrix.python-version }}
+       uses: actions/setup-python@v2
+       with:
+         python-version: ${{ matrix.python-version }}
+     - name: Install dependencies
+       run: |
+         pip install tox
+     - name: Test with tox
+       run: tox -e ${{ matrix.tox-env }}

.gitignore

@@ -1,3 +1,7 @@
 *pyc
 rate_xblock.egg-info
-*~
+*~
+.coverage
+.tox
+*.egg-info
+__pycache__

README.md

@@ -14,7 +14,9 @@ The goal of this XBlock is two-fold:
 Instructors have a good amount of control over the contents of
 the block:
 
-![Screenshot of the FeedbackXBlock](FeedbackXBlock.png)
+![Screenshot of the FeedbackXBlock -- good to bad scale](happy_sad_example.png)
+![Screenshot of the FeedbackXBlock -- scale where good is in the middle](happy_sad_happy_example.png)
+![Screenshot of the FeedbackXBlock -- numerical scale](numerical_example.png)
 
 This can be placed anywhere in the courseware, and students can
 provide feedback on those sections. With just a few database queries,
@@ -26,5 +28,5 @@ It installs on any Open edX install same as any other xblock:
 
     pip install -e git+https://github.com/pmitros/FeedbackXBlock.git#egg=rate==0.0
 
-From there, add 'feedback' to your list of advanced modules, and you're 
+From there, add "feedback" to your list of advanced modules, and you're 
 good to go. 

feedback/__init__.py

@@ -1 +1,5 @@
-from .feedback import FeedbackXBlock
+"""
+An edX XBlock designed to allow people to provide feedback on our
+course resources, and to think and synthesize about their experience
+in the course.
+"""

feedback/feedback.py

@@ -5,14 +5,15 @@
 in the course.
 """
 
-import cgi
+import html
 import random
-
 import pkg_resources
+import six
 
 from xblock.core import XBlock
 from xblock.fields import Scope, Integer, String, List, Float
-from xblock.fragment import Fragment
+from web_fragments.fragment import Fragment
+
 
 # We provide default text which is designed to elicit student thought. We'd
 # like instructors to customize this to something highly structured (not
@@ -23,14 +24,15 @@
                   "lessons learned, as well as key gaps in our presentation."
 DEFAULT_PLACEHOLDER = "Take a little bit of time to reflect here. " \
                       "Research shows that a meaningful synthesis will help " \
-                      "you better understand and remember material from this" \
+                      "you better understand and remember material from this " \
                       "course."
 DEFAULT_ICON = "face"
 DEFAULT_SCALETEXT = ["Excellent", "Good", "Average", "Fair", "Poor"]
 
-ICON_SETS = {'face': u"😁😊😐😞😭",
+# Unicode alt faces are cute, but we do nulls instead for a11y.
+ICON_SETS = {'face': [""]*5,  # u"😁😊😐😞😭",
              'num': u"12345",
-             'midface': u"😞😐😊😐😞"}
+             'midface': [""]*5}  # u"😞😐😊😐😞"}
 
 
 @XBlock.needs('i18n')
@@ -221,8 +223,7 @@ def get_url(icon_type, i):
                 votes,
                 act_urls,
                 ina_urls,
-                sel_urls
-               )
+                sel_urls)
         )
         if self.user_vote != -1:
             _ = self.runtime.service(self, 'i18n').ugettext
@@ -264,9 +265,9 @@ def studio_view(self, context):
         prompt = self.get_prompt(0)
         for idx in range(len(prompt['scale_text'])):
             prompt['likert{i}'.format(i=idx)] = prompt['scale_text'][idx]
-        frag = Fragment(unicode(html_str).format(**prompt))
+        frag = Fragment(six.text_type(html_str).format(**prompt))
         js_str = self.resource_string("static/js/src/studio.js")
-        frag.add_javascript(unicode(js_str))
+        frag.add_javascript(six.text_type(js_str))
         frag.initialize_js('FeedbackBlock',
                            {'icon_set': prompt['icon_set']})
         return frag
@@ -276,20 +277,15 @@ def studio_submit(self, data, suffix=''):
         """
         Called when submitting the form in Studio.
         """
-        print "Received: ", data
-        print "Old prompt: ", self.prompts[0]
         for item in ['freeform', 'likert', 'placeholder', 'icon_set']:
             item_submission = data.get(item, None)
             if item_submission and len(item_submission) > 0:
-                print "Setting", item
-                self.prompts[0][item] = cgi.escape(item_submission)
+                self.prompts[0][item] = html.escape(item_submission)
         for i in range(5):
             likert = data.get('likert{i}'.format(i=i), None)
             if likert and len(likert) > 0:
-                print "Setting", i
-                self.prompts[0]['scale_text'][i] = cgi.escape(likert)
+                self.prompts[0]['scale_text'][i] = html.escape(likert)
 
-        print "New prompt: ", self.prompts[0]
         return {'result': 'success'}
 
     def init_vote_aggregate(self):
@@ -309,7 +305,7 @@ def vote(self, data):
         """
         # prompt_choice is initialized by student view.
         # Ideally, we'd break this out into a function.
-        prompt = self.get_prompt(self.prompt_choice)
+        prompt = self.get_prompt(self.prompt_choice)  # noqa
 
         # Make sure we're initialized
         self.init_vote_aggregate()

feedback/static/html/feedback.html

@@ -9,7 +9,7 @@
       <label class="feedback_header" for="feedback_freeform_textarea">{freeform_prompt}</label>
     </div>
     <div class="feedback_freeform_input">
-      <textarea id="feedback_freeform_textarea" class="feedback_freeform_area" rows="6" cols="45" placeholder="{placeholder}">{self.user_freeform}</textarea>
+      <textarea name="feedback_freeform_textarea" class="feedback_freeform_area" rows="6" cols="45" placeholder="{placeholder}">{self.user_freeform}</textarea>
       <div class="feedback_thank_you" aria-live=polite>{response}</div>
     </div>
     <button type="submit" class="feedback_submit_feedback"> Submit Feedback </button>

feedback/static/html/scale_item.html

@@ -1,6 +1,6 @@
 <div class="feedback_likert_rating">
   <label title="{scale_text}" class="feedback_likert_label">
-    <input id="radio_{idx}" name="feedback_scale" class="feedback_radio" type="radio" {active}/>
+    <input data-id="radio_{idx}" name="feedback_scale" class="feedback_radio" type="radio" {active}/>
     <span class="feedback_icon feedback_icon_inactive">
       <img src="{ina_icon}" alt="{unicode_icon}"/>
     </span>

feedback/static/html/staff_item.html

@@ -1,6 +1,6 @@
 <div class="feedback_likert_rating">
   <label title="{scale_text}" class="feedback_likert_label">
-    <input id="radio_{idx}" name="feedback_scale" class="feedback_radio" type="radio" {active}/>
+    <input data-id="radio_{idx}" name="feedback_scale" class="feedback_radio" type="radio" {active}/>
     <span class="feedback_icon feedback_icon_inactive">
       <img src="{ina_icon}" alt="{unicode_icon}"/>
     </span>

feedback/static/html/studio_view.html

@@ -1,90 +1,102 @@
 <div class="wrapper-comp-settings is-active editor-with-buttons" id="settings-tab">
-  <p> This XBlock allows you to collect student feedback on pieces of
-    the course. This may be helpful either for course improvement, or
-    to give students a chance to reflect on what they have done.</p>
+  <ul class="list-input settings-list">
+    <li class="field comp-setting-entry is-set"> This XBlock allows
+      you to collect student feedback on pieces of the course. This
+      may be helpful either for course improvement, or to give
+      students a chance to reflect on what they have done.</li>
 
-  <p> Prior to using this block, we recommend reading about best
-    practices in the edX documentation. In particular, we do recommend
-    asking for structured, specific feedback, and we advise using this
-    in conjunction with
-    the <a href="http://edx.readthedocs.org/projects/edx-partner-course-staff/en/latest/content_experiments/content_experiments_configure.html">RCT
-    framework</a> for matrix sampling.</p>
+    <li class="field comp-setting-entry is-set"> Prior to using this
+      block, we recommend reading about best practices in the edX
+      documentation. In particular, we do recommend asking for
+      structured, specific feedback, and we advise using this in
+      conjunction with
+      the <a href="https://edx.readthedocs.io/projects/edx-partner-course-staff/en/latest/course_features/content_experiments/content_experiments_configure.html">RCT
+      framework</a> for matrix sampling.</li>
 
-  <ul class="list-input settings-list">
     <li class="field comp-setting-entry is-set">
       <div class="wrapper-comp-setting">
-        <label class="label setting-label" for="freeform">Freeform prompt</label>
-        <input class="input setting-input" name="freeform" id="freeform" value="{freeform}" type="text" />
+        <label class="label setting-label" aria-describedby="ff_p_div">Freeform prompt
+          <input class="input setting-input" name="freeform" id="freeform" value="{freeform}" type="text" />
+        </label>
       </div>
-      <span class="tip setting-help">Example: Please provide us feedback on this section.</span>
+      <span class="tip setting-help" id="ff_p_div">Example: Please provide us feedback on this section.</span>
     </li>
 
     <li class="field comp-setting-entry is-set">
       <div class="wrapper-comp-setting">
-        <label class="label setting-label" for="placeholder">Freeform placeholder</label>
-        <input class="input setting-input" name="placeholder" id="placeholder" value="{placeholder}" type="text" />
+        <label aria-describedby="placeholder_span" class="label setting-label>"Freeform placeholder
+          <input class="input setting-input" name="placeholder" id="placeholder" value="{placeholder}" type="text" />
+        </label>
       </div>
-      <span class="tip setting-help">This is shown as grayed out text before the student has answered.</span>
+      <span id="placeholder_span" class="tip setting-help">This is shown as grayed out text before the student has answered.</span>
     </li>
 
     <li class="field comp-setting-entry is-set">
       <div class="wrapper-comp-setting">
-        <label class="label setting-label" for="icon_set">Likert icon set</label>
+        <label aria-describedby="icon_set_option" class="label setting-label">Likert icon set
 	<select name="icon_set">
-	  <option value="face">Faces (happy-to-sad)</option>
-	  <option value="midface">Faces (sad-to-happy-to-sad)</option>
+	  <option value="face">Faces (happy to sad)</option>
+	  <option value="midface">Faces (sad to happy to sad)</option>
 	  <option value="num">Numeric</option></select>
+	</select>
+	</label>
       </div>
-      <span class="tip setting-help">We can either show happy/sad faces, or numbers 1-5.</span>
+      <span id="icon_set_option" class="tip setting-help">We can either show happy/sad faces, or numbers 1-5.</span>
     </li>
 
     <li class="field comp-setting-entry is-set">
       <div class="wrapper-comp-setting">
-        <label class="label setting-label" for="likert">Likert prompt</label>
-        <input class="input setting-input" name="likert" id="likert" value="{likert}" type="text" />
+        <label aria-describedby="lik_example" class="label setting-label">Likert prompt
+          <input class="input setting-input" name="likert" id="likert" value="{likert}" type="text" />
+        </label>
       </div>
-      <span class="tip setting-help">Example: Please rate your overall experience with this section.</span>
+      <span id="lik_example" class="tip setting-help">Example: Please rate your overall experience with this section.</span>
     </li>
 
     <li class="field comp-setting-entry is-set">
       <div class="wrapper-comp-setting">
-        <label class="label setting-label" for="likert0">Likert option 1</label>
-        <input class="input setting-input" name="likert0" id="likert0" value="{likert0}" type="text" />
+        <label aria-describedby="lik_excel" class="label setting-label">Likert option 1
+          <input class="input setting-input" name="likert0" id="likert0" value="{likert0}" type="text" />
+        </label>
       </div>
-      <span class="tip setting-help">Example: Excellent!</span>
+      <span id="lik_excel" class="tip setting-help">Example: Excellent!</span>
     </li>
 
     <li class="field comp-setting-entry is-set">
       <div class="wrapper-comp-setting">
-        <label class="label setting-label" for="likert1">Likert option 2</label>
-        <input class="input setting-input" name="likert1" id="likert1" value="{likert1}" type="text" />
+        <label aria-describedby="lik_good" class="label setting-label">Likert option 2
+          <input class="input setting-input" name="likert1" id="likert1" value="{likert1}" type="text" />
+        </label>
       </div>
-      <span class="tip setting-help">Example: Good</span>
+      <span id="lik_good" class="tip setting-help">Example: Good</span>
     </li>
 
     <li class="field comp-setting-entry is-set">
       <div class="wrapper-comp-setting">
-        <label class="label setting-label" for="likert2">Likert option 3</label>
-        <input class="input setting-input" name="likert2" id="likert2" value="{likert2}" type="text" />
+        <label aria-describedby="lik_ave" class="label setting-label">Likert option 3
+          <input class="input setting-input" name="likert2" id="likert2" value="{likert2}" type="text" />
       </div>
-      <span class="tip setting-help">Example: Average</span>
+      <span id="lik_ave" class="tip setting-help">Example: Average</span>
     </li>
 
     <li class="field comp-setting-entry is-set">
       <div class="wrapper-comp-setting">
-        <label class="label setting-label" for="likert3">Likert option 4</label>
-        <input class="input setting-input" name="likert3" id="likert3" value="{likert3}" type="text" />
+        <label aria-describedby="lik_fair" class="label setting-label">Likert option 4</label>
+          <input class="input setting-input" name="likert3" id="likert3" value="{likert3}" type="text" />
+        </label>
       </div>
-      <span class="tip setting-help">Example: Fair</span>
+      <span id="lik_fair" class="tip setting-help">Example: Fair</span>
     </li>
 
     <li class="field comp-setting-entry is-set">
       <div class="wrapper-comp-setting">
-        <label class="label setting-label" for="likert4">Likert option 5</label>
-        <input class="input setting-input" name="likert4" id="likert4" value="{likert4}" type="text" />
+        <label aria-describedby="lik_poor" class="label setting-label">Likert option 5</label>
+          <input class="input setting-input" name="likert4" id="likert4" value="{likert4}" type="text" />
+        </label>
       </div>
-      <span class="tip setting-help">Example: Poor</span>
+      <span id="lik_poor" class="tip setting-help">Example: Poor</span>
     </li>
+  </ul>
 
   <div class="xblock-actions">
     <ul>

feedback/static/js/src/feedback.js

@@ -14,7 +14,7 @@ function FeedbackXBlock(runtime, element) {
 	if ($(".feedback_radio:checked", element).length === 0) {
 	    vote = -1;
 	} else {
-	    vote = parseInt($(".feedback_radio:checked", element).attr("id").split("_")[1]);
+	    vote = parseInt($(".feedback_radio:checked", element).attr("data-id").split("_")[1]);
 	}
 	return vote;
     }

feedbacktests/__init__.py

@@ -1 +0,0 @@
-from .test_feedback import TestFeedback

feedbacktests/conftest.py

@@ -0,0 +1,27 @@
+import pytest
+from mock import Mock
+
+from workbench.runtime import WorkbenchRuntime
+from xblock.fields import ScopeIds
+from xblock.runtime import DictKeyValueStore, KvsFieldData
+
+from feedback.feedback import FeedbackXBlock
+
+
+def generate_scope_ids(runtime, block_type):
+    """ helper to generate scope IDs for an XBlock """
+    def_id = runtime.id_generator.create_definition(block_type)
+    usage_id = runtime.id_generator.create_usage(def_id)
+    return ScopeIds('user', block_type, def_id, usage_id)
+
+
+@pytest.fixture
+def feedback_xblock():
+    """Feedback XBlock pytest fixture."""
+    runtime = WorkbenchRuntime()
+    key_store = DictKeyValueStore()
+    db_model = KvsFieldData(key_store)
+    ids = generate_scope_ids(runtime, 'feedback')
+    feedback_xblock = FeedbackXBlock(runtime, db_model, scope_ids=ids)
+    feedback_xblock.usage_id = Mock()
+    return feedback_xblock