Skip to content

Commit

Permalink
La 175 segment unify pagination loop fix (#5596)
Browse files Browse the repository at this point in the history
Co-authored-by: Adrian Galvan <[email protected]>
  • Loading branch information
Vagoasdf and galvana authored Dec 17, 2024
1 parent 4f1a202 commit 937e713
Show file tree
Hide file tree
Showing 4 changed files with 89 additions and 0 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ The types of changes are:
- New page in the Cookie House sample app to demonstrate the use of embedding the FidesJS SDK on the page [#5564](https://github.com/ethyca/fides/pull/5564)
- Added event based communication example to the Cookie House sample app [#5597](https://github.com/ethyca/fides/pull/5597)
- Added new erasure tests for BigQuery Enterprise [#5554](https://github.com/ethyca/fides/pull/5554)
- Added new `has_next` parameter for the `link` pagination strategy [#5596](https://github.com/ethyca/fides/pull/5596)

### Changed
- Adjusted Ant's Select component colors and icon [#5594](https://github.com/ethyca/fides/pull/5594)
Expand Down
1 change: 1 addition & 0 deletions src/fides/api/schemas/saas/strategy_configuration.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ class LinkPaginationConfiguration(StrategyConfiguration):
source: LinkSource
rel: Optional[str] = None
path: Optional[str] = None
has_next: Optional[str] = None

@model_validator(mode="before")
@classmethod
Expand Down
7 changes: 7 additions & 0 deletions src/fides/api/service/pagination/pagination_strategy_link.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ def __init__(self, configuration: LinkPaginationConfiguration):
self.source = configuration.source
self.rel = configuration.rel
self.path = configuration.path
self.has_next = configuration.has_next

def get_next_request(
self,
Expand All @@ -40,6 +41,12 @@ def get_next_request(
if not response_data:
return None

if self.has_next:
has_next = pydash.get(response.json(), self.has_next)
logger.info(f"The {self.has_next} field has a value of {has_next}")
if str(has_next).lower() != "true":
return None

# read the next_link from the correct location based on the source value
next_link = None
if self.source == LinkSource.headers.value:
Expand Down
80 changes: 80 additions & 0 deletions tests/ops/service/pagination/test_pagination_strategy_link.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,42 @@ def response_with_empty_string_link():
return response


@pytest.fixture(scope="function")
def response_with_has_next_conditional_true():
response = Response()
response._content = bytes(
json.dumps(
{
"customers": [{"id": 1}, {"id": 2}, {"id": 3}],
"links": {
"next": "https://domain.com/customers?page=def",
"hasNext": True,
},
}
),
"utf-8",
)
return response


@pytest.fixture(scope="function")
def response_with_has_next_conditional_false():
response = Response()
response._content = bytes(
json.dumps(
{
"customers": [{"id": 1}, {"id": 2}, {"id": 3}],
"links": {
"next": "https://domain.com/customers?page=abc",
"hasNext": False,
},
}
),
"utf-8",
)
return response


def test_link_in_headers(response_with_header_link):
config = LinkPaginationConfiguration(source="headers", rel="next")
request_params: SaaSRequestParams = SaaSRequestParams(
Expand Down Expand Up @@ -132,6 +168,50 @@ def test_link_in_body_empty_string(response_with_empty_string_link):
assert next_request is None


## TODO: Tests for when the link exists but there is a conditional boolean that checks if there is a next page
def test_link_in_body_with_conditional_boolean_true(
response_with_has_next_conditional_true,
):
config = LinkPaginationConfiguration(
source="body", path="links.next", has_next="links.hasNext"
)
request_params: SaaSRequestParams = SaaSRequestParams(
method=HTTPMethod.GET,
path="/customers",
query_params={"page": "abc"},
)

paginator = LinkPaginationStrategy(config)
next_request: Optional[SaaSRequestParams] = paginator.get_next_request(
request_params, {}, response_with_has_next_conditional_true, "customers"
)

assert next_request == SaaSRequestParams(
method=HTTPMethod.GET,
path="/customers",
query_params={"page": "def"},
)


def test_link_in_body_with_conditional_boolean_false(
response_with_has_next_conditional_false,
):
config = LinkPaginationConfiguration(
source="body", path="links.next", has_next="links.hasNext"
)
request_params: SaaSRequestParams = SaaSRequestParams(
method=HTTPMethod.GET,
path="/customers",
query_params={"page": "abc"},
)

paginator = LinkPaginationStrategy(config)
next_request: Optional[SaaSRequestParams] = paginator.get_next_request(
request_params, {}, response_with_has_next_conditional_false, "customers"
)
assert next_request is None


def test_wrong_source():
with pytest.raises(ValueError) as exc:
LinkPaginationConfiguration(source="somewhere", path="links.next")
Expand Down

0 comments on commit 937e713

Please sign in to comment.