Skip to content

Conversation

@Snorre98
Copy link
Contributor

@Snorre98 Snorre98 commented Feb 14, 2025

💡 replaced this:#1594

There was a bug which made applicant priority of positions incorrect. This is because when priority is manipulated the total count of applications was including withdrawn applications.

This was the case both when a new application was created, but also when applications where withdrawn.

There also is a bug where the priority is set incorrectly in the scenario when the applicant applies for a position which they previously had withdrawn the application from (could also be described as "re-activating" the application). The application gets the priority it had when it was withdrawn, which seems like unwanted behavior. I think the behavior should be that the "re-activated" application should get the lowest priority, like when the applicant applies for a position for the first time.

@Snorre98 Snorre98 linked an issue Feb 14, 2025 that may be closed by this pull request
1 task
@Snorre98 Snorre98 self-assigned this Feb 14, 2025
@Snorre98 Snorre98 marked this pull request as ready for review February 14, 2025 21:08
@Snorre98 Snorre98 added the help wanted Extra attention is needed. label Feb 14, 2025
@Snorre98 Snorre98 changed the title makes sure that only "active" applications are considered when priori… Application priority bug Feb 14, 2025
@nemisis84 nemisis84 self-requested a review February 25, 2025 18:55
Copy link
Contributor

@nemisis84 nemisis84 left a comment

Choose a reason for hiding this comment

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

Haven't taken a deep look into this., but leaving a comment anyway The algorithm itself is probably fine and it seems to work in the browser.

The test should be covering and documenting the edge cases. However, I think all tests should have a docstring describing the test case.

@Snorre98 Snorre98 requested a review from nemisis84 February 25, 2025 23:51
Copy link
Contributor

@nemisis84 nemisis84 left a comment

Choose a reason for hiding this comment

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

Only approving because I have to. But nice that you added docstrings

@Snorre98 Snorre98 merged commit 1ae2b5f into master Mar 20, 2025
5 checks passed
@Snorre98 Snorre98 deleted the bug-withdrawn-application-backend-2 branch March 20, 2025 18:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

help wanted Extra attention is needed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] withdrawn application showing up for applicatn

4 participants