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.

Sign up for GitHub

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

Jump to bottom

Emergency mode banner for teacher dashboard #43702

Merged
merged 12 commits into from
Nov 22, 2021
Merged

Emergency mode banner for teacher dashboard #43702

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
69900e8
new banner component
Nov 12, 2021
a5b34db
update strings
Nov 12, 2021
433ae06
show banner for any script where we are not saving progress
Nov 16, 2021
965de9f
Merge branch 'staging' into emergency-mode-banners
Nov 18, 2021
a1c5a48
update strings to match other emergency mode banner language
Nov 19, 2021
482c0d3
remove unused strings
Nov 19, 2021
b4a2b57
use less confusing varibale name that we use elsewhere
Nov 19, 2021
9d7b03a
update tests
Nov 19, 2021
0543876
update tests
Nov 19, 2021
e79490c
make value always a bool
Nov 19, 2021
f017ad5
code review feedback
Nov 22, 2021
491ff78
Merge branch 'staging' into emergency-mode-banners
Nov 22, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions apps/i18n/common/en_us.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -618,6 +618,8 @@
"disabledProgress1": "Yikes! This week is Hour of Code and we are experiencing even more traffic than we anticipated.",
"disabledProgress2": "Unfortunately, we are not able to save the progress that you make on your course while we are dealing with this issue. Don't worry, your progress from before this week is still safe.",
"disabledProgress3": "You can still try Hour of Code tutorials even though your progress won't save.",
"disabledProgressTeacherDashboard1": "Warning: Progress not saving",
"disabledProgressTeacherDashboard2": "Unfortunately, we are not able to save the progress that your students make on their course while we are dealing with this issue. Don't worry, their progress from before this week is still safe. They can still try Hour of Code tutorials even though their progress won't save.",
Copy link
Contributor

Choose a reason for hiding this comment

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

(minor) I thought "while we are dealing with this issue" sounded a bit negative but I see that this string is derived from the one above so am ok with just leaving this as-is.

"disableMaker": "Disable Maker Toolkit",
"discussionGoal": "Discussion Goal",
"discountCodeSchoolConfirm": "Before you can receive your code, please verify the school at which you teach:",
Expand Down
28 changes: 28 additions & 0 deletions apps/src/templates/teacherDashboard/TeacherDashboardHeader.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import {
import {sectionShape} from '@cdo/apps/templates/teacherDashboard/shapes';
import Button from '../Button';
import DropdownButton from '../DropdownButton';
import {disabledBubblesSupportArticle} from '@cdo/apps/code-studio/disabledBubbles';

class TeacherDashboardHeader extends React.Component {
static propTypes = {
Expand Down Expand Up @@ -70,6 +71,30 @@ class TeacherDashboardHeader extends React.Component {
/>
));

progressNotSavingNotification() {
let details = (
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this be const?

<p>
{i18n.disabledProgress1()} {i18n.disabledProgressTeacherDashboard2()}{' '}
<a
target="_blank"
rel="noopener noreferrer"
href={disabledBubblesSupportArticle}
>
{i18n.learnMore()}
</a>
</p>
);

return (
<Notification
type={NotificationType.failure}
notice={i18n.disabledProgressTeacherDashboard1()}
details={details}
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think I have a strong opinion either way, but is there a reason to do the "Learn more" as an inline link to the article rather than a button? Mostly asking since the Notification component comes with the option to use a button built in, which to my memory is more commonly used.

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for the tip - I switched it to use the detailsLink props because the button looked weird with such a long notification text.

dismissable={false}
/>
);
}

render() {
return (
<div>
Expand All @@ -83,6 +108,9 @@ class TeacherDashboardHeader extends React.Component {
restrictSection={this.props.selectedSection.restrictSection}
loginType={this.props.selectedSection.loginType}
/>
{this.props.selectedSection.postMilestoneDisabled && (
<this.progressNotSavingNotification />
)}
<div style={styles.header}>
<div>
<h1>{this.props.selectedSection.name}</h1>
Expand Down
3 changes: 2 additions & 1 deletion apps/src/templates/teacherDashboard/shapes.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ export const sectionShape = PropTypes.shape({
scriptId: PropTypes.number,
grade: PropTypes.string,
providerManaged: PropTypes.bool.isRequired,
restrictSection: PropTypes.bool
restrictSection: PropTypes.bool,
postMilestoneDisabled: PropTypes.bool
});

// Used on the Teacher Dashboard for components that
Expand Down
3 changes: 2 additions & 1 deletion apps/src/templates/teacherDashboard/teacherSectionsRedux.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1187,7 +1187,8 @@ export const sectionFromServerSection = serverSection => ({
hidden: serverSection.hidden,
isAssigned: serverSection.isAssigned,
restrictSection: serverSection.restrict_section,
codeReviewEnabled: serverSection.code_review_enabled
codeReviewEnabled: serverSection.code_review_enabled,
postMilestoneDisabled: serverSection.post_milestone_disabled
});

/**
Expand Down
28 changes: 20 additions & 8 deletions apps/test/unit/templates/teacherDashboard/teacherSectionsReduxTest.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,8 @@ const sections = [
studentCount: 10,
hidden: false,
restrict_section: false,
code_review_enabled: true
code_review_enabled: true,
post_milestone_disabled: false
},
{
id: 12,
Expand All @@ -97,7 +98,8 @@ const sections = [
studentCount: 1,
hidden: false,
restrict_section: false,
code_review_enabled: true
code_review_enabled: true,
post_milestone_disabled: false
},
{
id: 307,
Expand All @@ -119,7 +121,8 @@ const sections = [
studentCount: 0,
hidden: false,
restrict_section: false,
code_review_enabled: true
code_review_enabled: true,
post_milestone_disabled: false
}
];

Expand Down Expand Up @@ -646,7 +649,8 @@ describe('teacherSectionsRedux', () => {
hidden: false,
isAssigned: undefined,
restrictSection: false,
codeReviewEnabled: true
codeReviewEnabled: true,
postMilestoneDisabled: false
});
});
});
Expand Down Expand Up @@ -787,7 +791,8 @@ describe('teacherSectionsRedux', () => {
createdAt: createdAt,
hidden: false,
restrict_section: false,
code_review_enabled: true
code_review_enabled: true,
post_milestone_disabled: false
};

function successResponse(customProps = {}) {
Expand Down Expand Up @@ -939,7 +944,8 @@ describe('teacherSectionsRedux', () => {
hidden: false,
isAssigned: undefined,
restrictSection: false,
codeReviewEnabled: true
codeReviewEnabled: true,
postMilestoneDisabled: false
}
});
});
Expand Down Expand Up @@ -995,7 +1001,8 @@ describe('teacherSectionsRedux', () => {
script_id: null,
hidden: false,
restrict_section: false,
code_review_enabled: true
code_review_enabled: true,
post_milestone_disabled: false
};

function successResponse(sectionId, customProps = {}) {
Expand Down Expand Up @@ -1228,7 +1235,8 @@ describe('teacherSectionsRedux', () => {
studentCount: 10,
hidden: false,
restrict_section: false,
code_review_enabled: true
code_review_enabled: true,
post_milestone_disabled: false
};

it('transfers some fields directly, mapping from snake_case to camelCase', () => {
Expand Down Expand Up @@ -1258,6 +1266,10 @@ describe('teacherSectionsRedux', () => {
section.code_review_enabled,
serverSection.codeReviewEnabled
);
assert.strictEqual(
section.post_milestone_disabled,
serverSection.postMilestoneDisabled
);
});

it('maps from a script object to a script_id', () => {
Expand Down
2 changes: 2 additions & 0 deletions dashboard/app/models/sections/section.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,8 @@ def summarize(include_students: true)
restrict_section: restrict_section,
code_review_enabled: code_review_enabled?,
is_assigned_csa: assigned_csa?,
# this will be true when we are in emergency mode and this section has any script assigned other than CSP or CSD
Copy link
Contributor

Choose a reason for hiding this comment

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

It might be more accurate to say that this will be true for CSF and HoC courses? (Even more precisely, it is the set of scripts returned here.)

Copy link
Contributor

Choose a reason for hiding this comment

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

+1 to this suggestion because making the comment more specific will be more future-proof

Copy link
Author

Choose a reason for hiding this comment

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

Thank you! This is definitely better!

post_milestone_disabled: !!script && !Gatekeeper.allows('postMilestone', where: {script_name: script.name}, default: true),
code_review_expires_at: code_review_expires_at
}
end
Expand Down
4 changes: 4 additions & 0 deletions dashboard/test/models/sections/section_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -394,6 +394,7 @@ def verify(actual, expected)
restrict_section: false,
code_review_enabled: true,
is_assigned_csa: false,
post_milestone_disabled: false,
code_review_expires_at: nil
}
# Compare created_at separately because the object's created_at microseconds
Expand Down Expand Up @@ -437,6 +438,7 @@ def verify(actual, expected)
restrict_section: false,
code_review_enabled: true,
is_assigned_csa: false,
post_milestone_disabled: false,
code_review_expires_at: nil
}
# Compare created_at separately because the object's created_at microseconds
Expand Down Expand Up @@ -483,6 +485,7 @@ def verify(actual, expected)
restrict_section: false,
code_review_enabled: true,
is_assigned_csa: false,
post_milestone_disabled: false,
code_review_expires_at: nil
}
# Compare created_at separately because the object's created_at microseconds
Expand Down Expand Up @@ -523,6 +526,7 @@ def verify(actual, expected)
restrict_section: false,
code_review_enabled: true,
is_assigned_csa: false,
post_milestone_disabled: false,
code_review_expires_at: nil
}
# Compare created_at separately because the object's created_at microseconds
Expand Down