Hide unit overview #17452
Hide unit overview #17452
Conversation
Bjvanminnen
force-pushed
the
hideUnitOverview
branch
from
785f32b
to
c7ead0f
Compare
| = t('beta') | ||
| - if @current_user.try(:script_hidden?, @script) | ||
| = render partial: 'hidden_script' | ||
| - else |
There was a problem hiding this comment.
Everything after this else is just whitespace diff
Bjvanminnen
force-pushed
the
hideUnitOverview
branch
from
3a5b949
to
75deea6
Compare
| @@ -0,0 +1,23 @@ | |||
| @eyes | |||
| @dashboard_db_access | |||
| @no_circle | |||
There was a problem hiding this comment.
no_circle because in circle we dont seed any courses. I'm going to explore fixing that in another branch, but dont want to let that block getting this in.
dashboard/app/models/user.rb
Outdated
| @@ -885,6 +885,20 @@ def script_level_hidden?(script_level) | |||
| end | |||
| end | |||
|
|
|||
| # Is the give script hidden for this user (based on the sections that they are in) | |||
dashboard/app/models/user.rb
Outdated
| def script_hidden?(script) | ||
| return false if try(:teacher?) | ||
|
|
||
| sections = sections_as_student.select {|s| s.deleted_at.nil?} |
There was a problem hiding this comment.
I'm surprised that excluding deleted sections isn't implicit in the sections_as_student call. What determines when we need to say with_deleted to get deleted entries, and when we'd expect to have to exclude them manually?
There was a problem hiding this comment.
I think this is code that was copied from elsewhere and written before we had the acts_as_paranoid. I'll go validate, but I suspect I can just rid of the deleted_at check here (and in the place this was copied from)
There was a problem hiding this comment.
Manually validated that sections_as_student does not return deleted sections, then fixed up both here and script_level_hidden?
If a teacher has hidden a unit, we will already not show that unit to you on the course overview page. However, before this PR you could still navigate directly to that unit if you knew the url. This makes is so that we instead get the above screenshot.
We don't expect people to hit this very often, though I suspect it could be more frequent than the equivalent UI for when you go to a puzzle page.