Skip to content

Implement adoptions API with tests and documentation#56

Merged
sandaliSS merged 3 commits into
masterfrom
tinak25/feature/adoptions-api
May 17, 2026
Merged

Implement adoptions API with tests and documentation#56
sandaliSS merged 3 commits into
masterfrom
tinak25/feature/adoptions-api

Conversation

@tinak25
Copy link
Copy Markdown
Collaborator

@tinak25 tinak25 commented May 16, 2026

Adoptions API

This implements the Adoptions API for managing tree adoption records.

It includes:

  • Create a new adoption record
  • Fetch all adoption records with pagination
  • Fetch a single adoption by ID
  • Update adoption details
  • Delete an adoption record

Improvements:

  • Added Adoptions module with routes, controller, service, and index file
  • Added role-based access control for Admin and Manager access
  • Added validation for adoption ID, adopter ID, FOB ID, and adoption date
  • Added checks to ensure the adopter exists before creating or updating an adoption
  • Prevented future adoption dates
  • Added proper error handling using the centralised error middleware
  • Updated API documentation for the Adoptions module
  • Ensured consistent API responses across endpoints

Testing:

  • Added unit tests for Adoptions service logic
  • Added integration tests for all Adoptions endpoints
  • Verified Admin can create, update, and delete adoptions
  • Verified Admin and Manager can list and fetch adoptions
  • Verified restricted roles receive 403 responses
  • Verified missing token returns 401
  • Verified invalid payloads return 400
  • Verified missing adoption/adopter records return 404
  • Confirmed tests, lint, and build are passing

@tinak25 tinak25 requested a review from AKV2703 May 16, 2026 08:10
@tinak25 tinak25 requested a review from sandaliSS May 16, 2026 11:43
Copy link
Copy Markdown
Collaborator

@AKV2703 AKV2703 left a comment

Choose a reason for hiding this comment

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

The overall structure and CRUD flow for the Adoptions API is implemented well, and the RBAC setup/routes generally align with the proposal. However, there are still a few important areas that should be addressed before merge. The main issues are around validation consistency, proposal compliance, and test coverage. listAdoptions() currently only supports pagination but is missing the required filters (fob_id, adopter, year). Some validation paths are using incorrect error codes, and pagination parsing in the controller silently defaults invalid values instead of rejecting them. The current delete implementation/tests also hard delete records, while the proposal expects adoption history/cancellation data to be preserved, so this behaviour should be reviewed. On the testing side, unit/integration coverage should be expanded for RBAC restrictions, filter behaviour, invalid query params, stricter validation scenarios, and cleanup of created test data. Overall the foundation is good, but these gaps should be resolved to fully align with the project specification and avoid future issues.

Comment thread src/modules/adoptions/adoptions.service.ts
Comment thread src/modules/adoptions/adoptions.controller.ts
Comment thread tests/unit/adoptions.test.ts
Comment thread tests/integration/adoptions.test.ts
Copy link
Copy Markdown
Collaborator

@AKV2703 AKV2703 left a comment

Choose a reason for hiding this comment

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

Reviewed the latest updates and the implementation is now in a much better state overall. The service, controller, validation flow, RBAC handling, documentation, and test coverage have all been significantly improved and the main concerns raised earlier were addressed properly. Approving this PR now.

Copy link
Copy Markdown
Collaborator

@sandaliSS sandaliSS left a comment

Choose a reason for hiding this comment

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

approved

@sandaliSS sandaliSS merged commit 0c5b05e into master May 17, 2026
1 check passed
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