HR data ingest #593
Draft
HR data ingest #593
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
SamDudley
commented
Jan 10, 2025
payroll/services/ingest.py
Outdated
Comment on lines
75
to
81
| def hr_row_to_employee(hr_row: HrRow) -> Employee: | ||
| employee = Employee() | ||
| employee_fields = vars(employee) | ||
| matching_fields = [field for field in hr_row._fields if field in employee_fields] | ||
| for field in matching_fields: | ||
| setattr(employee, field, getattr(hr_row, field)) | ||
| return employee |
There was a problem hiding this comment.
vars is the wrong approach as it returns all the underlying class attributes and we wouldn't want a field to match on accident.
in django you can get the fields of a model through the Model._meta.get_fields() API
https://docs.djangoproject.com/en/5.1/ref/models/meta/#retrieving-all-field-instances-of-a-model
however, in this case I think we should avoid iteration for the following reasons:
- binds the
HrRowfield names closely to theEmployeefieldnames (this isn't that bad) - no opportunity to validate/parse/convert the values
- all fields will be attempted where an explicit list of which to update is safer if the csv data/model changes
i think the better approach would be to build up the Employee model in a procedural way and validate data as we go along
emp = Employee()
# no need to worry about the field names not matching
emp.cost_centre_id = row.cost_centre_code
# now we can validate the data
emp.fte = float(row.fte)
...
|
Great work on this! Let's chat about my comments offline :) |
sam has made a mess of the code, sorry! haha
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.