Fix the two most common LessonsServer errors and add tests. #6707
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
emilia-friedberg
approved these changes
Sep 9, 2020
emilia-friedberg
added a commit
that referenced
this pull request
Sep 9, 2020
* Remove references to prefilledText in all LMS apps (#6693) * remove prefill question * new test snapshot * Remove leftover unused write to Firebase (#6695) * try committing everything but yarn lock * put expose loader back because it turns out we do need that * minor c hange so I can deploy again * Add a "last_active" trait to Schools when syncing Vitally This trait is calculated by the last time a student completed an activity assigned by a teacher at this school * Remove an extraneous leftover line * dumb * add check for empty questions array (#6694) * Fix some caching issues in the CMS (#6697) * Fix some caching issues in the CMS * Break out of module to ensure Response called. * Be more specific with our `maximum` call to disambiguate * remove commented out loaders * Add analytics.group calls if current_user.school exists * Tweak tests to account for in-memory vs in-db time precision * Fix the two most common LessonsServer errors and add tests. (#6707) * Install jest. * Add tests for createSession * Fix error with teacher_ids (#6710) Co-authored-by: cissyxyu <57366100+cissyxyu@users.noreply.github.com> Co-authored-by: Anathomical <thomas.robertson@quill.org> Co-authored-by: Eric Adams <erictayloradams@gmail.com> Co-authored-by: dandrabik <dandrabik@users.noreply.github.com>
emilia-friedberg
added a commit
that referenced
this pull request
Sep 9, 2020
* Remove references to prefilledText in all LMS apps (#6693) * remove prefill question * new test snapshot * Remove leftover unused write to Firebase (#6695) * try committing everything but yarn lock * put expose loader back because it turns out we do need that * minor c hange so I can deploy again * Add a "last_active" trait to Schools when syncing Vitally This trait is calculated by the last time a student completed an activity assigned by a teacher at this school * Remove an extraneous leftover line * dumb * add check for empty questions array (#6694) * Fix some caching issues in the CMS (#6697) * Fix some caching issues in the CMS * Break out of module to ensure Response called. * Be more specific with our `maximum` call to disambiguate * remove commented out loaders * Add analytics.group calls if current_user.school exists * Tweak tests to account for in-memory vs in-db time precision * bunch of clean up in the webpack files * run terser plugin in parallel * start using the cache-loader for jsx and tsx files * tiny change to deploy again * get rid of cache-loader * Fix the two most common LessonsServer errors and add tests. (#6707) * Install jest. * Add tests for createSession * just remove sass-loader from evaluating .css files * Fix error with teacher_ids (#6710) * add the hard source webpack plugin * re-install terser webpack plugin * whitespace change to redeploy * get rid of stuff that really isn't helping Co-authored-by: cissyxyu <57366100+cissyxyu@users.noreply.github.com> Co-authored-by: Anathomical <thomas.robertson@quill.org> Co-authored-by: Eric Adams <erictayloradams@gmail.com> Co-authored-by: dandrabik <dandrabik@users.noreply.github.com>
emilia-friedberg
added a commit
that referenced
this pull request
Sep 9, 2020
* try committing changes * fix memory issue * memory update for prod-cms call * additional speed improvements for webpack (#6702) * Remove references to prefilledText in all LMS apps (#6693) * remove prefill question * new test snapshot * Remove leftover unused write to Firebase (#6695) * try committing everything but yarn lock * put expose loader back because it turns out we do need that * minor c hange so I can deploy again * Add a "last_active" trait to Schools when syncing Vitally This trait is calculated by the last time a student completed an activity assigned by a teacher at this school * Remove an extraneous leftover line * dumb * add check for empty questions array (#6694) * Fix some caching issues in the CMS (#6697) * Fix some caching issues in the CMS * Break out of module to ensure Response called. * Be more specific with our `maximum` call to disambiguate * remove commented out loaders * Add analytics.group calls if current_user.school exists * Tweak tests to account for in-memory vs in-db time precision * Fix the two most common LessonsServer errors and add tests. (#6707) * Install jest. * Add tests for createSession * Fix error with teacher_ids (#6710) Co-authored-by: cissyxyu <57366100+cissyxyu@users.noreply.github.com> Co-authored-by: Anathomical <thomas.robertson@quill.org> Co-authored-by: Eric Adams <erictayloradams@gmail.com> Co-authored-by: dandrabik <dandrabik@users.noreply.github.com> * final round of improvements to our current prod webpack build (#6713) * Remove references to prefilledText in all LMS apps (#6693) * remove prefill question * new test snapshot * Remove leftover unused write to Firebase (#6695) * try committing everything but yarn lock * put expose loader back because it turns out we do need that * minor c hange so I can deploy again * Add a "last_active" trait to Schools when syncing Vitally This trait is calculated by the last time a student completed an activity assigned by a teacher at this school * Remove an extraneous leftover line * dumb * add check for empty questions array (#6694) * Fix some caching issues in the CMS (#6697) * Fix some caching issues in the CMS * Break out of module to ensure Response called. * Be more specific with our `maximum` call to disambiguate * remove commented out loaders * Add analytics.group calls if current_user.school exists * Tweak tests to account for in-memory vs in-db time precision * bunch of clean up in the webpack files * run terser plugin in parallel * start using the cache-loader for jsx and tsx files * tiny change to deploy again * get rid of cache-loader * Fix the two most common LessonsServer errors and add tests. (#6707) * Install jest. * Add tests for createSession * just remove sass-loader from evaluating .css files * Fix error with teacher_ids (#6710) * add the hard source webpack plugin * re-install terser webpack plugin * whitespace change to redeploy * get rid of stuff that really isn't helping Co-authored-by: cissyxyu <57366100+cissyxyu@users.noreply.github.com> Co-authored-by: Anathomical <thomas.robertson@quill.org> Co-authored-by: Eric Adams <erictayloradams@gmail.com> Co-authored-by: dandrabik <dandrabik@users.noreply.github.com> Co-authored-by: cissyxyu <57366100+cissyxyu@users.noreply.github.com> Co-authored-by: Anathomical <thomas.robertson@quill.org> Co-authored-by: Eric Adams <erictayloradams@gmail.com> Co-authored-by: dandrabik <dandrabik@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
WHAT
Add a few tests to LessonsServer. Fix the two most frequently occurring nil errors.
https://sentry.io/organizations/quillorg-5s/issues/1723648326/?project=1876481&query=is%3Aunresolved
https://sentry.io/organizations/quillorg-5s/issues/1722612569/?project=1876481&query=is%3Aunresolved
WHY
Basically, this code section would error any time a session wasn't in RethinkDB yet. Adding tests to confirm my fix works.
HOW
Setup jest (a version that works with this node version). I made a smaller function that was more easily testable, and focused on that.
A few notes
UnhandledPromiseRejectionWarning:warning, but swallows the error. I tried some of the workarounds with no success, then moved on.exporting the "private function" since usingrewireand other test exposure methods for non-exported methods weren't working. I don't love this, but I'd rather have the tests than not. Open to otherScreenshots
(If applicable. Also, please censor any sensitive data)