feat: add current workforce endpoint#31
Open
akalipetis wants to merge 1 commit into
Open
Conversation
Contributor
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.
|
|
||
|
|
||
| class CurrentWorkforceTests(TestCase): | ||
| def test_current_workforce_request_omits_none_and_preserves_empty_string(self): |
|
|
||
|
|
||
| @dataclass(frozen=True) | ||
| class CurrentWorkforceRequest: |
|
|
||
|
|
||
| @dataclass | ||
| class CurrentWorkforceRecord: |
Comment on lines
+298
to
+299
| List[CurrentWorkforceRecord]: Current workforce records, each wrapping | ||
| the raw response object returned by the API. |
Comment on lines
+1
to
+2
| from __future__ import annotations | ||
|
|
parisk
requested changes
May 19, 2026
Contributor
parisk
left a comment
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.
Comment on lines
+307
to
+314
| response = self._execute_service("EX_BASE_05", parameters) | ||
|
|
||
| if not response: | ||
| return CurrentWorkforceRecord.parse_many(None) | ||
|
|
||
| payload = response.json() | ||
|
|
||
| return CurrentWorkforceRecord.parse_many(payload) |
Contributor
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:
Suggested change
| 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) |
This file contains hidden or 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
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.