La 175 segment unify pagination loop fix #5596
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Skipped Deployment
|
fides
|
||||||||||||||||||||||||||||||
| Project |
fides
|
| Branch Review |
refs/pull/5596/merge
|
| Run status |
|
| Run duration | 00m 57s |
| Commit |
|
| Committer | Bruno Gutierrez Rios |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
Failures
|
0
|
Flaky
|
0
|
Pending
|
0
|
Skipped
|
0
|
Passing
|
4
|
Upgrade your plan to view test results. | |
| View all changes introduced in this branch ↗︎ | |
| "customers": [{"id": 1}, {"id": 2}, {"id": 3}], | ||
| "links": { | ||
| "next": "https://domain.com/customers?page=def", | ||
| "hasNext": "true", |
There was a problem hiding this comment.
Now that we know what the actual payload looks like, we should replace this with a boolean
| "hasNext": "true", | |
| "hasNext": True, |
Same thing for the other fixture
There was a problem hiding this comment.
Replacing them with base boolean.
I was thinking, would it be redundant to also test that it keeps catching these string values correctly? I.E if a API returns TRUE/FALSE in all caps, or something similar
There was a problem hiding this comment.
I think this is ok for now, we can handle those use cases if we run into them
Improved logging to check the solution Co-authored-by: Adrian Galvan <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5596 +/- ##
==========================================
+ Coverage 80.96% 87.15% +6.18%
==========================================
Files 388 387 -1
Lines 23895 23900 +5
Branches 2585 2585
==========================================
+ Hits 19347 20830 +1483
+ Misses 4028 2512 -1516
- Partials 520 558 +38 ☔ View full report in Codecov by Sentry. |
fides
|
||||||||||||||||||||||||||||||
| Project |
fides
|
| Branch Review |
main
|
| Run status |
|
| Run duration | 00m 48s |
| Commit |
|
| Committer | Bruno Gutierrez Rios |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
|
Failures
|
0
|
|
Flaky
|
0
|
|
Pending
|
0
|
|
Skipped
|
0
|
|
Passing
|
4
|
Upgrade your plan to view test results. | |
| View all changes introduced in this branch ↗︎ | |
Closes LA#175
Description Of Changes
Sets up a new parameter on link pagination, to check for a "has next propiety", optionally included on the response parameters, which determines if there is a next link to be searched for
Some endpoints (see Segment Unify ) rely on this response parameter, and they return the current page on the has next value
Code Changes
Steps to Confirm
Ideally we would check with Segment Unify. Currently being tested with Unit Testing
Pre-Merge Checklist
CHANGELOG.mdupdated