Skip to content

feat: add current workforce endpoint#31

Open
akalipetis wants to merge 3 commits into
mainfrom
feature/ex-base-05-current-workforce
Open

feat: add current workforce endpoint#31
akalipetis wants to merge 3 commits into
mainfrom
feature/ex-base-05-current-workforce

Conversation

@akalipetis

Copy link
Copy Markdown
Contributor

No description provided.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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 service EX_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.

Comment thread tests/test_current_workforce.py
Comment thread ergani/models.py
Comment thread ergani/models.py
Comment thread ergani/client.py
Comment thread ergani/models.py

@parisk parisk left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Great work. Again, one non-blocking comments, but requesting changes to ensure we reviewed copilot's review.

Comment thread ergani/client.py
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)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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)

Base automatically changed from akalipetis-execute-services to main May 19, 2026 16:56
claude added 2 commits June 15, 2026 16:31
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.
@cloudflare-workers-and-pages

Copy link
Copy Markdown

Deploying ergani with  Cloudflare Pages  Cloudflare Pages

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

View logs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants