Move students: success and error notifications #22190
Conversation
…sferStatus info on success
… entire transferStatus
| /> | ||
| ) | ||
| }, | ||
| { | ||
| name: 'Move students dialog', | ||
| description: 'Ability to move students in a certain section to a different section or teacher', | ||
| description: 'Dialog when an error has occurred', |
| sections={sections} | ||
| updateStudentTransfer={() => console.log('updating...')} | ||
| transferStudents={() => console.log('transferring...')} | ||
| cancelStudentTransfer={() => console.log('cancelling...')} |
There was a problem hiding this comment.
There's a storybook "actions" addon designed for stubbing callbacks like this, that logs to the onscreen console (not the developer console) and reports any arguments passed to the callback as well. Example:
code-dot-org/apps/src/lib/ui/ConfirmEnableMakerDialog.story.jsx
Lines 3 to 17 in 734199b
| @@ -51,6 +59,7 @@ describe('MoveStudents', () => { | |||
| {...DEFAULT_PROPS} | |||
| updateStudentTransfer={updateStudentTransfer} | |||
| transferStudents={transferStudents} | |||
| cancelStudentTransfer={cancelStudentTransfer} | |||
There was a problem hiding this comment.
You might want to compose DEFAULT_PROPS and the spy callbacks in your beforeEach function, since you pass them in every single test.
| ); | ||
|
|
||
| wrapper.find('Button').simulate('click'); | ||
| expect(cancelStudentTransfer.callCount).to.equal(0); |
There was a problem hiding this comment.
Tip: We've already got the sinon-chai assertions enabled so you can make assertions about spies and stubs like this:
expect(cancelStudentTransfer).not.to.have.been.called;
expect(cancelStudentTransfer).to.have.been.calledOnce;
|
A little suggestion you can feel free to do later or not take if it's already discussed in the spec. For these cases it feels like the error message would make more sense to have right under where you entered the section. |
|
@caleybrock re: error message placement, i think you have a good point. poorva and i discussed the other day and decided to put the error at the top of the modal since the error won't always have to do with the section input--it could be a more general failure (although it'll usually be the former, so it might make sense to move it at some point...) |
This is PR no. 4 for rewriting the 'move students' functionality in React. For context, check out my previous PR or the spec.
This implements success and error notification functionality. Because success notifications live in the parent component (
ManageStudentsTable), the code to render those notifications will be in the next (and final!) PR that hooks up the new 'move students' dialog to the 'manage students' tab.Errors, on the other hand, are rendered in the 'move students' dialog itself using error text returned from the server.
Examples