Skip to content
This repository has been archived by the owner on May 13, 2024. It is now read-only.

Added Employment change model, refactoring working calculation #981

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
114 changes: 66 additions & 48 deletions timed/employment/models.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
"""Models for the employment app."""

from datetime import date, timedelta
from turtle import mode
Copy link
Contributor

Choose a reason for hiding this comment

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

This import seem unused


from dateutil import rrule
from django.conf import settings
Expand All @@ -13,7 +14,7 @@
from timed.models import WeekdaysField
from timed.projects.models import CustomerAssignee, ProjectAssignee, TaskAssignee
from timed.tracking.models import Absence

from timed.employment.scheduls import get_schedule
Copy link
Contributor

Choose a reason for hiding this comment

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

There's a typo in scheduls, should be schedules

also in the filename of timed/employment/schedules.py


class Location(models.Model):
"""Location model.
Expand Down Expand Up @@ -259,54 +260,11 @@ def calculate_worktime(self, start, end):
:returns: tuple of 3 values reported, expected and delta in given
time frame
"""
Copy link
Contributor

Choose a reason for hiding this comment

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

The docstring does not seem to fit the function anymore since you moved the code to another function that is called by it.

I wonder if it would be sufficient to create the part for the employment.

But I'm not familiar enough with the model situation here e. g. why the calculation is bound to the Location model in the first place..

from timed.tracking.models import Absence, Report

# shorten time frame to employment
start = max(start, self.start_date)
end = min(self.end_date or date.today(), end)

# workdays is in isoweekday, byweekday expects Monday to be zero
week_workdays = [int(day) - 1 for day in self.location.workdays]
workdays = rrule.rrule(
rrule.DAILY, dtstart=start, until=end, byweekday=week_workdays
).count()

# converting workdays as db expects 1 (Sunday) to 7 (Saturday)
workdays_db = [
# special case for Sunday
int(day) == 7 and 1 or int(day) + 1
for day in self.location.workdays
]
holidays = PublicHoliday.objects.filter(
location=self.location,
date__gte=start,
date__lte=end,
date__week_day__in=workdays_db,
).count()

expected_worktime = self.worktime_per_day * (workdays - holidays)

overtime_credit_data = OvertimeCredit.objects.filter(
user=self.user_id, date__gte=start, date__lte=end
).aggregate(total_duration=Sum("duration"))
overtime_credit = overtime_credit_data["total_duration"] or timedelta()

reported_worktime_data = Report.objects.filter(
user=self.user_id, date__gte=start, date__lte=end
).aggregate(duration_total=Sum("duration"))
reported_worktime = reported_worktime_data["duration_total"] or timedelta()

absences = sum(
[
absence.calculate_duration(self)
for absence in Absence.objects.filter(
user=self.user_id, date__gte=start, date__lte=end
).select_related("absence_type")
],
timedelta(),
)

reported = reported_worktime + absences + overtime_credit
schedule = get_schedule(start,end,self.employment)

expected_worktime = self.worktime_per_day * (schedule(0) - schedule(1))
reported = schedule(3) + schedule(4) + schedule(2)
Comment on lines +266 to +267
Copy link
Contributor

Choose a reason for hiding this comment

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

get_schedule does not return a function. So I assume these calls are actually meant to be lookups schedule[0] etc.
see my comment regarding the named tuples, though.


return (reported, expected_worktime, reported - expected_worktime)

Expand Down Expand Up @@ -414,3 +372,63 @@ def get_active_employment(self):
return current_employment
except Employment.DoesNotExist:
return None

class EmploymentChange(models.Model):

""" Employment working time percentage change, It can be
1- Increasing the working time percentage
2- Decreasing the working time percentage
"""

change_choices = [
('increase','Increase'),
('decrease','Decrease')
]

employment = models.ForeignKey(Employment,on_delete=models.PROTECT,related_name="employment_change")
start_date = models.DateField()
end_date = models.DateField()
change_type = models.CharField(max_length=15, choices=change_choices)
change_percentage = models.FloatField()



def calculate_employment_change(self, start, end):
"""
Calculate employment working time change
"""

#Get employment schedule
schedule = get_schedule(start,end,self.employment)

# new working time percentage
if self.change_type == 'increase':
new_percentage = self.employment.percentage + self.change_percentage
else:
new_percentage = self.employment.percentage - self.change_percentage

new_worktime = (self.employment.worktime_per_day * new_percentage)/100
expected_worktime = new_worktime * (schedule(0) - schedule(1))
reported = schedule(3) + schedule(4) + schedule(2)

return (reported, expected_worktime, reported - expected_worktime)




















70 changes: 70 additions & 0 deletions timed/employment/scheduls.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
from datetime import date, timedelta
from dateutil import rrule
from django.db.models import Sum
from typing import Union

from timed.employment.models import Employment, EmploymentChange, PublicHoliday,OvertimeCredit
from timed.tracking.models import Absence, Report


def get_schedule(start,end,employment: Union[Employment, EmploymentChange]):

"""
Obtaining weekly working days, holidays, overtime credit,
reported working time and absences

:params start: working starting time on a given day
:params end : working end time of a given day
:employment : Employement or EmploymentChange object
"""
Comment on lines +10 to +19
Copy link
Contributor

Choose a reason for hiding this comment

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

get_schedule makes me expect some planned or scheduled entity. however the actually returned object is more of a summary or report on actually accounted for time spent and expected workload taking into account spent hours, scheduled workdays and days off..

so the original calculate_worktime or estimate_worktime or even estimate_timeframed_workload that would make reference to the parameters were somehow more descriptive.


if isinstance(employment, Employment):
location = employment.location
user_id = employment.user.id
elif isinstance(employment, EmploymentChange):
location = employment.employment.location
user_id = employment.employment.user.id

# shorten time frame to employment
start = max(start, employment.start_date)
end = min(employment.end_date or date.today(), end)

Copy link
Contributor

Choose a reason for hiding this comment

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

why did you remove the comment?
# workdays is in isoweekday, byweekday expects Monday to be zero

week_workdays = [int(day) - 1 for day in employment.location.workdays]
workdays = rrule.rrule(
rrule.DAILY, dtstart=start, until=end, byweekday=week_workdays
).count()

# converting workdays as db expects 1 (Sunday) to 7 (Saturday)
workdays_db = [
# special case for Sunday
int(day) == 7 and 1 or int(day) + 1
for day in location.workdays
]
holidays = PublicHoliday.objects.filter(
location=location,
date__gte=start,
date__lte=end,
date__week_day__in=workdays_db,
).count()

overtime_credit_data = OvertimeCredit.objects.filter(
user=user_id, date__gte=start, date__lte=end
).aggregate(total_duration=Sum("duration"))
overtime_credit = overtime_credit_data["total_duration"] or timedelta()

reported_worktime_data = Report.objects.filter(
user=user_id, date__gte=start, date__lte=end
).aggregate(duration_total=Sum("duration"))
reported_worktime = reported_worktime_data["duration_total"] or timedelta()

absences = sum(
[
absence.calculate_duration(employment)
for absence in Absence.objects.filter(
user=user_id, date__gte=start, date__lte=end
).select_related("absence_type")
],
timedelta(),
)

return (workdays,holidays,overtime_credit,reported_worktime,absences)
Copy link
Contributor

Choose a reason for hiding this comment

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

While the return value is perfectly fine in terms of correctness it could be nice for a different developer (or even the same far enough in the future ;) to get some datastructure back that documents it's elements meaning like a namedtuple or something.

Workload is just an example name..

Workload = namedtuple('Workload', ["workdays", "holidays", "overtime_credit", "reported_worktime", "absenses"])

This could also be used in the function's definition of the return value -> Workload

That would then be much more informative when dealing with the function's output.

output.workdays rather than output[0]