Hide Playthrough Issues on Exploration Editor Stats Tab if Exploration is not Whitelisted #5947

brianrodri · 2018-12-06T06:50:39Z

Playthroughs should only be visible and recorded when a particular exploration is whitelisted. This allows us to continue working on changes and avoid recording potentially unsafe data until we're confident that Playthroughs are ready for general usage.

Testing Steps:

Go to: Admin Page > Activities
- Load Single Exploration 'Hola' (exploration id: '7')
Go to: Admin Page > Config
- Add element '7' to "The set of exploration IDs for recording issues and playthroughs"
- Set "The probability of recording playthroughs" to 1 (i.e. 100%)
- Save changes
Play Hola exploration
- Answer with 'Hola' and move to next card
- Immediately navigate away from the exploration (and accept warning)
- This should log an "Early Quit Playthrough"
Go to: Hola's Exploration Editor > Stats Tab
- Make sure Playthrough directive is visible and has one issue
Go to: Admin Page > Config
- Remove '7' from "The set of exploration IDs for recording issues and playthroughs"
- Save changes
Go to: Hola's Exploration Editor > Stats Tab
- Make sure Playthrough directive is not visible
Play Hola exploration
- Answer with 'x' several times (~5s apart)
- Navigate away from the exploration (and accept warning)
- This should log a "Multiple Incorrect Answer Playthrough"
Go to: Admin Page > Config
- Add element '7' to "The set of exploration IDs for recording issues and playthroughs"
Go to: Hola's Exploration Editor > Stats Tab
- Make sure Playthrough directive is visible and still has exactly one issue
Play Hola exploration
- Answer with 'y' several times (~5s apart)
- Navigate away from the exploration (and accept warning)
- This should log a "Multiple Incorrect Answer Playthrough"
Go to: Hola's Exploration Editor > Stats Tab
- Make sure Playthrough directive is visible and has exactly two issues

…velop

seanlip · 2018-12-06T07:01:48Z

@nithusha21 @BenHenning Please ignore the note on #5945. It's this PR (not that one) that should be merged for December.

Thanks!

nithusha21 · 2018-12-06T07:38:11Z

Hi @brianrodri thanks for the detailed testing instructions! However, I do not have admin permissions on the test server. Do you want me to test this locally?

Also, thanks for the PR, will make sure it is cherrypicked!

seanlip · 2018-12-06T08:24:07Z

I've updated the test server config (and let @nithusha21 know)!

seanlip · 2018-12-06T08:34:12Z

@brianrodri -- e2e tests are failing?

codecov-io · 2018-12-06T14:56:25Z

Codecov Report

Merging #5947 into develop will decrease coverage by <.01%.
The diff coverage is 0%.

@@             Coverage Diff             @@
##           develop    #5947      +/-   ##
===========================================
- Coverage    45.48%   45.47%   -0.01%     
===========================================
  Files          517      517              
  Lines        30372    30378       +6     
  Branches      4568     4571       +3     
===========================================
  Hits         13814    13814              
- Misses       16558    16564       +6

Impacted Files	Coverage Δ
..._editor/statistics_tab/PlaythroughIssuesService.js	`1.69% <0%> (-0.16%)`	⬇️
...head/pages/exploration_editor/ExplorationEditor.js	`5.22% <0%> (ø)`	⬆️
...exploration_editor/statistics_tab/StatisticsTab.js	`2.17% <0%> (-0.03%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a7f2ae8...62b6d77. Read the comment docs.

seanlip · 2018-12-06T19:47:47Z

core/templates/dev/head/pages/exploration_editor/statistics_tab/PlaythroughIssuesService.js

        explorationId = newExplorationId;
        explorationVersion = newExplorationVersion;
+        whitelistedExplorationIds = newWhitelistedExplorationIds || [];


Hi @brianrodri, I am a bit confused about this. Why do we need the || []? In other words why is newWhitelistedExplorationIds ever null?

…n is not Whitelisted (oppia#5947) * Fix hola's multiple choice question. * Pass whitelist to StatsTab * Fix line-too-long * Remove console * always set whitelist to a list

* Hide Playthrough Issues on Exploration Editor Stats Tab if Exploration is not Whitelisted (#5947) * Fix hola's multiple choice question. * Pass whitelist to StatsTab * Fix line-too-long * Remove console * always set whitelist to a list * Small landing page changes based on feedback from Kefeh. (#5951) * Fix error while displaying suggestion threads on learner dashboard page (#5965) * fix error on learner dashboard page * fix style

nithusha21 · 2018-12-18T14:39:08Z

Hi @brianrodri just leaving a note here. I have tested this PR on the test server, and everything works perfectly fine. Thanks a lot for the detailed instructions to test!

brianrodri added 23 commits August 11, 2018 16:50

Fix hola's multiple choice question.

c9d4d22

Merge remote-tracking branch 'upstream/develop' into develop

9b16504

Merge remote-tracking branch 'upstream/develop' into develop

80cf6c9

Merge branch 'develop' of https://github.com/brianrodri/oppia into de…

1904b2d

…velop

Merge remote-tracking branch 'upstream/develop' into develop

1d21cf8

Merge remote-tracking branch 'upstream/develop' into develop

0d41226

Merge remote-tracking branch 'upstream/develop' into develop

cbd7666

Merge remote-tracking branch 'upstream/develop' into develop

8db0992

Merge remote-tracking branch 'upstream/develop' into develop

18658de

Merge remote-tracking branch 'upstream/develop' into develop

bca45b2

Merge remote-tracking branch 'upstream/develop' into develop

d956212

Merge remote-tracking branch 'upstream/develop' into develop

67bf6e2

Merge remote-tracking branch 'upstream/develop' into develop

afc36e2

Merge remote-tracking branch 'upstream/develop' into develop

926a925

Merge remote-tracking branch 'upstream/develop' into develop

bfb2fb7

Merge remote-tracking branch 'upstream/develop' into develop

e4efd3e

Merge remote-tracking branch 'upstream/develop' into develop

0478611

Merge remote-tracking branch 'upstream/develop' into develop

0899b3a

Merge remote-tracking branch 'upstream/develop' into develop

050f37f

Merge remote-tracking branch 'upstream/develop' into develop

37aef06

Pass whitelist to StatsTab

8b06177

Fix line-too-long

30c9d96

Remove console

172292f

brianrodri added the PR: for current release label Dec 6, 2018

brianrodri self-assigned this Dec 6, 2018

brianrodri requested review from seanlip, nithusha21 and BenHenning December 6, 2018 06:50

seanlip approved these changes Dec 6, 2018

View reviewed changes

seanlip added the PR: LGTM label Dec 6, 2018

always set whitelist to a list

62b6d77

brianrodri changed the title ~~Hide Playthrough Issues when whitelist is not enabled~~ Hide Playthrough Issues on Exploration Editor Stats Tab if Exploration is not Whitelisted Dec 6, 2018

brianrodri removed request for nithusha21 and BenHenning December 6, 2018 15:34

brianrodri merged commit e93451b into oppia:develop Dec 6, 2018

brianrodri deleted the hide-playthroughs branch December 6, 2018 15:42

seanlip reviewed Dec 6, 2018

View reviewed changes

BenHenning added the PR: released label Dec 18, 2018

BenHenning removed PR: released PR: for current release labels Dec 20, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Hide Playthrough Issues on Exploration Editor Stats Tab if Exploration is not Whitelisted #5947

Hide Playthrough Issues on Exploration Editor Stats Tab if Exploration is not Whitelisted #5947

brianrodri commented Dec 6, 2018 •

edited

Loading

seanlip commented Dec 6, 2018 •

edited

Loading

nithusha21 commented Dec 6, 2018

seanlip commented Dec 6, 2018

seanlip commented Dec 6, 2018

codecov-io commented Dec 6, 2018 •

edited

Loading

seanlip Dec 6, 2018

nithusha21 commented Dec 18, 2018

Hide Playthrough Issues on Exploration Editor Stats Tab if Exploration is not Whitelisted #5947

Hide Playthrough Issues on Exploration Editor Stats Tab if Exploration is not Whitelisted #5947

Conversation

brianrodri commented Dec 6, 2018 • edited Loading

seanlip commented Dec 6, 2018 • edited Loading

nithusha21 commented Dec 6, 2018

seanlip commented Dec 6, 2018

seanlip commented Dec 6, 2018

codecov-io commented Dec 6, 2018 • edited Loading

Codecov Report

seanlip Dec 6, 2018

Choose a reason for hiding this comment

nithusha21 commented Dec 18, 2018

brianrodri commented Dec 6, 2018 •

edited

Loading

seanlip commented Dec 6, 2018 •

edited

Loading

codecov-io commented Dec 6, 2018 •

edited

Loading