From 37d2d84e1e51b36c7fc621ff205c10a742c1dd0c Mon Sep 17 00:00:00 2001 From: Erin Peach Date: Thu, 10 Mar 2022 14:25:51 -0800 Subject: [PATCH 1/8] Prioritize override in filter boolean --- apps/src/dance/DanceVisualizationColumn.jsx | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/apps/src/dance/DanceVisualizationColumn.jsx b/apps/src/dance/DanceVisualizationColumn.jsx index 3d77d390960e8..1b122481d95d8 100644 --- a/apps/src/dance/DanceVisualizationColumn.jsx +++ b/apps/src/dance/DanceVisualizationColumn.jsx @@ -85,7 +85,12 @@ class DanceVisualizationColumn extends React.Component { const signedInOver13 = this.props.userType === 'teacher' || this.props.userType === 'student'; const signedOutAge = signedOutOver13(); - return signedInOver13 || signedOutAge; + // Student is signed out and <13 or the override filter is on + if (!signedOutAge) { + return signedOutAge; + } + // Check whether user is signed in and >13 + return signedInOver13; } render() { From 0df46893ff53fb68e24f1350be695da510c4503d Mon Sep 17 00:00:00 2001 From: Erin Peach Date: Fri, 11 Mar 2022 12:27:17 -0800 Subject: [PATCH 2/8] Add specific session storage for filter --- apps/src/dance/DanceVisualizationColumn.jsx | 15 ++++++++------- apps/src/templates/AgeDialog.jsx | 12 +++++++----- .../features/star_labs/dance/age_filter.feature | 1 + 3 files changed, 16 insertions(+), 12 deletions(-) diff --git a/apps/src/dance/DanceVisualizationColumn.jsx b/apps/src/dance/DanceVisualizationColumn.jsx index ba9f5d3b47014..c07995cca95c1 100644 --- a/apps/src/dance/DanceVisualizationColumn.jsx +++ b/apps/src/dance/DanceVisualizationColumn.jsx @@ -8,7 +8,7 @@ import PropTypes from 'prop-types'; import Radium from 'radium'; import {connect} from 'react-redux'; import i18n from '@cdo/locale'; -import AgeDialog, {signedOutOver13} from '../templates/AgeDialog'; +import AgeDialog, {signedOutOver13, songFilterOn} from '../templates/AgeDialog'; const SongSelector = Radium( class extends React.Component { @@ -86,13 +86,14 @@ class DanceVisualizationColumn extends React.Component { const signedInOver13 = this.props.userType === 'teacher' || (this.props.userType === 'student' && !this.props.under13); - const signedOutAge = signedOutOver13(); - // Student is signed out and <13 or the override filter is on - if (!signedOutAge) { - return signedOutAge; + const signedOutOverAge = signedOutOver13(); + const songFilter = songFilterOn(); + // Override filter is on + if (songFilter) { + return false; } - // Check whether user is signed in and >13 - return signedInOver13; + // Check whether user is > 13 + return signedInOver13 || signedOutOverAge; } render() { diff --git a/apps/src/templates/AgeDialog.jsx b/apps/src/templates/AgeDialog.jsx index adfac697517da..48ea05515e134 100644 --- a/apps/src/templates/AgeDialog.jsx +++ b/apps/src/templates/AgeDialog.jsx @@ -15,11 +15,16 @@ import queryString from 'query-string'; * a different session storage key here. */ const AGE_DIALOG_SESSION_KEY = 'ad_anon_over13'; +const SONG_FILTER_SESSION_KEY = 'song_filter_on'; export const signedOutOver13 = () => { return sessionStorage.getItem(AGE_DIALOG_SESSION_KEY) === 'true'; }; +export const songFilterOn = () => { + return sessionStorage.getItem(SONG_FILTER_SESSION_KEY) === 'true'; +}; + class AgeDialog extends Component { state = { open: true @@ -35,10 +40,6 @@ class AgeDialog extends Component { storage: window.sessionStorage }; - setManualFilter = () => { - this.setSessionStorage(false); - }; - setSessionStorage = over13 => { this.props.storage.setItem(AGE_DIALOG_SESSION_KEY, over13); this.setState({open: false}); @@ -48,7 +49,8 @@ class AgeDialog extends Component { // If the song filter override has been turned on, set session storage // Dialog won't render if (queryString.parse(window.location.search).songfilter === 'on') { - this.setManualFilter(); + this.props.storage.setItem(SONG_FILTER_SESSION_KEY, true); + this.setState({open: false}); } } diff --git a/dashboard/test/ui/features/star_labs/dance/age_filter.feature b/dashboard/test/ui/features/star_labs/dance/age_filter.feature index b2c253df8ff00..038068ad782c3 100644 --- a/dashboard/test/ui/features/star_labs/dance/age_filter.feature +++ b/dashboard/test/ui/features/star_labs/dance/age_filter.feature @@ -29,6 +29,7 @@ Feature: Dance Lab Age Filter And I see option "Synthesize" or "Ed Sheeran - Shape of You" in the dropdown "#song_selector" Then I am on "http://studio.code.org/s/allthethings/lessons/37/levels/1?noautoplay=true&songfilter=on" + And I reload the page And I wait for the page to fully load And I wait until I don't see selector "#p5_loading" And I wait for the song selector to load From 6f73f4a5ae4aaf4206f9ff13ccd893e04b3297ad Mon Sep 17 00:00:00 2001 From: Erin Peach Date: Fri, 18 Mar 2022 16:38:30 -0700 Subject: [PATCH 3/8] Simplify naming --- apps/src/dance/DanceVisualizationColumn.jsx | 41 ++++++++++++--------- apps/src/templates/AgeDialog.jsx | 4 +- 2 files changed, 26 insertions(+), 19 deletions(-) diff --git a/apps/src/dance/DanceVisualizationColumn.jsx b/apps/src/dance/DanceVisualizationColumn.jsx index cae4c84411ad8..4bbf23aa73dcc 100644 --- a/apps/src/dance/DanceVisualizationColumn.jsx +++ b/apps/src/dance/DanceVisualizationColumn.jsx @@ -8,7 +8,10 @@ import PropTypes from 'prop-types'; import Radium from 'radium'; import {connect} from 'react-redux'; import i18n from '@cdo/locale'; -import AgeDialog, {signedOutOver13, songFilterOn} from '../templates/AgeDialog'; +import AgeDialog, { + signedOutUnder13 as signedOutAgeCheck, + songFilterOn +} from '../templates/AgeDialog'; const SongSelector = Radium( class extends React.Component { @@ -17,7 +20,7 @@ const SongSelector = Radium( setSong: PropTypes.func.isRequired, selectedSong: PropTypes.string, songData: PropTypes.objectOf(PropTypes.object).isRequired, - filterOff: PropTypes.bool.isRequired + filterOn: PropTypes.bool.isRequired }; changeSong = event => { @@ -40,7 +43,8 @@ const SongSelector = Radium( > {Object.keys(this.props.songData).map( (option, i) => - (this.props.filterOff || !this.props.songData[option].pg13) && ( + // Song should be displayed if it is not pg13 or if the filter is off. + (!this.props.filterOn || !this.props.songData[option].pg13) && ( @@ -67,33 +71,36 @@ class DanceVisualizationColumn extends React.Component { }; state = { - filterOff: this.setFilterStatus() + filterOn: this.initializeFilterStatus() }; /* Turn the song filter off */ turnFilterOff = () => { - this.setState({filterOff: true}); + this.setState({filterOn: false}); }; /* The filter defaults to on. If the user is over 13 (identified via account or anon dialog), filter turns off. */ - setFilterStatus() { - // userType - 'teacher', 'student', 'unknown' - signed out users. - // under13 - boolean for signed in user representing age category. Teacher assumed > 13. - const signedInOver13 = - this.props.userType === 'teacher' || - (this.props.userType === 'student' && !this.props.under13); - const signedOutOverAge = signedOutOver13(); + initializeFilterStatus() { + const {userType, under13} = this.props; + + // Check if song filter override is triggered and initialize song filter to true. const songFilter = songFilterOn(); - // Override filter is on if (songFilter) { - return false; + return true; } - // Check whether user is > 13 - return signedInOver13 || signedOutOverAge; + + // userType - 'teacher', 'student', 'unknown' - signed out users. + // under13 - boolean for signed in user representing age category. Teacher assumed > 13. + const signedInUnder13 = userType === 'student' && under13; + const signedOutUnder13 = signedOutAgeCheck(); + + // Return true (filter on) if user is under 13, whether they are signed in or out. + // Return false if the user is over 13, signed in or out. + return signedInUnder13 || signedOutUnder13; } render() { @@ -118,7 +125,7 @@ class DanceVisualizationColumn extends React.Component { setSong={this.props.setSong} selectedSong={this.props.selectedSong} songData={this.props.songData} - filterOff={this.state.filterOff} + filterOn={this.state.filterOn} /> )} diff --git a/apps/src/templates/AgeDialog.jsx b/apps/src/templates/AgeDialog.jsx index 48ea05515e134..ebc2162da4ce1 100644 --- a/apps/src/templates/AgeDialog.jsx +++ b/apps/src/templates/AgeDialog.jsx @@ -17,8 +17,8 @@ import queryString from 'query-string'; const AGE_DIALOG_SESSION_KEY = 'ad_anon_over13'; const SONG_FILTER_SESSION_KEY = 'song_filter_on'; -export const signedOutOver13 = () => { - return sessionStorage.getItem(AGE_DIALOG_SESSION_KEY) === 'true'; +export const signedOutUnder13 = () => { + return sessionStorage.getItem(AGE_DIALOG_SESSION_KEY) !== 'true'; }; export const songFilterOn = () => { From cdfdc8ebf7cc2d11825f5861f6d1d8b5b4d9544f Mon Sep 17 00:00:00 2001 From: Erin Peach Date: Wed, 23 Mar 2022 14:35:51 -0700 Subject: [PATCH 4/8] Move logic out of return --- apps/src/dance/DanceVisualizationColumn.jsx | 35 +++++++++++++-------- 1 file changed, 22 insertions(+), 13 deletions(-) diff --git a/apps/src/dance/DanceVisualizationColumn.jsx b/apps/src/dance/DanceVisualizationColumn.jsx index 4bbf23aa73dcc..70ebbeee083d6 100644 --- a/apps/src/dance/DanceVisualizationColumn.jsx +++ b/apps/src/dance/DanceVisualizationColumn.jsx @@ -29,6 +29,19 @@ const SongSelector = Radium( }; render() { + const { + selectedSong, + songData, + enableSongSelection, + filterOn + } = this.props; + + let songKeys = Object.keys(songData); + if (filterOn) { + // Filter is on, only include songs that are not pg13 + songKeys = songKeys.filter(key => !songData[key].pg13); + } + return (
); @@ -71,7 +80,7 @@ class DanceVisualizationColumn extends React.Component { }; state = { - filterOn: this.initializeFilterStatus() + filterOn: this.getFilterStatus() }; /* @@ -84,7 +93,7 @@ class DanceVisualizationColumn extends React.Component { /* The filter defaults to on. If the user is over 13 (identified via account or anon dialog), filter turns off. */ - initializeFilterStatus() { + getFilterStatus() { const {userType, under13} = this.props; // Check if song filter override is triggered and initialize song filter to true. From 59566cbf8f0cad94d1f17b09692d8182c9fcbc60 Mon Sep 17 00:00:00 2001 From: Erin Peach Date: Wed, 23 Mar 2022 15:10:21 -0700 Subject: [PATCH 5/8] Create util for filtering and add test --- apps/src/dance/DanceVisualizationColumn.jsx | 7 ++--- apps/src/dance/songs.js | 12 ++++++++ apps/test/unit/dance/songsTest.js | 31 +++++++++++++++++++++ 3 files changed, 45 insertions(+), 5 deletions(-) create mode 100644 apps/test/unit/dance/songsTest.js diff --git a/apps/src/dance/DanceVisualizationColumn.jsx b/apps/src/dance/DanceVisualizationColumn.jsx index 70ebbeee083d6..3aa762a9ccd1d 100644 --- a/apps/src/dance/DanceVisualizationColumn.jsx +++ b/apps/src/dance/DanceVisualizationColumn.jsx @@ -12,6 +12,7 @@ import AgeDialog, { signedOutUnder13 as signedOutAgeCheck, songFilterOn } from '../templates/AgeDialog'; +import {getFilteredSongKeys} from '@cdo/apps/dance/songs'; const SongSelector = Radium( class extends React.Component { @@ -36,11 +37,7 @@ const SongSelector = Radium( filterOn } = this.props; - let songKeys = Object.keys(songData); - if (filterOn) { - // Filter is on, only include songs that are not pg13 - songKeys = songKeys.filter(key => !songData[key].pg13); - } + let songKeys = getFilteredSongKeys(songData, filterOn); return (
diff --git a/apps/src/dance/songs.js b/apps/src/dance/songs.js index d5bf698015b82..62b82e4043b4e 100644 --- a/apps/src/dance/songs.js +++ b/apps/src/dance/songs.js @@ -106,3 +106,15 @@ export function parseSongOptions(songManifest) { }); return songs; } + +// Given a songManifest, returns a list of keys that represent the songs to +// be displayed based on whether the song filter is on. +export function getFilteredSongKeys(fullSongManifest, filterOn) { + let allSongKeys = Object.keys(fullSongManifest); + if (filterOn) { + // Filter is on, only include songs that are not pg13. + return allSongKeys.filter(key => !fullSongManifest[key].pg13); + } + // Filter is off, all songs may be displayed. + return allSongKeys; +} diff --git a/apps/test/unit/dance/songsTest.js b/apps/test/unit/dance/songsTest.js new file mode 100644 index 0000000000000..95b1608116b56 --- /dev/null +++ b/apps/test/unit/dance/songsTest.js @@ -0,0 +1,31 @@ +import {expect} from '../../util/reconfiguredChai'; +import {getFilteredSongKeys} from '@cdo/apps/dance/songs'; + +let testSongManifest = { + allAgesSong: { + text: 'Singer - Song For Everybody', + pg13: false, + url: 'singer_song' + }, + olderStudentSong: { + text: 'Band - Song for Older Students', + pg13: true, + url: 'band_song' + } +}; + +describe('Dance Party Songs Utils', function() { + describe('Song Filtering', function() { + it('when filtering is on, should only return keys of songs that are not pg-13', function() { + let filteredSongs = getFilteredSongKeys(testSongManifest, true); + expect(filteredSongs.length).to.equal(1); + expect(filteredSongs[0]).to.equal('allAgesSong'); + }); + + it('when filtering is off, all song keys are returned', function() { + let filteredSongs = getFilteredSongKeys(testSongManifest, false); + expect(filteredSongs.length).to.equal(2); + expect(filteredSongs[1]).to.equal('olderStudentSong'); + }); + }); +}); From 8ba18a624a0f9e959cec728a0d43fe20af15b3bd Mon Sep 17 00:00:00 2001 From: Erin Peach Date: Wed, 23 Mar 2022 17:13:10 -0700 Subject: [PATCH 6/8] Apply PR Feedback --- apps/src/dance/DanceVisualizationColumn.jsx | 7 +++---- apps/src/dance/songs.js | 2 +- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/apps/src/dance/DanceVisualizationColumn.jsx b/apps/src/dance/DanceVisualizationColumn.jsx index 3aa762a9ccd1d..18ca21872b606 100644 --- a/apps/src/dance/DanceVisualizationColumn.jsx +++ b/apps/src/dance/DanceVisualizationColumn.jsx @@ -9,7 +9,7 @@ import Radium from 'radium'; import {connect} from 'react-redux'; import i18n from '@cdo/locale'; import AgeDialog, { - signedOutUnder13 as signedOutAgeCheck, + signedOutUnder13, songFilterOn } from '../templates/AgeDialog'; import {getFilteredSongKeys} from '@cdo/apps/dance/songs'; @@ -37,7 +37,7 @@ const SongSelector = Radium( filterOn } = this.props; - let songKeys = getFilteredSongKeys(songData, filterOn); + const songKeys = getFilteredSongKeys(songData, filterOn); return (
@@ -102,11 +102,10 @@ class DanceVisualizationColumn extends React.Component { // userType - 'teacher', 'student', 'unknown' - signed out users. // under13 - boolean for signed in user representing age category. Teacher assumed > 13. const signedInUnder13 = userType === 'student' && under13; - const signedOutUnder13 = signedOutAgeCheck(); // Return true (filter on) if user is under 13, whether they are signed in or out. // Return false if the user is over 13, signed in or out. - return signedInUnder13 || signedOutUnder13; + return signedInUnder13 || signedOutUnder13(); } render() { diff --git a/apps/src/dance/songs.js b/apps/src/dance/songs.js index 62b82e4043b4e..d09f2fca2aea8 100644 --- a/apps/src/dance/songs.js +++ b/apps/src/dance/songs.js @@ -110,7 +110,7 @@ export function parseSongOptions(songManifest) { // Given a songManifest, returns a list of keys that represent the songs to // be displayed based on whether the song filter is on. export function getFilteredSongKeys(fullSongManifest, filterOn) { - let allSongKeys = Object.keys(fullSongManifest); + const allSongKeys = Object.keys(fullSongManifest); if (filterOn) { // Filter is on, only include songs that are not pg13. return allSongKeys.filter(key => !fullSongManifest[key].pg13); From aacfe3db21a198d62a05ea83521abb085f16ec32 Mon Sep 17 00:00:00 2001 From: Erin Peach Date: Thu, 24 Mar 2022 11:28:20 -0700 Subject: [PATCH 7/8] Break out logic for signed in and signed out --- apps/src/dance/DanceVisualizationColumn.jsx | 18 +++++++++--------- apps/src/templates/AgeDialog.jsx | 4 ++-- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/apps/src/dance/DanceVisualizationColumn.jsx b/apps/src/dance/DanceVisualizationColumn.jsx index 18ca21872b606..c8c49be07ec09 100644 --- a/apps/src/dance/DanceVisualizationColumn.jsx +++ b/apps/src/dance/DanceVisualizationColumn.jsx @@ -8,10 +8,7 @@ import PropTypes from 'prop-types'; import Radium from 'radium'; import {connect} from 'react-redux'; import i18n from '@cdo/locale'; -import AgeDialog, { - signedOutUnder13, - songFilterOn -} from '../templates/AgeDialog'; +import AgeDialog, {signedOutOver13, songFilterOn} from '../templates/AgeDialog'; import {getFilteredSongKeys} from '@cdo/apps/dance/songs'; const SongSelector = Radium( @@ -100,12 +97,15 @@ class DanceVisualizationColumn extends React.Component { } // userType - 'teacher', 'student', 'unknown' - signed out users. - // under13 - boolean for signed in user representing age category. Teacher assumed > 13. - const signedInUnder13 = userType === 'student' && under13; + // If user is signed out, query session key to determine age. + // Return false (no filter), if user is over 13. + if (userType === 'unknown') { + return !signedOutOver13(); + } - // Return true (filter on) if user is under 13, whether they are signed in or out. - // Return false if the user is over 13, signed in or out. - return signedInUnder13 || signedOutUnder13(); + // User is signed in (student or teacher) and the filter is off. + // Return true (filterOn) if the user is under 13. Teachers assumed over13. + return under13; } render() { diff --git a/apps/src/templates/AgeDialog.jsx b/apps/src/templates/AgeDialog.jsx index ebc2162da4ce1..48ea05515e134 100644 --- a/apps/src/templates/AgeDialog.jsx +++ b/apps/src/templates/AgeDialog.jsx @@ -17,8 +17,8 @@ import queryString from 'query-string'; const AGE_DIALOG_SESSION_KEY = 'ad_anon_over13'; const SONG_FILTER_SESSION_KEY = 'song_filter_on'; -export const signedOutUnder13 = () => { - return sessionStorage.getItem(AGE_DIALOG_SESSION_KEY) !== 'true'; +export const signedOutOver13 = () => { + return sessionStorage.getItem(AGE_DIALOG_SESSION_KEY) === 'true'; }; export const songFilterOn = () => { From fc83001deeff789a85178f3aa49b3c27ea0dc661 Mon Sep 17 00:00:00 2001 From: Erin Peach Date: Tue, 29 Mar 2022 13:57:45 -0700 Subject: [PATCH 8/8] Improve comments and function name --- apps/src/dance/DanceVisualizationColumn.jsx | 16 ++++++++++------ apps/src/templates/AgeDialog.jsx | 2 +- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/apps/src/dance/DanceVisualizationColumn.jsx b/apps/src/dance/DanceVisualizationColumn.jsx index c8c49be07ec09..35536c985eab6 100644 --- a/apps/src/dance/DanceVisualizationColumn.jsx +++ b/apps/src/dance/DanceVisualizationColumn.jsx @@ -8,7 +8,10 @@ import PropTypes from 'prop-types'; import Radium from 'radium'; import {connect} from 'react-redux'; import i18n from '@cdo/locale'; -import AgeDialog, {signedOutOver13, songFilterOn} from '../templates/AgeDialog'; +import AgeDialog, { + ageDialogSelectedOver13, + songFilterOn +} from '../templates/AgeDialog'; import {getFilteredSongKeys} from '@cdo/apps/dance/songs'; const SongSelector = Radium( @@ -97,14 +100,15 @@ class DanceVisualizationColumn extends React.Component { } // userType - 'teacher', 'student', 'unknown' - signed out users. - // If user is signed out, query session key to determine age. - // Return false (no filter), if user is over 13. + // If the user is signed out . . . if (userType === 'unknown') { - return !signedOutOver13(); + // Query session key set from user selection in age dialog. + // Return false (no filter), if user is over 13. + return !ageDialogSelectedOver13(); } - // User is signed in (student or teacher) and the filter is off. - // Return true (filterOn) if the user is under 13. Teachers assumed over13. + // User is signed in (student or teacher) and the filter override is not turned on. + // Return true (filter should be turned on) if the user is under 13. Teachers assumed over13. return under13; } diff --git a/apps/src/templates/AgeDialog.jsx b/apps/src/templates/AgeDialog.jsx index 48ea05515e134..ef091c5d46492 100644 --- a/apps/src/templates/AgeDialog.jsx +++ b/apps/src/templates/AgeDialog.jsx @@ -17,7 +17,7 @@ import queryString from 'query-string'; const AGE_DIALOG_SESSION_KEY = 'ad_anon_over13'; const SONG_FILTER_SESSION_KEY = 'song_filter_on'; -export const signedOutOver13 = () => { +export const ageDialogSelectedOver13 = () => { return sessionStorage.getItem(AGE_DIALOG_SESSION_KEY) === 'true'; };