feat: add current workforce endpoint#31
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a typed client method and response/request models for the Ergani current workforce service endpoint.
Changes:
- Added
get_current_workforce()to execute serviceEX_BASE_05. - Added current workforce request/record models and parsing helpers.
- Added unit tests for serialization, parsing, and client service invocation.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
ergani/client.py |
Adds the current workforce client endpoint. |
ergani/models.py |
Adds request/record models and response parsing for current workforce data. |
tests/test_current_workforce.py |
Adds tests covering request serialization, response parsing, and client behavior. |
Comments suppressed due to low confidence (5)
tests/test_current_workforce.py:169
- Existing unittest tests in this repository consistently annotate test methods with
-> None; this new test method omits that return annotation, which makes the file inconsistent with the established test style.
def test_current_workforce_record_parse_many_reads_wrapped_response(self):
tests/test_current_workforce.py:326
- Existing unittest tests in this repository consistently annotate test methods with
-> None; this new test method omits that return annotation, which makes the file inconsistent with the established test style.
def test_current_workforce_record_parse_many_requires_list_payload(self):
tests/test_current_workforce.py:330
- Existing unittest tests in this repository consistently annotate test methods with
-> None; this new test method omits that return annotation, which makes the file inconsistent with the established test style.
def test_get_current_workforce_uses_afm_filter_and_parses_wrapped_response(self):
tests/test_current_workforce.py:345
- Existing unittest tests in this repository consistently annotate test methods with
-> None; this new test method omits that return annotation, which makes the file inconsistent with the established test style.
def test_get_current_workforce_preserves_empty_string_filter(self):
tests/test_current_workforce.py:358
- Existing unittest tests in this repository consistently annotate test methods with
-> None; this new test method omits that return annotation, which makes the file inconsistent with the established test style.
def test_get_current_workforce_omits_none_filter_and_handles_empty_response(self):
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
parisk
left a comment
There was a problem hiding this comment.
Great work. Again, one non-blocking comments, but requesting changes to ensure we reviewed copilot's review.
| response = self._execute_service("EX_BASE_05", parameters) | ||
|
|
||
| if not response: | ||
| return CurrentWorkforceRecord.parse_many(None) | ||
|
|
||
| payload = response.json() | ||
|
|
||
| return CurrentWorkforceRecord.parse_many(payload) |
There was a problem hiding this comment.
Similar to my comment in the other PR, I believe this is both verbose and idiomatic. Although it's a non blocking comment, I would structure it with a default value and a walrus:
| response = self._execute_service("EX_BASE_05", parameters) | |
| if not response: | |
| return CurrentWorkforceRecord.parse_many(None) | |
| payload = response.json() | |
| return CurrentWorkforceRecord.parse_many(payload) | |
| payload = None | |
| if response := self._execute_service("EX_BASE_05", parameters): | |
| payload = response.json() | |
| return CurrentWorkforceRecord.parse_many(payload) |
Resolve additive conflicts in ergani/client.py: keep the new get_current_workforce (EX_BASE_05) method alongside the get_branch_details (EX_BASE_02) and get_employer_details (EX_BASE_01) methods, and union the model imports.
Align get_current_workforce error handling with get_branch_details (EX_BASE_02) and get_employer_details (EX_BASE_01), which wrap response.json() and raise a descriptive ValueError on non-JSON payloads.
Deploying ergani with
|
| Latest commit: |
3db96ae
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://34a41f25.ergani.pages.dev |
| Branch Preview URL: | https://feature-ex-base-05-current-w.ergani.pages.dev |
No description provided.