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

finalize user admission priorities for backend #987

Merged
merged 14 commits into from
Jun 16, 2024
1 change: 1 addition & 0 deletions backend/root/utils/routes.py
Original file line number Diff line number Diff line change
Expand Up @@ -505,6 +505,7 @@
samfundet__active_recruitments = 'samfundet:active_recruitments'
samfundet__recruitment_positions = 'samfundet:recruitment_positions'
samfundet__recruitment_positions_gang = 'samfundet:recruitment_positions_gang'
samfundet__recruitment_user_priority_update = 'samfundet:recruitment_user_priority_update'
samfundet__active_recruitment_positions = 'samfundet:active_recruitment_positions'
samfundet__applicants_without_interviews = 'samfundet:applicants_without_interviews/'
samfundet__occupied_timeslots = 'samfundet:occupied_timeslots'
Expand Down
51 changes: 51 additions & 0 deletions backend/samfundet/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,18 @@ def fixture_gang(fixture_organization: Organization) -> Iterator[Gang]:
organization.delete()


@pytest.fixture
def fixture_gang2(fixture_organization: Organization) -> Iterator[Gang]:
organization = Gang.objects.create(
name_nb='Gang 2',
name_en='Gang 2',
abbreviation='G2',
organization=fixture_organization,
)
yield organization
organization.delete()


@pytest.fixture
def fixture_text_item() -> Iterator[TextItem]:
text_item = TextItem.objects.create(
Expand Down Expand Up @@ -287,6 +299,26 @@ def fixture_recruitment_position(fixture_recruitment: Recruitment, fixture_gang:
recruitment_position.delete()


@pytest.fixture
def fixture_recruitment_position2(fixture_recruitment: Recruitment, fixture_gang2: Gang) -> Iterator[Recruitment]:
recruitment_position2 = RecruitmentPosition.objects.create(
name_nb='Position 2 NB',
name_en='Position 2 EN',
short_description_nb='Short Description NB',
short_description_en='Short Description EN',
long_description_nb='Long Description NB',
long_description_en='Long Description EN',
is_funksjonaer_position=False,
default_admission_letter_nb='Default Admission Letter NB',
default_admission_letter_en='Default Admission Letter EN',
tags='tag1,tag2',
gang=fixture_gang2,
recruitment=fixture_recruitment,
)
yield recruitment_position2
recruitment_position2.delete()


@pytest.fixture
def fixture_informationpage() -> Iterator[InformationPage]:
informationpage = InformationPage.objects.create(title_nb='Norsk tittel', title_en='Engel', slug_field='Sygard')
Expand Down Expand Up @@ -326,6 +358,25 @@ def fixture_recruitment_admission(
admission.delete()


@pytest.fixture
def fixture_recruitment_admission2(
fixture_user: User,
fixture_recruitment_position2: RecruitmentPosition,
fixture_recruitment: Recruitment,
) -> Iterator[RecruitmentAdmission]:
admission2 = RecruitmentAdmission.objects.create(
admission_text='Test admission text',
recruitment_position=fixture_recruitment_position2,
recruitment=fixture_recruitment,
user=fixture_user,
applicant_priority=2,
recruiter_priority=RecruitmentPriorityChoices.NOT_SET,
recruiter_status=RecruitmentStatusChoices.NOT_SET,
)
yield admission2
admission2.delete()


@pytest.fixture
def fixture_venue() -> Iterator[Venue]:
venue = Venue.objects.create(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
# Generated by Django 5.0.2 on 2024-06-07 09:52

from django.db import migrations, models


class Migration(migrations.Migration):
dependencies = [
("samfundet", "0014_gangsection"),
]

operations = [
migrations.AlterField(
model_name="recruitmentadmission",
name="applicant_priority",
field=models.PositiveIntegerField(
blank=True, help_text="The priority of the admission", null=True
),
),
]
45 changes: 41 additions & 4 deletions backend/samfundet/models/recruitment.py
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ class RecruitmentAdmission(CustomBaseModel):
)
recruitment = models.ForeignKey(Recruitment, on_delete=models.CASCADE, help_text='The recruitment that is recruiting', related_name='admissions')
user = models.ForeignKey(User, on_delete=models.CASCADE, help_text='The user that is applying', related_name='admissions')
applicant_priority = models.IntegerField(null=True, blank=True, help_text='The priority of the admission')
applicant_priority = models.PositiveIntegerField(null=True, blank=True, help_text='The priority of the admission')

created_at = models.DateTimeField(null=True, blank=True, auto_now_add=True)

Expand All @@ -195,6 +195,42 @@ class RecruitmentAdmission(CustomBaseModel):
choices=RecruitmentStatusChoices.choices, default=RecruitmentStatusChoices.NOT_SET, help_text='The status of the admission'
)

def organize_priorities(self) -> None:
emilte marked this conversation as resolved.
Show resolved Hide resolved
emilte marked this conversation as resolved.
Show resolved Hide resolved
"""Organizes priorites from 1 to n, so that it is sequential with no gaps"""
admissions_for_user = RecruitmentAdmission.objects.filter(recruitment=self.recruitment, user=self.user).order_by('applicant_priority')
for i in range(len(admissions_for_user)):
correct_position = i + 1
if admissions_for_user[i].applicant_priority != correct_position:
admissions_for_user[i].applicant_priority = correct_position
admissions_for_user[i].save()

def update_priority(self, direction: int) -> None:
magsyg marked this conversation as resolved.
Show resolved Hide resolved
"""
Method for moving priorites up or down,
positive direction indicates moving it to higher priority,
negative direction indicates moving it to lower priority,
can move n positions up or down

"""
# Use order for more simple an unified for direction
Copy link
Member

Choose a reason for hiding this comment

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

Skjønner ikke

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hva er det du ikke skjønner?
Får tak i alle admissions for bruker og recruitment

Copy link
Member

Choose a reason for hiding this comment

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

Mistenker at man kan gjøre dette enklere, men det avhenger av noen ting sikkert.
Er det validering på at man ikke kan ha samme priority, er vi sikret mot feil tilstand?

Copy link
Member

@emilte emilte Jun 13, 2024

Choose a reason for hiding this comment

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

Er høy eller lav prio best? Er det som rekkefølge slik at 1 er øverst?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Lav, er mer rangering

Copy link
Member

Choose a reason for hiding this comment

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

Gjerne svar på alle spørsmålene. Skal komme opp med et forslag, tror jeg har en god ide.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Den er sikret mot at gjennom bruk av denne, så kan ikke flere ha samme rangering, siden den vill bytte opp plassering og sortere.
Den PR er lagd for å organisere og omgjøre prioriteringer.
Den kan settes manuelt til samme, men den beskyttelsen vil være i en annen PR.

Copy link
Member

Choose a reason for hiding this comment

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

Så det er en db constraint på vei inn?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Det er ikke en DB constraint nei, det kan settes manuellt, og da sier ingenting at det ikke er mulig.
En DB constraint i dette tilfellet kan foresake en form for deadlock, hvor når den bytter om så vil det være et tilfelle 2 har samme verdi, og vil da fucke ting opp.

Copy link
Member

Choose a reason for hiding this comment

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

(Denne tråden var vi ikke ferdig med.)

Det er litt av poenget med constraint, garantere at ingen av dem har samme prio.

Jeg tror dette kan gjøres mye enklere.
Bare bulk flytt på settet du finner.

qs.filter(...).update(prio=F('prio')+1)
qs.filter(...).update(prio=F('prio')-1)

ordering = f"{'' if direction < 0 else '-' }applicant_priority"
admissions_for_user = RecruitmentAdmission.objects.filter(recruitment=self.recruitment, user=self.user).order_by(ordering)
direction = abs(direction) # convert to absolute
for i in range(len(admissions_for_user)):
if admissions_for_user[i].id == self.id: # find current
# Find index of which to switch priority with
switch = len(admissions_for_user) - 1 if i + direction >= len(admissions_for_user) else i + direction
new_priority = admissions_for_user[switch].applicant_priority
# Move priorites down in direction
for ii in range(switch, i, -1):
admissions_for_user[ii].applicant_priority = admissions_for_user[ii - 1].applicant_priority
admissions_for_user[ii].save()
# update priority
admissions_for_user[i].applicant_priority = new_priority
admissions_for_user[i].save()
break
self.organize_priorities()

def __str__(self) -> str:
return f'Admission: {self.user} for {self.recruitment_position} in {self.recruitment}'

Expand All @@ -205,12 +241,13 @@ def save(self, *args: tuple, **kwargs: dict) -> None: # noqa: C901
"""
if not self.recruitment:
self.recruitment = self.recruitment_position.recruitment
"""If the admission is saved without an interview, try to find an interview from a shared position."""
# If the admission is saved without an interview, try to find an interview from a shared position.
if not self.applicant_priority:
current_applications_count = RecruitmentAdmission.objects.filter(user=self.user).count()
self.organize_priorities()
current_applications_count = RecruitmentAdmission.objects.filter(user=self.user, recruitment=self.recruitment).count()
# Set the applicant_priority to the number of applications + 1 (for the current application)
self.applicant_priority = current_applications_count + 1
emilte marked this conversation as resolved.
Show resolved Hide resolved
"""If the admission is saved without an interview, try to find an interview from a shared position."""
# If the admission is saved without an interview, try to find an interview from a shared position.
if self.withdrawn:
self.recruiter_priority = RecruitmentPriorityChoices.NOT_WANTED
self.recruiter_status = RecruitmentStatusChoices.AUTOMATIC_REJECTION
Expand Down
58 changes: 58 additions & 0 deletions backend/samfundet/models/tests/test_recruitment.py
Original file line number Diff line number Diff line change
Expand Up @@ -184,3 +184,61 @@ def test_check_withdraw_sets_unwanted(self, fixture_recruitment_admission: Recru

assert fixture_recruitment_admission.recruiter_status == RecruitmentStatusChoices.AUTOMATIC_REJECTION
assert fixture_recruitment_admission.recruiter_priority == RecruitmentPriorityChoices.NOT_WANTED

def test_priority_up(self, fixture_recruitment_admission: RecruitmentAdmission, fixture_recruitment_admission2: RecruitmentAdmission):
assert fixture_recruitment_admission.applicant_priority == 1
assert fixture_recruitment_admission2.applicant_priority == 2

# Test general up
fixture_recruitment_admission2.update_priority(1)

assert RecruitmentAdmission.objects.get(id=fixture_recruitment_admission.id).applicant_priority == 2
assert RecruitmentAdmission.objects.get(id=fixture_recruitment_admission2.id).applicant_priority == 1

# Test up overloading
RecruitmentAdmission.objects.get(id=fixture_recruitment_admission.id).update_priority(2)

assert RecruitmentAdmission.objects.get(id=fixture_recruitment_admission.id).applicant_priority == 1
assert RecruitmentAdmission.objects.get(id=fixture_recruitment_admission2.id).applicant_priority == 2

# Test up from top position does not change anything
RecruitmentAdmission.objects.get(id=fixture_recruitment_admission.id).update_priority(1)

assert RecruitmentAdmission.objects.get(id=fixture_recruitment_admission.id).applicant_priority == 1
assert RecruitmentAdmission.objects.get(id=fixture_recruitment_admission2.id).applicant_priority == 2

def test_priority_down(self, fixture_recruitment_admission: RecruitmentAdmission, fixture_recruitment_admission2: RecruitmentAdmission):
# intial priority
assert fixture_recruitment_admission.applicant_priority == 1
assert fixture_recruitment_admission2.applicant_priority == 2

# Test general up
fixture_recruitment_admission.update_priority(-1)

assert RecruitmentAdmission.objects.get(id=fixture_recruitment_admission.id).applicant_priority == 2
assert RecruitmentAdmission.objects.get(id=fixture_recruitment_admission2.id).applicant_priority == 1

# Test up overloading
RecruitmentAdmission.objects.get(id=fixture_recruitment_admission2.id).update_priority(-2)

assert RecruitmentAdmission.objects.get(id=fixture_recruitment_admission.id).applicant_priority == 1
assert RecruitmentAdmission.objects.get(id=fixture_recruitment_admission2.id).applicant_priority == 2

# Test up from top position does not change anything
RecruitmentAdmission.objects.get(id=fixture_recruitment_admission2.id).update_priority(-1)

assert RecruitmentAdmission.objects.get(id=fixture_recruitment_admission.id).applicant_priority == 1
assert RecruitmentAdmission.objects.get(id=fixture_recruitment_admission2.id).applicant_priority == 2

def test_auto_newest_lowest_pri(self, fixture_recruitment_admission: RecruitmentAdmission, fixture_recruitment_position2: RecruitmentPosition):
"""Tests that the newest admission gets automatically the lowest applicant priority"""
# intial priority
assert fixture_recruitment_admission.applicant_priority == 1

new_admission = RecruitmentAdmission.objects.create(
admission_text='Test admission text 2',
recruitment_position=fixture_recruitment_position2,
recruitment=fixture_recruitment_position2.recruitment,
user=fixture_recruitment_admission.user,
)
assert new_admission.applicant_priority == 2
4 changes: 4 additions & 0 deletions backend/samfundet/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -569,6 +569,10 @@ class Meta:
fields = '__all__'


class RecruitmentUpdateUserPrioritySerializer(serializers.Serializer):
direction = serializers.IntegerField(label='direction', write_only=True)


class UserForRecruitmentSerializer(serializers.ModelSerializer):
recruitment_admission_ids = serializers.SerializerMethodField()

Expand Down
58 changes: 58 additions & 0 deletions backend/samfundet/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -920,3 +920,61 @@ def test_update_admission(
assert response.status_code == status.HTTP_200_OK
assert response.data['admission_text'] == post_data2['admission_text']
# Assert the returned data based on the logic in the view


def test_recruitment_admission_update_pri_up(
fixture_rest_client: APIClient,
fixture_user: User,
fixture_recruitment_admission: RecruitmentAdmission,
fixture_recruitment_admission2: RecruitmentAdmission,
):
### Arrange ###
fixture_rest_client.force_authenticate(user=fixture_user)
assert fixture_recruitment_admission.applicant_priority == 1
assert fixture_recruitment_admission2.applicant_priority == 2

url = reverse(
routes.samfundet__recruitment_user_priority_update,
kwargs={'pk': fixture_recruitment_admission2.id},
)

### Act ###
response: Response = fixture_rest_client.put(path=url, data={'direction': 1})

### Assert ###
assert response.status_code == status.HTTP_200_OK
# Assert the returned data based on the logic in the view
assert len(response.data) == 2
assert response.data[0]['id'] == str(fixture_recruitment_admission2.pk)
assert response.data[0]['applicant_priority'] == 1
assert response.data[1]['id'] == str(fixture_recruitment_admission.pk)
assert response.data[1]['applicant_priority'] == 2


def test_recruitment_admission_update_pri_down(
fixture_rest_client: APIClient,
fixture_user: User,
fixture_recruitment_admission: RecruitmentAdmission,
fixture_recruitment_admission2: RecruitmentAdmission,
):
### Arrange ###
fixture_rest_client.force_authenticate(user=fixture_user)
assert fixture_recruitment_admission.applicant_priority == 1
assert fixture_recruitment_admission2.applicant_priority == 2

url = reverse(
routes.samfundet__recruitment_user_priority_update,
kwargs={'pk': fixture_recruitment_admission.id},
)

### Act ###
response: Response = fixture_rest_client.put(path=url, data={'direction': -1})

### Assert ###
assert response.status_code == status.HTTP_200_OK
# Assert the returned data based on the logic in the view
assert len(response.data) == 2
assert response.data[0]['id'] == str(fixture_recruitment_admission2.pk)
assert response.data[0]['applicant_priority'] == 1
assert response.data[1]['id'] == str(fixture_recruitment_admission.pk)
assert response.data[1]['applicant_priority'] == 2
1 change: 1 addition & 0 deletions backend/samfundet/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@
path('active-recruitments/', views.ActiveRecruitmentsView.as_view(), name='active_recruitments'),
path('recruitment-positions/', views.RecruitmentPositionsPerRecruitmentView.as_view(), name='recruitment_positions'),
path('recruitment-positions-gang/', views.RecruitmentPositionsPerGangView.as_view(), name='recruitment_positions_gang'),
path('recruitment-user-priority-update/<slug:pk>/', views.RecruitmentAdmissionApplicantPriorityView.as_view(), name='recruitment_user_priority_update'),
path('active-recruitment-positions/', views.ActiveRecruitmentPositionsView.as_view(), name='active_recruitment_positions'),
path('applicants-without-interviews/', views.ApplicantsWithoutInterviewsView.as_view(), name='applicants_without_interviews/'),
path('occupiedtimeslot/', views.OccupiedtimeslotView.as_view(), name='occupied_timeslots'),
Expand Down
33 changes: 33 additions & 0 deletions backend/samfundet/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@
RecruitmentPositionSerializer,
RecruitmentStatisticsSerializer,
RecruitmentAdmissionForGangSerializer,
RecruitmentUpdateUserPrioritySerializer,
RecruitmentAdmissionForApplicantSerializer,
)
from .models.event import Event, EventGroup
Expand Down Expand Up @@ -708,6 +709,38 @@ def list(self, request: Request) -> Response:
return Response(serializer.data)


class RecruitmentAdmissionApplicantPriorityView(APIView):
permission_classes = [IsAuthenticated]
serializer_class = RecruitmentUpdateUserPrioritySerializer
emilte marked this conversation as resolved.
Show resolved Hide resolved

def put(
self,
request: Request,
pk: int,
) -> Response:
direction = RecruitmentUpdateUserPrioritySerializer(data=request.data)
if direction.is_valid():
robines marked this conversation as resolved.
Show resolved Hide resolved
direction = direction.validated_data['direction']
else:
return Response(direction.errors, status=status.HTTP_400_BAD_REQUEST)

# Dont think we need any extra perms in this view, admin should not be able to change priority
admission = get_object_or_404(
RecruitmentAdmission,
id=pk,
user=request.user,
)
admission.update_priority(direction)
serializer = RecruitmentAdmissionForApplicantSerializer(
RecruitmentAdmission.objects.filter(
recruitment=admission.recruitment,
user=request.user,
).order_by('applicant_priority'),
many=True,
)
return Response(serializer.data)


class RecruitmentAdmissionForGangView(ModelViewSet):
permission_classes = [IsAuthenticated]
serializer_class = RecruitmentAdmissionForGangSerializer
Expand Down
Loading
Loading