Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

unhide script and warn when assigning it from the script overview page #24684

Merged
merged 13 commits into from Sep 8, 2018

Conversation

davidsbailey
Copy link
Member

Background

Spec line item: https://docs.google.com/document/d/17X2bEVK4YO4gymmVEzAnDYCYTjE5xHQTlDSAZbEprTo/edit#bookmark=id.hko4b4h5at6x

Terminology

The term "hidden script" is ambiguous. This PR talks about hiding scripts for certain sections as determined by the SectionHiddenScript model, which a teacher controls by that script's course's CourseOverview page. No part of this PR has anything to do with the hidden property on the Script model, which is controlled via the levelbuilder UI.

Description

  1. if a script is hidden for a section, and the section is assigned to that script via SectionsController.update, unhide the script for that section.
  2. if the script is assigned via the AssignToSection button on the ScriptOverview page, show a more accurate warning in the dialog prior to unhiding and assigning.

Future work

  1. show a more accurate warning when the script is assigned via EditSectionForm, pending in warn before unhiding and assigning script via EditSectionForm #24682

Screenshots

overview-unhide-and-assign

Copy link
Contributor

@islemaster islemaster left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! Well-tested.

@@ -1111,6 +1112,18 @@ def summarize(include_stages = true, user = nil)
summary
end

# @return {Hash<string,number[]>|number[]}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like the last bit of this type signature is wrong; just {Hash<string,number[]>}?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch! copy-pasta.

@davidsbailey davidsbailey merged commit f1fd719 into staging Sep 8, 2018
@davidsbailey davidsbailey deleted the overview-unhide-assigned-unit branch September 10, 2018 17:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants