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

Assignment updates: confirmation dialog for assignment of hidden units #31899

Merged
merged 8 commits into from Nov 18, 2019

Conversation

Erin007
Copy link
Contributor

@Erin007 Erin007 commented Nov 14, 2019

LP-1023

Currently if a teacher tries to assign a unit that they have previously hidden from the Edit Section Dialog, they see a confirmation dialog prompting them to unhide the unit before assigning:
edit-section-hidden-assignment

This changes opens a similar dialog when a teacher tries to assign a previously hidden unit from the script or course overview page:
confirm-assigning-hidden-units

To accomplish this I renamed the ConfirmAssignment component to ConfirmHiddenAssignment and modified it to only handle this case because we are no longer using the confirmation dialog to confirm any other type of assignment. To populate the message in the dialog I needed to pass sectionName and assignmentName into the AssignButton component which handles when the dialog is displayed.

I added a UI test to ensure that the confirmation dialog opens when it should and "unhide and assign" works correctly. (However, this file of tests is currently being skipped because it's flaky, but it's a good start for when I re-enable).

@@ -8,7 +8,7 @@ import i18n from '@cdo/locale';
import Button from '@cdo/apps/templates/Button';
import DropdownButton from '@cdo/apps/templates/DropdownButton';
import BaseDialog from '@cdo/apps/templates/BaseDialog';
import ConfirmAssignment from './ConfirmAssignment';
import {ConfirmHiddenAssignment as ConfirmAssignment} from './ConfirmHiddenAssignment';
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this being used to confirm at hidden assignment or something else?

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh wait is this the old functionality that we will deprecate once the new features go out?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, AssignToSection will be deleted when the new feature goes out.

Copy link
Member

Choose a reason for hiding this comment

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

should this be imported as just {ConfirmHiddenAssignment} now?

Copy link
Contributor

@dmcavoy dmcavoy left a comment

Choose a reason for hiding this comment

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

Looks good! Just had one question

Copy link
Member

@davidsbailey davidsbailey left a comment

Choose a reason for hiding this comment

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

Great work @Erin007 !

@@ -8,7 +8,7 @@ import i18n from '@cdo/locale';
import Button from '@cdo/apps/templates/Button';
import DropdownButton from '@cdo/apps/templates/DropdownButton';
import BaseDialog from '@cdo/apps/templates/BaseDialog';
import ConfirmAssignment from './ConfirmAssignment';
import {ConfirmHiddenAssignment as ConfirmAssignment} from './ConfirmHiddenAssignment';
Copy link
Member

Choose a reason for hiding this comment

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

should this be imported as just {ConfirmHiddenAssignment} now?

And I press the first ".uitest-assign-button" element
And I wait until element ".uitest-confirm-assignment-dialog" is visible
And I wait until element "#confirm-assign" is visible
And I press the first "#confirm-assign" element
Copy link
Member

Choose a reason for hiding this comment

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

did the assignment work? the suspense is killing me!

can you please check that #confirm-assign is gone, and .uitest-unassign-button is visible?

Copy link

@clareconstantine clareconstantine left a comment

Choose a reason for hiding this comment

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

Nice work!

@Erin007 Erin007 merged commit 03b2d14 into staging Nov 18, 2019
@Erin007 Erin007 deleted the confirm-assignment-of-hidden-units branch November 18, 2019 23:53
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

4 participants