-
Notifications
You must be signed in to change notification settings - Fork 381
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
Delete notifications older than 90 days #1328
Delete notifications older than 90 days #1328
Conversation
Kudos, SonarCloud Quality Gate passed!
|
@rithviknishad , @vigneshhari |
Can you create a test for this as well? Once you have the tests added we can merge. |
Yes, added the tests. Please check |
You could have gotten away without mocking in these tests by using the update method of the django queryset ( they do not invoke the save methods and the created timestamps can be added manually "to my knowledge" ), even in this case, we are not mocking the database interactions, we are just mocking the time method. |
Yaa ik that i haven't mocked any database interactions in the pushed code . I just asked it coz I first wrote the test by mocking database interactions, later i realised that isn't a gud idea. So i changed it. Thanks for sharing ur opinion. 😄 |
Proposed Changes
Associated Issue
Architecture changes
@coronasafe/code-reviewers
Merge Checklist
/docs