CSL-2041: Improve can_show()

Clean up a few other small issues in tour.

Author: bgoldowsky

src/frontend/js/tour.js

@@ -59,13 +59,13 @@
                 $next[0].focus();
                 var name = $next.attr('id').replace('tour_', '');
                 window.parent.clusiveEvents.addTipViewToQueue(name);
-            }
+            };
 
             // Hide tip/tour tooltip if showing
             $('#tip').CFW_Tooltip('hide');
 
             if ($curr.length) {
-                // Wait until hide animation is complete before callling show
+                // Wait until hide animation is complete before calling show
                 $curr.CFW_Popover('hide').CFW_transition(null, function() {
                     document.body.classList.add(CLASS_TOUR);
                     $trigger.one('afterShow.cfw.popover', showComplete);
@@ -102,4 +102,4 @@
     };
 
     window.tour = tourModule.prototype;
-}(jQuery));
+}(jQuery));

src/pages/templates/pages/partial/dashboard_panel_affect.html

@@ -1,4 +1,4 @@
-<div class="box" {% if clusive_user.role == 'ST' %} data-clusive-tip-id="student_reactions" {% endif %}>
+<div class="box" data-clusive-tip-id="student_reactions">
     <h2>Your thoughts <span class="icon-thought" aria-hidden="true"></span></h2>
     <div class="box-divider"></div>
     {% if data.empty %}

src/roster/models.py

@@ -580,6 +580,9 @@ class UserStats (models.Model):
     # Total active time using Clusive
     active_duration = models.DurationField(null=True)
 
+    def __str__(self):
+        return '<Stats for %s>' % (self.user)
+
     class Meta:
         verbose_name = 'user stats'
         verbose_name_plural = 'user stats'

src/shared/templates/shared/partial/popover_tour.html

@@ -132,7 +132,7 @@
             </div>
             <div class="popover-body">
                 <div class="embed-fluid embed-fluid-16x9 mb-0_5">
-                    <iframe id="studentReactionsIframe" src="https://videos.sproutvideo.com/embed/119ed9b21f1aeac598/961f727539a6dc9b?type=hd" frameborder="0" allow="accelerometer; autoplay; clipboard-write; encrypted-media; gyroscope; picture-in-picture" allowfullscreen aria-label="How to find and use Your students reactions video"></iframe>
+                    <iframe id="studentReactionsIframe0" src="https://videos.sproutvideo.com/embed/119ed9b21f1aeac598/961f727539a6dc9b?type=hd" frameborder="0" allow="accelerometer; autoplay; clipboard-write; encrypted-media; gyroscope; picture-in-picture" allowfullscreen aria-label="How to find and use Your students reactions video"></iframe>
                 </div>
                 <p>Click on a reaction word and see a list of Clusive readings that have inspired your students, and other Clusive readers, to react with that word.</p>
                 {% include "shared/partial/popover_tour_footer.html" with resource_id="SREACT" resource_name="your students&apos; reactions" %}
@@ -144,7 +144,7 @@
             </div>
             <div class="popover-body">
                 <div class="embed-fluid embed-fluid-16x9 mb-0_5">
-                    <iframe id="studentReactionsIframe" src="https://videos.sproutvideo.com/embed/449ed4bf1c1ee2cbcd/7eeb1eb5c62a2389?type=hd" frameborder="0" allow="accelerometer; autoplay; clipboard-write; encrypted-media; gyroscope; picture-in-picture" allowfullscreen aria-label="How to find and use Your thoughts video"></iframe>
+                    <iframe id="studentReactionsIframe1" src="https://videos.sproutvideo.com/embed/449ed4bf1c1ee2cbcd/7eeb1eb5c62a2389?type=hd" frameborder="0" allow="accelerometer; autoplay; clipboard-write; encrypted-media; gyroscope; picture-in-picture" allowfullscreen aria-label="How to find and use Your thoughts video"></iframe>
                 </div>
                 <p>Click on a reaction word and see Clusive readings that inspired you and others to react with that word.</p>
                 {% include "shared/partial/popover_tour_footer.html" with resource_id=None %}
@@ -160,7 +160,7 @@
             </div>
             <div class="popover-body">
                 <div class="embed-fluid embed-fluid-16x9 mb-0_5">
-                    <iframe id="readingDataIframe" src="https://videos.sproutvideo.com/embed/449ed9b61c1aecc5cd/5993d5016938853d?type=hd" frameborder="0" allow="accelerometer; autoplay; clipboard-write; encrypted-media; gyroscope; picture-in-picture" allowfullscreen aria-label="How to find and use Student Reading data video"></iframe>
+                    <iframe id="readingDataIframe0" src="https://videos.sproutvideo.com/embed/449ed9b61c1aecc5cd/5993d5016938853d?type=hd" frameborder="0" allow="accelerometer; autoplay; clipboard-write; encrypted-media; gyroscope; picture-in-picture" allowfullscreen aria-label="How to find and use Student Reading data video"></iframe>
                 </div>
                 <p>Get a quick view of student reading interests and learning. Check out reading data on assigned and popular readings. Get a summary of student self-rating of learning, and see answers to prompts you have customized.</p>
                 {% include "shared/partial/popover_tour_footer.html" with resource_id="SREADP" resource_name="student reading data" %}
@@ -172,7 +172,7 @@
             </div>
             <div class="popover-body">
                 <div class="embed-fluid embed-fluid-16x9 mb-0_5">
-                    <iframe id="readingDataIframe" src="https://videos.sproutvideo.com/embed/ea9ed4b41b15e2c263/bd13498e66ded346?type=hd" frameborder="0" allow="accelerometer; autoplay; clipboard-write; encrypted-media; gyroscope; picture-in-picture" allowfullscreen aria-label="Your Reading video"></iframe>
+                    <iframe id="readingDataIframe1" src="https://videos.sproutvideo.com/embed/ea9ed4b41b15e2c263/bd13498e66ded346?type=hd" frameborder="0" allow="accelerometer; autoplay; clipboard-write; encrypted-media; gyroscope; picture-in-picture" allowfullscreen aria-label="Your Reading video"></iframe>
                 </div>
                 <p>See assigned readings your recent readings, and popular readings to check out. Go to the library to find more things to read. Have a document you want to upload? Click Upload a new reading to get started!</p>
                 {% include "shared/partial/popover_tour_footer.html" with resource_id=None %}

src/tips/models.py

@@ -35,6 +35,13 @@
     'wordbank',
 ]
 
+RES_READING_TIPS = [
+    'settings',
+    'readaloud',
+    'context',
+    'wordbank',
+]
+
 LIBRARY_TIPS = [
     'view',
     'filters',
@@ -64,7 +71,7 @@
 PAGE_TIPS_MAP = {
     'Dashboard': DASHBOARD_TIPS,
     'Reading': READING_TIPS,
-    'ResourceReading': READING_TIPS,
+    'ResourceReading': RES_READING_TIPS,
     'Library': LIBRARY_TIPS,
     'Wordbank': WORD_BANK_TIPS,
     'Manage': MANAGE_TIPS,
@@ -86,29 +93,28 @@ class TipType(models.Model):
     max = models.PositiveSmallIntegerField(verbose_name='Maximum times to show')
     interval = models.DurationField(verbose_name='Interval between shows')
 
-    def can_show(self, page: str, version_count: int, user: ClusiveUser):
+    def can_show(self, page: str, version_count: int, user: ClusiveUser, stats: UserStats):
         """Test whether this tip can be shown on a particular page"""
-        is_student_or_guest = user.role == Roles.STUDENT or user.role == Roles.GUEST
-        # Teacher/parent-only tips
-        if (self.name in TEACHER_ONLY_TIPS or page == 'ResourceReading') and is_student_or_guest:
+        is_student_or_guest = not user.can_manage_periods
+
+        # Check for proper role
+        if self.name in TEACHER_ONLY_TIPS and is_student_or_guest:
+            return False
+
+        # Check for proper page
+        if not self.name in PAGE_TIPS_MAP.get(page, []):
             return False
+
         # Switch TipType requires multiple versions
-        if self.name == 'switch':
-            return (page == 'Reading' or page == 'ResourceReading') and version_count > 1
-        # Thoughts TipType is only for students or guests
-        if self.name == 'thoughts' and not is_student_or_guest:
+        if self.name == 'switch' and version_count == 1:
             return False
 
-        # 'wordbank', 'manage', and 'reources' TipTypes appear on multiple pages.
-        # Check first whether the `page` parameter is 'WordBank', 'Manage', or
-        # 'Resources'.
-        if page in PAGES_WITH_OWN_TIP and self.name in PAGE_TIPS_MAP[page]:
-            return True
-        # Most tooltips need to check if on correct page
-        if self.name in PAGE_TIPS_MAP.get(page, []):
-            return True
-        # Unknown tip never shown
-        return False
+        # student_reactions TipType, for students, requires that they have read at least one book
+        if self.name == 'student_reactions' and is_student_or_guest and stats and stats.reading_views == 0:
+            return False
+
+        # Passed all tests
+        return True
 
     def __str__(self):
         return '<TipType %s>' % self.name
@@ -195,29 +201,33 @@ def register_action(cls, user: ClusiveUser, action: str, timestamp):
     def available_tips(cls, user: ClusiveUser, page: str, version_count: int):
         """Return all tips that are currently available to show this user."""
 
+        stats: UserStats
+        stats = UserStats.for_clusive_user(user)
+
         # All tips are currently disallowed on the user's FIRST reading page view
         if page == 'Reading' or page == 'ResourceReading':
-            stats: UserStats
-            stats = UserStats.for_clusive_user(user)
             if stats.reading_views < 1:
                 return []
 
         # Check tip history to see which are ready to be shown
         histories = TipHistory.objects.filter(user=user).order_by('type__priority')
+
         return [h for h in histories
-                if h.type.can_show(page=page, version_count=version_count, user=user)
+                if h.type.can_show(page=page, version_count=version_count, user=user, stats=stats)
                 and h.ready_to_show()]
 
     @classmethod
     def tour_list(cls, user: ClusiveUser, page: str, version_count: int = 0):
         """Return names of all tips that should make up the tour for the given user and page."""
         histories = TipHistory.objects.filter(user=user).order_by('type__tour_position')
-        can_show = [h.type.name for h in histories if h.type.can_show(page=page, version_count=version_count, user=user)]
+        stats = UserStats.for_clusive_user(user)
+        can_show = [h.type.name for h in histories if
+                    h.type.can_show(page=page, version_count=version_count, user=user, stats=stats)]
         # Showing the 'tour' tip during a tour would be weirdly recursive.
         if 'tour' in can_show:
             can_show.remove('tour')
         logger.debug('Tour list: %s', can_show)
-        return can_show if len(can_show) > 1 else None
+        return can_show if len(can_show) > 0 else None
 
     @classmethod
     def get_tip_to_show(cls, clusive_user: ClusiveUser, page: str, version_count=0):

src/tips/tests.py

@@ -1,16 +1,12 @@
 import logging
-from datetime import datetime, timedelta
+from datetime import timedelta
 
 from django.contrib.auth.models import User
-from django.core.exceptions import ValidationError
 from django.test import TestCase
 from django.utils import timezone
 
 from roster.models import ClusiveUser, Roles
-
-from .models import TipType, TEACHER_ONLY_TIPS, DASHBOARD_TIPS, READING_TIPS, \
-    LIBRARY_TIPS, WORD_BANK_TIPS, MANAGE_TIPS, RESOURCES_TIPS, \
-    PAGES_WITH_OWN_TIP, PAGE_TIPS_MAP, TipHistory
+from .models import TipType, TipHistory
 
 logger = logging.getLogger(__name__)
 
@@ -52,16 +48,18 @@
     'Reading': ['switch', 'settings', 'readaloud', 'context', 'thoughts', 'wordbank'],
     'Library': ['view', 'filters', 'search'],
     'Wordbank': ['wordbank'],
+    # can_show() is true, but doesn't really matter since student shouldn't ever end up on the ResourceReading page:
+    'ResourceReading': ['settings', 'readaloud', 'context', 'wordbank'],
 }
 
 TEACHER_OR_PARENT_CAN_SHOW = {
     'Dashboard': ['student_reactions', 'reading_data', 'activity', 'manage'],
-    'Reading': ['switch', 'settings', 'readaloud', 'context', 'wordbank'],
+    'Reading': ['switch', 'settings', 'readaloud', 'context', 'thoughts', 'wordbank'],
     'Library': ['view', 'filters', 'search', 'book_actions'],
     'Wordbank': ['wordbank'],
     'Resources': ['resources'],
     'Manage': ['manage'],
-    'ResourceReading': ['switch', 'settings', 'readaloud', 'context', 'wordbank'],
+    'ResourceReading': ['settings', 'readaloud', 'context', 'wordbank'],
 }
 
 START_DELTA = 250 # msec
@@ -144,7 +142,7 @@ def can_show_tips(self, clusive_user):
         version_count = 3
         for page_name in PAGE_NAMES:
             for tip in TipType.objects.all():
-                actual = tip.can_show(page_name, version_count, clusive_user)
+                actual = tip.can_show(page_name, version_count, clusive_user, None)
                 expected = self.look_up_expected(clusive_user, page_name, tip)
                 self.assertEqual(
                     actual, expected,
@@ -166,7 +164,7 @@ def test_can_show_switch_single_version(self):
         """
         for clusive_user in ClusiveUser.objects.all():
             switch_tip = TipType.objects.get(name='switch')
-            actual = switch_tip.can_show('Reading', 1, clusive_user)
+            actual = switch_tip.can_show('Reading', 1, clusive_user, None)
             self.assertFalse(
                 actual,
                 f"Can show 'switch' tip for {clusive_user.user.username} on Reading page"