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

[WIP] Implement missing API functions #8

Closed
wants to merge 16 commits into from
Closed
Show file tree
Hide file tree
Changes from 10 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
119 changes: 93 additions & 26 deletions src/personio_py/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ def request(self, path: str, method='GET', params: Dict[str, Any] = None,
_headers.update(headers)
# make the request
url = urljoin(self.base_url, path)
response = requests.request(method, url, headers=_headers, params=params, data=data)
response = requests.request(method, url, headers=_headers, params=params, json=data)
# re-new the authorization header
authorization = response.headers.get('Authorization')
if authorization:
Expand All @@ -127,8 +127,7 @@ def request_json(self, path: str, method='GET', params: Dict[str, Any] = None,
during this request (default: True for json requests)
:return: the parsed json response, when the request was successful, or a PersonioApiError
"""
headers = {'accept': 'application/json'}
response = self.request(path, method, params, data, headers, auth_rotation=auth_rotation)
response = self.request(path, method, params, data, auth_rotation=auth_rotation)
if response.ok:
try:
return response.json()
Expand Down Expand Up @@ -269,8 +268,7 @@ def update_employee(self, employee: Employee):
"""
placeholder; not ready to be used
"""
# TODO implement
pass
raise NotImplementedError()

def get_attendances(self, employees: Union[int, List[int], Employee, List[Employee]],
start_date: datetime = None, end_date: datetime = None) -> List[Attendance]:
Expand All @@ -291,28 +289,94 @@ def get_attendances(self, employees: Union[int, List[int], Employee, List[Employ
return self._get_employee_metadata(
'company/attendances', Attendance, employees, start_date, end_date)

def create_attendances(self, attendances: List[Attendance]):
def create_attendances(self, attendances: List[Attendance]) -> bool:
"""
placeholder; not ready to be used
Create all given attendance records.

Note: Attendances are created sequentially. This function stops on first error.
All attendance records before the error will be created, all records after the error will be skipped.
Comment on lines +296 to +297
Copy link
Contributor

Choose a reason for hiding this comment

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

were you able to verify that this works? The documentation does not indicate what happens when there are errors in only some of the attendance records.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is not documented but I verified this using a personio test account

Copy link
Contributor

Choose a reason for hiding this comment

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

in this case, maybe we should tell the user which attendances have been created and which have failed? We could return a dictionary like

{
  'success': [list of attendance objects that were created],
  'failure': [list of attendance objects that were not created],
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I only did a very quick test of this behavior, but IIRC the response does not contain the information where the error happened.


:param attendances: A list attendance records to be created.
"""
# attendances can be created individually, but here you can push a huge bunch of items
# in a single request, which can be significantly faster
# TODO implement
pass
data_to_send = [attendance.to_body_params(patch_existing_attendance=False) for attendance in attendances]
response = self.request_json(path='company/attendances', method='POST', data={"attendances": data_to_send})
if response['success']:
for i in range(len(attendances)):
attendances[i].id_ = response['data']['id'][i]
attendances[i].set_client(self)
return True
return False

def update_attendance(self, attendance_id: int):
def update_attendance(self, attendance: Attendance, remote_query_id=False):
"""
placeholder; not ready to be used
Update an existing attendance record

Either an attendance id or o remote query is required. Remote queries are only executed if required.
An Attendance object returned by get_attendances() include the attendance id. DO NOT SET THE ID YOURSELF.

:param attendance: The Attendance object holding the new data.
:param remote_query_id: Allow a remote query for the id if it is not set within the given Attendance object.
:raises:
ValueError: If a query is required but not allowed or the query does not provide exactly one result.
"""
# TODO implement
pass
if attendance.id_ is not None:
# remote query not necessary
response = self.request_json(path='company/attendances/' + str(attendance.id_), method='PATCH',
data=attendance.to_body_params(patch_existing_attendance=True))
return response
else:
if remote_query_id:
attendance = self.__add_remote_attendance_id(attendance)
self.update_attendance(attendance)
else:
raise ValueError("You either need to provide the attendance id or allow a remote query.")

def delete_attendance(self, attendance_id: int):
def delete_attendance(self, attendance: Attendance or int, remote_query_id=False):
"""
placeholder; not ready to be used
Delete an existing record

Either an attendance id or o remote query is required. Remote queries are only executed if required.
An Attendance object returned by get_attendances() include the attendance id. DO NOT SET THE ID YOURSELF.

:param attendance: The Attendance object holding the new data or an attendance record id to delete.
:param remote_query_id: Allow a remote query for the id if it is not set within the given Attendance object.
:raises:
ValueError: If a query is required but not allowed or the query does not provide exactly one result.
"""
# TODO implement
pass
if isinstance(attendance, int):
response = self.request_json(path='company/attendances/' + str(attendance), method='DELETE')
return response
elif isinstance(attendance, Attendance):
if attendance.id_ is not None:
return self.delete_attendance(attendance.id_)
else:
if remote_query_id:
attendance = self.__add_remote_attendance_id(attendance)
self.delete_attendance(attendance.id_)
else:
raise ValueError("You either need to provide the attendance id or allow a remote query.")
else:
raise ValueError("attendance must be an Attendance object or an integer")

def __add_remote_attendance_id(self, attendance: Attendance) -> Attendance:
"""
Queries the API for an attendance record matching the given Attendance object and adds the remote id.

:param attendance: The attendance object to be updated
:return: The attendance object with the attendance_id set
"""
if attendance.employee_id is None:
raise ValueError("For a remote query an employee_id is required")
if attendance.date is None:
raise ValueError("For a remote query a date is required")
matching_remote_attendances = self.get_attendances(employees=[attendance.employee_id],
start_date=attendance.date, end_date=attendance.date)
if len(matching_remote_attendances) == 0:
raise ValueError("The attendance to patch was not found")
elif len(matching_remote_attendances) > 1:
raise ValueError("More than one attendance found.")
attendance.id_ = matching_remote_attendances[0].id_
return attendance

def get_absence_types(self) -> List[AbsenceType]:
"""
Expand Down Expand Up @@ -348,24 +412,27 @@ def get_absences(self, employees: Union[int, List[int], Employee, List[Employee]

def get_absence(self, absence_id: int) -> Absence:
"""
placeholder; not ready to be used
Get an absence record from a given id.

:param absence_id: The absence id to fetch.
"""
# TODO implement
pass
response = self.request_json('company/time-offs/' + str(absence_id))
return Absence.from_dict(response['data'], self)

def create_absence(self, absence: Absence):
"""
placeholder; not ready to be used
"""
# TODO implement
pass

data = absence.to_body_params()
response = self.request_json('company/time-offs', method='POST', data=data)
return response

def delete_absence(self, absence_id: int):
"""
placeholder; not ready to be used
"""
# TODO implement
pass
raise NotImplementedError()

def _get_employee_metadata(
self, path: str, resource_cls: Type[PersonioResourceType],
Expand Down
62 changes: 57 additions & 5 deletions src/personio_py/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -289,6 +289,9 @@ def _check_client(self, client: 'Personio' = None) -> 'Personio':
client.authenticate()
return client

def set_client(self, client: 'Personio'):
self._client = client

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure if there is a need for a setter here, this looks like java code to me ^^

We can change create_attendances() so that this is not needed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Solution proposal: I could change the objects client to be public accessible


class LabeledAttributesMixin(PersonioResource):
"""
Expand Down Expand Up @@ -602,6 +605,19 @@ def _create(self, client: 'Personio'):
def _delete(self, client: 'Personio'):
pass

def to_body_params(self):
data = {
'empolyee_id': self.employee.id_,
'time_off_type_id': self.time_off_type.id_,
'start_date': self.start_date,
'end_date': self.end_date,
'half_day_start': self.half_day_start,
'half_day_end': self.half_day_end
}
if self.comment is not None:
data['comment'] = self.comment
return data


class Attendance(WritablePersonioResource):

Expand Down Expand Up @@ -652,13 +668,49 @@ def to_dict(self, nested=False) -> Dict[str, Any]:
return d

def _create(self, client: 'Personio'):
pass
self._client.create_attendances([self])
Copy link
Contributor

Choose a reason for hiding this comment

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

I left this part unfinished... client should have been an optional parameter and the function should choose between client and self._client or raise an error if both are None. I should probably fix this myself ^^


def _update(self, client: 'Personio'):
pass
def _update(self, client: 'Personio', allow_remote_query: bool = False):
self._client.update_attendance(self, remote_query_id=allow_remote_query)

def _delete(self, client: 'Personio'):
pass
def _delete(self, client: 'Personio', allow_remote_query: bool = False):
self._client.delete_attendance(self, remote_query_id=allow_remote_query)

def to_body_params(self, patch_existing_attendance=False):
"""
Return the Attendance object in the representation expected by the Personio API

For an attendance record to be created all_values_required needs to be True.
For patch operations only the attendance id is required, but it is not
included into the body params.

:param patch_existing_attendance Get patch body. If False a create body is returned.
"""
if patch_existing_attendance:
if self.id_ is None:
raise ValueError("An attendance id is required")
body_dict = {}
if self.date is not None:
body_dict['date'] = self.date.strftime("%Y-%m-%d")
if self.start_time is not None:
body_dict['start_time'] = self.start_time
if self.end_time is not None:
body_dict['end_time'] = self.end_time
if self.break_duration is not None:
body_dict['break'] = self.break_duration
if self.comment is not None:
body_dict['comment'] = self.comment
return body_dict
else:
return \
{
"employee": self.employee_id,
"date": self.date.strftime("%Y-%m-%d"),
"start_time": self.start_time,
"end_time": self.end_time,
"break": self.break_duration,
"comment": self.comment
}


class Employee(WritablePersonioResource, LabeledAttributesMixin):
Expand Down