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

Pilot (Student Libraries): Added error handling for when students upload a non-existant library #28789

Merged
merged 2 commits into from May 29, 2019

Conversation

jmkulwik
Copy link
Contributor

In the past, if students imported an invalid library ID, their project would be unusable. Now, an error message is displayed, and the library isn't imported.

studentLibrariesErrorUploadMessage

Spec for reference: https://docs.google.com/document/d/1jMLm_ghCshFen-h9vInAuXwEGXI-HPMjSP8dYeISfM0/edit?pli=1#

This code is largely experimental. If we go forward with Student Libraries, all code here will go through a second CR for production quality review. Review this PR to evaluate the following:

Isolation: Will the reviewed code affect parts of the product that are not part of the pilot?
Will the code fail in ways that will cause the pilot to be unsuccessful?
Is there an obviously easier or better way to implement the piloted feature?
Although the piloting process is still under review, I am following the procedures in this doc: https://docs.google.com/document/d/10QvRAVOTEenFJa1wwrY0Twkd0anTDpkkzNeGnT_HH0o/edit

@jmkulwik jmkulwik requested a review from ryansloan May 28, 2019 23:25
Copy link
Contributor

@ajpal ajpal left a comment

Choose a reason for hiding this comment

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

lgtm for pilot!

@jmkulwik
Copy link
Contributor Author

Tests passed with the previous commit. The only thing that changed between commit 1 and 2 was a minor styling detail. Because of this, I'm skipping the UI tests and completing the PR.

@jmkulwik jmkulwik merged commit 423000c into staging May 29, 2019
jmkulwik added a commit that referenced this pull request Jun 26, 2019
…empty-string"

This reverts commit 423000c, reversing
changes made to 215ef3c.
@jmkulwik jmkulwik deleted the libraries-import-empty-string branch February 20, 2020 01:34
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