Hide course versions

apps/src/code-studio/components/progress/ScriptOverview.jsx

@@ -15,7 +15,7 @@ import ProgressTable from '@cdo/apps/templates/progress/ProgressTable';
 import ProgressLegend from '@cdo/apps/templates/progress/ProgressLegend';
 import { resourceShape } from '@cdo/apps/templates/courseOverview/resourceType';
 import { hasLockableStages } from '@cdo/apps/code-studio/progressRedux';
-import ScriptOverviewHeader from './ScriptOverviewHeader';
+import ScriptOverviewHeader, { scriptVersionShape } from './ScriptOverviewHeader';
 import { isScriptHiddenForSection } from '@cdo/apps/code-studio/hiddenStageRedux';
 import { onDismissRedirectDialog, dismissedRedirectDialog } from '@cdo/apps/util/dismissVersionRedirect';
 
@@ -31,11 +31,7 @@ class ScriptOverview extends React.Component {
     showScriptVersionWarning: PropTypes.bool,
     redirectScriptUrl: PropTypes.string,
     showRedirectWarning: PropTypes.bool,
-    versions: PropTypes.arrayOf(PropTypes.shape({
-      name: PropTypes.string.isRequired,
-      version_year: PropTypes.string.isRequired,
-      version_title: PropTypes.string.isRequired,
-    })).isRequired,
+    versions: PropTypes.arrayOf(scriptVersionShape).isRequired,
     courseName: PropTypes.string,
 
     // redux provided

apps/src/code-studio/components/progress/ScriptOverviewHeader.jsx

@@ -43,6 +43,13 @@ const styles = {
   },
 };
 
+export const scriptVersionShape = PropTypes.shape({
+  name: PropTypes.string.isRequired,
+  version_year: PropTypes.string.isRequired,
+  version_title: PropTypes.string.isRequired,
+  can_view_version: PropTypes.bool.isRequired,
+});
+
 /**
  * This component takes some of the HAML generated content on the script overview
  * page, and moves it under our React root. This is done so that we can have React
@@ -70,11 +77,7 @@ class ScriptOverviewHeader extends Component {
     showCourseUnitVersionWarning: PropTypes.bool,
     showScriptVersionWarning: PropTypes.bool,
     showRedirectWarning: PropTypes.bool,
-    versions: PropTypes.arrayOf(PropTypes.shape({
-      name: PropTypes.string.isRequired,
-      version_year: PropTypes.string.isRequired,
-      version_title: PropTypes.string.isRequired,
-    })).isRequired,
+    versions: PropTypes.arrayOf(scriptVersionShape).isRequired,
     showHiddenUnitWarning: PropTypes.bool,
     courseName: PropTypes.string,
   };
@@ -162,6 +165,9 @@ class ScriptOverviewHeader extends Component {
       versionWarningDetails = i18n.wrongCourseVersionWarningDetails();
     }
 
+    // Only display viewable versions in script version dropdown.
+    const filteredVersions = versions.filter(version => version.can_view_version);
+
     return (
       <div>
         {plcHeaderProps &&
@@ -217,7 +223,7 @@ class ScriptOverviewHeader extends Component {
                 <span className="betatext">{betaTitle}</span>
                 }
               </h1>
-              {versions.length > 1 &&
+              {filteredVersions.length > 1 &&
                 <span style={styles.versionWrapper}>
                   <span style={styles.versionLabel}>{i18n.courseOverviewVersionLabel()}</span>&nbsp;
                   <select
@@ -226,7 +232,7 @@ class ScriptOverviewHeader extends Component {
                     style={styles.versionDropdown}
                     id="version-selector"
                   >
-                    {versions.map(version => (
+                    {filteredVersions.map(version => (
                       <option key={version.name} value={version.name}>
                         {version.version_year}
                       </option>

apps/src/templates/courseOverview/CourseOverview.js

@@ -69,7 +69,9 @@ export default class CourseOverview extends Component {
     hasVerifiedResources: PropTypes.bool.isRequired,
     versions: PropTypes.arrayOf(PropTypes.shape({
       name: PropTypes.string.isRequired,
-      version_year: PropTypes.string.isRequired
+      version_year: PropTypes.string.isRequired,
+      version_title: PropTypes.string.isRequired,
+      can_view_version: PropTypes.bool.isRequired,
     })).isRequired,
     showVersionWarning: PropTypes.bool,
     showRedirectWarning: PropTypes.bool,
@@ -150,6 +152,9 @@ export default class CourseOverview extends Component {
     const showNotification = viewAs === ViewType.Teacher && isTeacher &&
       !isVerifiedTeacher && hasVerifiedResources;
 
+    // Only display viewable versions in course version dropdown.
+    const filteredVersions = versions.filter(version => version.can_view_version);
+
     return (
       <div style={mainStyle}>
         {redirectToCourseUrl && !dismissedRedirectDialog(name) &&
@@ -181,7 +186,7 @@ export default class CourseOverview extends Component {
         }
         <div style={styles.titleWrapper}>
           <h1 style={styles.title}>{assignmentFamilyTitle}</h1>
-          {versions.length > 1 &&
+          {filteredVersions.length > 1 &&
             <span style={styles.versionWrapper}>
               <span style={styles.versionLabel}>{i18n.courseOverviewVersionLabel()}</span>&nbsp;
               <select
@@ -190,7 +195,7 @@ export default class CourseOverview extends Component {
                 style={styles.versionDropdown}
                 id="version-selector"
               >
-                {versions.map(version => (
+                {filteredVersions.map(version => (
                   <option key={version.name} value={version.name}>
                     {version.version_title}
                   </option>

apps/test/unit/templates/courseOverview/CourseOverviewTest.js

@@ -121,10 +121,10 @@ describe('CourseOverview', () => {
       utils.navigateToHref.restore();
     });
 
-    it('appears when two versions are present', () => {
+    it('appears when two versions are present and viewable', () => {
       const versions = [
-        {name: 'csp', version_year: '2017'},
-        {name: 'csp-2018', version_year: '2018'},
+        {name: 'csp', version_year: '2017', version_title: 'csp 2017', can_view_version: true},
+        {name: 'csp-2018', version_year: '2018', version_title: 'csp 2018', can_view_version: true},
       ];
       const wrapper = shallow(
         <CourseOverview
@@ -143,9 +143,10 @@ describe('CourseOverview', () => {
       expect(utils.navigateToHref).to.have.been.calledOnce;
     });
 
-    it('does not appear when only one version is present', () => {
+    it('does not appear when only one version is viewable', () => {
       const versions = [
-        {name: 'csp', version_year: '2017'},
+        {name: 'csp', version_year: '2017', version_title: 'csp 2017', can_view_version: false},
+        {name: 'csp-2018', version_year: '2018', version_title: 'csp 2018', can_view_version: true},
       ];
       const wrapper = shallow(
         <CourseOverview

dashboard/app/models/course.rb

@@ -257,7 +257,7 @@ def summarize(user = nil)
       end,
       teacher_resources: teacher_resources,
       has_verified_resources: has_verified_resources?,
-      versions: summarize_versions
+      versions: summarize_versions(user)
     }
   end
 
@@ -276,11 +276,11 @@ def summarize_short
 
   # Returns an array of objects showing the name and version year for all courses
   # sharing the family_name of this course, including this one.
-  def summarize_versions
+  def summarize_versions(user = nil)
     return [] unless family_name
     Course.
       where("properties -> '$.family_name' = ?", family_name).
-      map {|c| {name: c.name, version_year: c.version_year, version_title: c.localized_version_title}}.
+      map {|c| {name: c.name, version_year: c.version_year, version_title: c.localized_version_title, can_view_version: c.can_view_version?(user)}}.
       sort_by {|info| info[:version_year]}.
       reverse
   end
@@ -368,15 +368,19 @@ def redirect_to_course_url(user)
 
   # @param user [User]
   # @return [Boolean] Whether the user can view the course.
-  def can_view_version?(user)
-    return nil unless user
-    # Restrictions only apply to students.
+  def can_view_version?(user = nil)
+    latest_course_version = Course.latest_version(family_name)
+    is_latest = latest_course_version == self
+
+    # All users can see the latest course version.
+    return true if is_latest
+
+    # Restrictions only apply to students and logged out users.
+    return false if user.nil?
     return true unless user.student?
 
-    # A student can view the course version if...
-    # it is the latest, they are assigned to it, or they have progress in it.
-    latest_course_version = Course.latest_version(family_name)
-    latest_course_version == self || user.section_courses.include?(self) || has_progress?(user)
+    # A student can view the course version if they are assigned to it or they have progress in it.
+    user.section_courses.include?(self) || has_progress?(user)
   end
 
   # @param family_name [String] The family name for a course family.

dashboard/app/models/script.rb

@@ -454,19 +454,20 @@ def link
   # @param locale [String] User or request locale. Optional.
   # @return [Boolean] Whether the user can view the script.
   def can_view_version?(user, locale: nil)
-    return nil unless user
-    # Restrictions only apply to students.
-    return true unless user.student?
+    latest_stable_version = Script.latest_stable_version(family_name)
+    latest_stable_version_in_locale = Script.latest_stable_version(family_name, locale: locale)
+    is_latest = latest_stable_version == self || latest_stable_version_in_locale == self
 
-    # A student can view the script version if...
-    # 1) it is the latest stable version in English, 2) it is the latest stable version in their locale,
-    # 3) they are assigned to it, 4) or they have progress in it.
-    return true if Script.latest_stable_version(family_name) == self
+    # All users can see the latest script version in English and in their locale.
+    return true if is_latest
 
-    latest_stable_version_in_locale = Script.latest_stable_version(family_name, locale: locale)
-    has_progress = user.scripts.include?(self)
+    # Restrictions only apply to students and logged out users.
+    return false if user.nil?
+    return true unless user.student?
 
-    latest_stable_version_in_locale == self || user.assigned_script?(self) || has_progress
+    # A student can view the script version if they are assigned to it or they have progress in it.
+    has_progress = user.scripts.include?(self)
+    user.assigned_script?(self) || has_progress
   end
 
   # @param family_name [String] The family name for a script family.
@@ -483,7 +484,7 @@ def self.latest_stable_version(family_name, version_year: nil, locale: 'en-us')
     # Only select stable, supported scripts (ignore supported locales if locale is an English-speaking locale).
     # Match on version year if one is supplied.
     supported_stable_scripts = script_versions.select do |script|
-      is_supported = script.supported_locales&.include?(locale) || locale.start_with?('en')
+      is_supported = script.supported_locales&.include?(locale) || locale&.start_with?('en')
       if version_year
         script.is_stable && is_supported && script.version_year == version_year
       else
@@ -1198,7 +1199,7 @@ def summarize(include_stages = true, user = nil)
       age_13_required: logged_out_age_13_required?,
       show_course_unit_version_warning: !course&.has_dismissed_version_warning?(user) && has_older_course_progress,
       show_script_version_warning: !user_script&.version_warning_dismissed && !has_older_course_progress && has_older_script_progress,
-      versions: summarize_versions,
+      versions: summarize_versions(user),
       supported_locales: supported_locales,
       section_hidden_unit_info: section_hidden_unit_info(user),
     }
@@ -1274,12 +1275,12 @@ def summarize_i18n(include_stages=true)
 
   # Returns an array of objects showing the name and version year for all scripts
   # sharing the family_name of this course, including this one.
-  def summarize_versions
+  def summarize_versions(user = nil)
     return [] unless family_name
     return [] unless courses.empty?
     Script.
       where(family_name: family_name).
-      map {|s| {name: s.name, version_year: s.version_year, version_title: s.version_year}}.
+      map {|s| {name: s.name, version_year: s.version_year, version_title: s.version_year, can_view_version: s.can_view_version?(user)}}.
       sort_by {|info| info[:version_year]}.
       reverse
   end

dashboard/test/models/course_test.rb

@@ -322,6 +322,11 @@ class CanViewVersion < ActiveSupport::TestCase
       assert @csp_2017.can_view_version?(create(:teacher))
     end
 
+    test 'nil user can only view latest version in course family' do
+      assert @csp_2018.can_view_version?(nil)
+      refute @csp_2017.can_view_version?(nil)
+    end
+
     test 'student can view version if it is the latest version in course family' do
       assert @csp_2018.can_view_version?(@student)
       refute @csp_2017.can_view_version?(@student)

dashboard/test/ui/features/course_versions.feature

@@ -7,13 +7,21 @@ Scenario: Version warning announcement on course and unit overview pages
 
   When I am on "http://studio.code.org/courses/csp-2018"
   And I wait to see ".uitest-CourseScript"
-  And element "#version-selector" is visible
+  And element "#version-selector" is not visible
   Then element ".announcement-notification:contains(newer version)" does not exist
 
+  # students must be assigned or have progress to view older script versions
+
+  Given I am assigned to script "csp3-2017"
+  When I am on "http://studio.code.org/courses/csp-2018"
+  And I wait to see ".uitest-CourseScript"
+  And element "#version-selector" is visible
+  Then element ".announcement-notification:contains(newer version)" is visible
+
   When I am on "http://studio.code.org/s/csp2-2018"
   And I wait until element "#script-title" is visible
   And element "#version-selector" is not visible
-  Then element ".announcement-notification:contains(newer version)" does not exist
+  Then element ".announcement-notification:contains(newer version)" is visible
 
   # generate some progress in csp-2017
 
@@ -57,9 +65,10 @@ Scenario: Version warning announcement on course and unit overview pages
 Scenario: Versions warning announcement on script overview page
   When I am on "http://studio.code.org/s/coursea-2018"
   And I wait until element "#script-title" is visible
-  And element "#version-selector" is visible
+  And element "#version-selector" is not visible
   Then element ".announcement-notification:contains(newer version)" does not exist
 
+  Given I am assigned to script "coursea-2017"
   When I am on "http://studio.code.org/s/coursea-2017/next"
   And I wait until current URL contains "/s/coursea-2017/stage/1/puzzle/1"
 
@@ -92,6 +101,12 @@ Scenario: Versions warning announcement on script overview page
 @as_student
 @no_mobile
 Scenario: Switch versions using dropdown on script overview page
+  # Older script versions are not visible to students who are not assigned to them
+  When I am on "http://studio.code.org/s/coursea-2017"
+  And I get redirected to "s/coursea-2018" via "dashboard"
+  And I wait until element "#script-title" is visible
+  And element "#version-selector" is not visible
+
   Given I am assigned to script "coursea-2017"
   When I am on "http://studio.code.org/s/coursea-2017"
   And I wait until element "#script-title" is visible