Lab2: fix viewing student work on a sublevel #58666
Conversation
| const userId = useSelector( | ||
| (state: {progress: ProgressState}) => | ||
| state.progress.viewAsUserId || undefined | ||
| const currentLevelId = useAppSelector(state => state.progress.currentLevelId); |
There was a problem hiding this comment.
Thanks for updating all of these to use useAppSelector.
| script_level = params[:script_level_id] ? | ||
| ScriptLevel.cache_find(params[:script_level_id].to_i) : | ||
| level.script_levels.find_by_script_id(script_id) |
There was a problem hiding this comment.
Is there a potential malicious case in which the request provides a script_level_id which is used here to check for permission, but which is unrelated to the level_id which is used below to actually retrieve the channel?
There was a problem hiding this comment.
Oh true, that's a good point...yeah it would make sense to have a stricter check here. Ideally we should be able to use the script ID in combination with sublevel ID to find the script level ID. I'll look into that.
There was a problem hiding this comment.
Updated this with a check in projects_controller which ensures that if a script level ID is provided, it either matches the provided level ID or is associated with a valid parent level of provided sublevel ID. Added a unit test for this too. Let me know if you think this feels strict enough!
There was a problem hiding this comment.
I don't know these subsystems very well, but that seems like a good improvement, with good test coverage too, thanks!
sanchitmalhotra126
requested review from
breville,
a team and
kobryan0619
and removed request for
a team
Thanks! Good call on UI tests - I'll add a check to the existing lab2 bubble choice UI tests. |
Fix for viewing student work as a teacher on a lab2 sublevel. This was broken because the get_or_create_for_level API needed a script level ID to check if the teacher could view the student's work, which is not present on a sublevel. The fix here is just to explicitly pass through the script level ID of the current or parent level so that we can correctly check if the teacher can view the student's work.
Demo (disregard the duplicate levels 8 and 9, that's a local bug with my DB seeding).
Lab2SublevelTeacherPanel.mov
Links
https://codedotorg.atlassian.net/browse/LABS-803
Testing story
Tested on Lab2 allthethings progression, music intro script, and Gen AI pilot script. Also tested with individual levels and standalone projects to make sure no behavior regressed.