-
Notifications
You must be signed in to change notification settings - Fork 1
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
Bug withdrawn application backend #1594
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
idk se på litt
|
||
# Core application fields |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does this even mean?
# Track original withdrawn state to detect changes | ||
self._original_withdrawn = self.withdrawn if self.pk else False |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why?
def _is_withdrawal(self) -> bool: | ||
""" | ||
Determines if this save operation represents a new withdrawal. | ||
Returns: | ||
bool: True if this is a new withdrawal, False otherwise | ||
""" | ||
return self.pk and self.withdrawn and not self._original_withdrawn |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Denne syntes jeg er rar. Er det ikke mulig å sammenlikne args med nåværende tilstand?
Hvordan er denne over 1000 linjer lang wut |
Hmmm ser ganske greit ut, men burde nok egt vært flere issues, ser ut som dette er en stor refactor av application og modulasering med _ functionene |
⭐ alot lines added are comments and doc strings! ;-) 400 lines are tests
👁️ seed script was viloating validation in the refactored RecruitmentApplication model
What
This PR refactors the RecruitmentApplication model. I found a bug which caused the applicants prioitiy of positions to "come out of sync". This PR fixes this bug by adding constraints to the RecruitmentApplication model, as well as refactors the save and clean methodes.
Why
First of all; when an application was withdrawn the "active applications" count was not updated. Therefor it as possible to set position priority on a scale of "all applications" including withdrawn applications.
This would cause the priority to not be deterministic. By refershing the page, a new query would be made which fethced the correct priority sequence, but by reprioritizing the priority sequence could come out of sync again.
New validation and helper methodes in RecruitmentApplication
RecruitmentApplication became extencive because I created helper methodes for almost everything. I also added some validation which should have been implemented before, like validation constrains for reprioritization deadline and priority deadline for recruitment admins.
The logic for update_applicant_state is unchanged.
get_total_interviews is unchanged
get_total_applications is unchanged
Views and tests
Views
Some of the views related to fetching applications for applicant where also refactored.
RecruitmentApplicationForApplicantView now only gives non-withdrawn applications. There is a view dedicated to getting withdrawn applications.
RecruitmentApplicationWithdrawApplicantView was extended with a get methode
RecruitmentApplicationApplicantPriorityView was refactored to work with the refactored RecruitmentApplication model better
Tests
Replaces backend part of #1542
closes #1535