-
Notifications
You must be signed in to change notification settings - Fork 30
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
Backend Release to Main [1.80] #995
Merged
Merged
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
Added four unit tests covering one positive case and three negative cases. Moved userId error handling to higher level in controller since it makes more sense to check if userId is not present before anything else.
Added 2 negative cases to the unit test suite. Refactored 'htmlContentSanitizer' file because I was getting module error. Also moved up the initial userId check in the notification controller.
Added Positive case 200 for getUserNotifications. Furthermore, I updated the docs for the negative and postive cases. Still need to add one last negative to complete the suite.
Added negative case for getUserNotifications to check 'Internal Error' shows if there is an error in fetching process. Furthermore, I updated the docs for the negative case. Still need refactor the test suite. There is still a lot of repetition in my code
…ations Added all four test cases for getUnreadUserNotifications.This test suite is essentially the same as getUserNotifications. The controller bahaves the same way. Additionally, updated the docs for the test cases added in the suite.
Added four additional tests for getSentNotifications inside controller. I found a bug while working on this test. Because of how logical operators work in JavaScript, the logic that was set up inside the method would always return false. This is not intended behavior so I corrected it by using '&&' instead of '||'. This is on line 81 of the 'notificationsController'
Added two negative test cases that are for error handling. Came across the same bug again where inside the controller the operator '||' would make the method always throw a forbidden error. Changed the operator to '&&' which fixed this issue.
Added remaining test cases and updated requirements docs. Still need to add 'deleteUserNotifications' and 'markNotificationAsRead'. Aiming to complete them tonight
…uites Added the remaining tests for marking notification as read and deleting notification. Also, fix more bugs in the controller with the operator not evaluating the function correctly. Next steps are to refactor my tests and trim repeated code
Made use of Diegos 'assertResMock' helper function to slim down some of the tests.
…nController_unit_tests Jordy add notification controller unit tests
Anirudh - Fetch end date field from dashboardhelper
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.
Includes: Jordy add notification controller unit tests #925