Skip to content

Binding missing pydantic models for Persons Page Validation - #138

Open
NethmikaKekuu wants to merge 2 commits into
LDFLK:mainfrom
NethmikaKekuu:person-page-pv
Open

Binding missing pydantic models for Persons Page Validation#138
NethmikaKekuu wants to merge 2 commits into
LDFLK:mainfrom
NethmikaKekuu:person-page-pv

Conversation

@NethmikaKekuu

@NethmikaKekuu NethmikaKekuu commented Aug 28, 2026

Copy link
Copy Markdown
Member

closes #137

Summary by CodeRabbit

  • New Features

    • Added structured person history responses, including ministry details, terms, and presidential service indicators.
    • Added aggregate ministry and presidency history information.
  • Bug Fixes

    • Improved history result validation and consistency.
    • Ensured history entries are correctly sorted and internal timing details are excluded from responses.
  • Tests

    • Expanded coverage for complete history data, sorting, and aggregate counts.

@coderabbitai

coderabbitai Bot commented Aug 28, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 3ce4ff09-da59-4028-82d3-c70fd1056b08

📥 Commits

Reviewing files that changed from the base of the PR and between a83485b and f4f172a.

📒 Files selected for processing (4)
  • src/models/__init__.py
  • src/models/person_schemas.py
  • src/services/person_service.py
  • test/test_person_service.py

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

Changes

Person History Response

Layer / File(s) Summary
Define and export response models
src/models/person_schemas.py, src/models/__init__.py
Adds strict Pydantic models for ministry history items and person history responses. Exports PersonHistoryResponse.
Bind service output and tests
src/services/person_service.py, test/test_person_service.py
Builds the service result with PersonHistoryResponse.model_dump(). Tests validate ordering, field removal, and aggregate counts.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to f4f17

The person-history response is now validated against an explicit schema while preserving the existing endpoint behavior. No actionable merge-blocking risk remains after normal checks and review.

Suggested reviewers: chanukauoj, rusiru-erandaka

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 4 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The changes satisfy issue #137 by adding Pydantic models for person history responses, binding the service output to the response model, exporting the model, and updating Persons page tests.
Out of Scope Changes check ✅ Passed All changes support issue #137. No unrelated code or test changes are evident.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main change: adding missing Pydantic models for Persons page validation.
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@NethmikaKekuu

Copy link
Copy Markdown
Member Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Aug 28, 2026

Copy link
Copy Markdown
✅ Action performed

Full review finished.

@NethmikaKekuu NethmikaKekuu changed the title Binding missing pydantic models for Perons Page Validation Binding missing pydantic models for Persons Page Validation Aug 28, 2026
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.

Binding Missing Pydantic Models in BFF APIs & fix tests for Persons page

1 participant