Skip to content

Added controller method that returns not accepted policies by user#1207

Open
dati18 wants to merge 11 commits into
mainfrom
dat/active-policy-not-accept-by-user
Open

Added controller method that returns not accepted policies by user#1207
dati18 wants to merge 11 commits into
mainfrom
dat/active-policy-not-accept-by-user

Conversation

@dati18

@dati18 dati18 commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Controller logic updated in PoliciesController.php with new endpoint getMissingPolicies
  • API route added in routes/api.php (new authenticated route GET v1/policies/missing, handled by PoliciesController@getMissingPolicies)
  • New test class covering both current and missing policies, including coverage for:
    • auth required on missing endpoint
    • only latest active policy per type is returned
    • future policies are excluded
    • already accepted policies are excluded for current user
    • another user’s acceptances do not affect current user
    • empty list when all current policies are accepted
    • current policies response structure and payload fragments
  • The older PoliciesController test under tests/Http/Controllers was merged into the route-level file above because they do full http behaviors like auth middleware, URL, JSON request/response, calling endpoints, etc., not isolated controller functions

Bug: T432337

@dati18
dati18 marked this pull request as draft July 17, 2026 15:03
@dati18
dati18 force-pushed the dat/active-policy-not-accept-by-user branch from 6ef72d9 to f2a65e6 Compare July 20, 2026 15:03
@dati18
dati18 marked this pull request as ready for review July 20, 2026 15:06
Comment thread app/Http/Controllers/PoliciesController.php Outdated
Comment thread app/Http/Controllers/PoliciesController.php Outdated
Comment thread tests/Routes/PoliciesControllerTest.php
@dati18
dati18 force-pushed the dat/active-policy-not-accept-by-user branch from 5c104b5 to cb54707 Compare July 21, 2026 15:36
@dati18
dati18 force-pushed the dat/active-policy-not-accept-by-user branch 2 times, most recently from 4e1f1dd to 6f350a7 Compare July 22, 2026 07:52

@rosalieper rosalieper 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.

I think you have accidentally added unwanted changes to this PR. I see changes from #1207 here. ignore

@dati18
dati18 force-pushed the dat/active-policy-not-accept-by-user branch from 6f350a7 to db25856 Compare July 22, 2026 07:56
@dati18
dati18 dismissed rosalieper’s stale review July 22, 2026 08:29

stale request

@dati18
dati18 requested a review from rosalieper July 22, 2026 08:29

@rosalieper rosalieper 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.

I am happy with the change 💯

Comment thread app/Http/Controllers/PoliciesController.php

@tarrow tarrow 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.

I really like a lot of this patch.

The tests are super comprehensive and readable to me.

I liked that you identified that this should be in Routes test.

I'm still a bit sceptical about the query logic which I personally find hard a little harder to parse than the far less efficient first draft but I do agree that performance probably matters here so it's an acceptable tradeoff for me.

I've left a small suggestion for the direction I looked into just for your interest but I don't think we should look further into this.

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.

3 participants