Skip to content

Conversation

@anandmatt
Copy link

Description of the Change

Cron job that was supposed to delete old applications had no implementation. Changes done to call this cron job to delete old applications that were turned down, 7 days after a role has been appointed and saved.

Applicable Issues

Fixes issue #786

feature

@anandmatt anandmatt linked an issue Nov 30, 2023 that may be closed by this pull request
@robindymer robindymer self-requested a review December 6, 2023 17:38
@ludvigalden ludvigalden requested review from ludvigalden and removed request for robindymer January 26, 2024 17:01
Copy link
Contributor

@ludvigalden ludvigalden left a comment

Choose a reason for hiding this comment

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

Any idea why the CRON-job might not be invoked by django-kronos, @anandmatt?

).exclude(
status='appointed'
).filter(
status='turned_down'
Copy link
Contributor

Choose a reason for hiding this comment

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

The change to remove only turned_down applications is good, because currently, it expects that applications are appointed within a week of the application end date, which is not always the case. The status is set to turned_down only when they have been appointed.

if not created:
appl.status = 'appointed'
appl.save()
cron.remove_old_applications()
Copy link
Contributor

Choose a reason for hiding this comment

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

So a CRON-job is not supposed to be invoked manually, by definition. Unless there is some issue with django-kronos not installing the CRON-jobs correctly, I don't think there is any problem in terms of remove_old_applications not being called. The crontab on the server looks fine. So I think the change to invoke it without django-kronos should be removed.

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.

Delete old applications

4 participants