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

Delete Invite #1147

Merged
merged 8 commits into from Apr 12, 2022
Merged

Delete Invite #1147

merged 8 commits into from Apr 12, 2022

Conversation

ghost
Copy link

@ghost ghost commented Apr 8, 2022

This patch deletes invites that are older than a week.

Fixes #1137.

  • Tests passing
  • Black formatting
  • Migrations for any changes to the database schema
  • Rebase/merge the dev branch
  • Note in the CHANGELOG

This patch deletes invites that are older than a week.

Signed-off-by: Zishan Mirza <[email protected]>
@codecov
Copy link

codecov bot commented Apr 8, 2022

Codecov Report

Merging #1147 (2c67678) into dev (3de9c27) will decrease coverage by 0.10%.
The diff coverage is 70.96%.

@@            Coverage Diff             @@
##              dev    #1147      +/-   ##
==========================================
- Coverage   82.73%   82.63%   -0.11%     
==========================================
  Files          29       29              
  Lines        3395     3426      +31     
==========================================
+ Hits         2809     2831      +22     
- Misses        586      595       +9     
Impacted Files Coverage Δ
dds_web/scheduled_tasks.py 24.77% <70.00%> (+16.34%) ⬆️
dds_web/database/models.py 94.28% <100.00%> (+0.01%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3de9c27...2c67678. Read the comment docs.

Copy link
Member

@i-oden i-oden left a comment

Choose a reason for hiding this comment

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

Looks good, a few comments though.
Apart from this, could you also keep track of the non-deleted invites in a similar way as the other scheduled tasks? So have a dict that you put the invite errors in and afterward print them out. If there's an issue with one of the invite deletions it should still be possible to delete the others.

So there should be 2 try-except blocks, one outside of the for loop and one per commit.

dds_web/scheduled_tasks.py Outdated Show resolved Hide resolved
dds_web/scheduled_tasks.py Outdated Show resolved Hide resolved
This patch updates the "if" statement, adds a "try" statement, and
includes "errors".

Signed-off-by: Zishan Mirza <[email protected]>
@ghost
Copy link
Author

ghost commented Apr 11, 2022

I have updated the if statement, added a try statement, and included errors.

@i-oden i-oden assigned ghost Apr 11, 2022
@i-oden i-oden added must have Cannot deliver on target date without this backend labels Apr 11, 2022
dds_web/scheduled_tasks.py Outdated Show resolved Hide resolved
This patch updates "expiration".

Signed-off-by: Zishan Mirza <[email protected]>
@ghost
Copy link
Author

ghost commented Apr 11, 2022

I have updated expiration.

This patch adds a unit test.

Signed-off-by: Zishan Mirza <[email protected]>
Copy link
Member

@i-oden i-oden left a comment

Choose a reason for hiding this comment

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

There was one tiny typo and also we need to save the error message and print that out. Also shouldn't raise in the inner try except, otherwise it will not continue with the next invite.

dds_web/scheduled_tasks.py Show resolved Hide resolved
dds_web/scheduled_tasks.py Outdated Show resolved Hide resolved
dds_web/scheduled_tasks.py Outdated Show resolved Hide resolved
dds_web/scheduled_tasks.py Outdated Show resolved Hide resolved
This patch updates the "delete_invite" function.

Signed-off-by: Zishan Mirza <[email protected]>
Copy link
Member

@i-oden i-oden left a comment

Choose a reason for hiding this comment

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

Looks good!

@i-oden i-oden merged commit 5e18c54 into ScilifelabDataCentre:dev Apr 12, 2022
@ghost ghost deleted the delete_invite branch May 24, 2022 09:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend must have Cannot deliver on target date without this
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cronjob: Clean up invites which are not answered after a week
1 participant