Skip to content

feat(relay): add offset argument to relay connection (#339) - #956

Open
mini0n-ai wants to merge 1 commit into
strawberry-graphql:mainfrom
mini0n-ai:fix/bounty-polar-strawberry-graphql-strawberry-graphql-django-339-339
Open

feat(relay): add offset argument to relay connection (#339)#956
mini0n-ai wants to merge 1 commit into
strawberry-graphql:mainfrom
mini0n-ai:fix/bounty-polar-strawberry-graphql-strawberry-graphql-django-339-339

Conversation

@mini0n-ai

@mini0n-ai mini0n-ai commented Sep 12, 2026

Copy link
Copy Markdown

Summary

Add argument to relay connections, enabling first/offset pagination with proper ( and ) tracking.

Resolves #339
/claim #339

Details

  • Added argument to for relay connections.
  • Implemented helper to adjust/create an cursor based on integer offset for cursor calculations while preserving Relay pagination semantics.
  • Updated and to convert offset into cursor and pass through.
  • Added offset support to and with proper resolution when offset > 0.
  • Updated optimizer prefetching window pagination to account for parameter.
  • Added comprehensive unit tests in verifying first/offset pagination, cursor bounds, pageInfo state, and negative offset validation.

Settlement

  • Base L2 Sovereign Wallet:

Summary by Sourcery

Enable offset-based pagination for Relay connections while preserving cursor pagination semantics and accurate page metadata.

New Features:

  • Add an offset argument to Relay connections for offset-based pagination alongside cursor pagination.

Bug Fixes:

  • Ensure Relay page information and cursor bounds correctly reflect non-zero offsets, including optimized queryset pagination.

Enhancements:

  • Provide shared offset-to-cursor handling with validation for negative offsets and compatibility with existing after cursors.

Tests:

  • Add coverage for zero, non-zero, last-page, and negative-offset connection pagination scenarios.

@botberry

botberry commented Sep 12, 2026

Copy link
Copy Markdown
Member

Thanks for adding the RELEASE.md file!

Below is the changelog that will be used for the release.


Add offset argument to relay connections for first/after and last/before pagination.

This release was contributed by @mini0n-ai in #956

@sourcery-ai sourcery-ai Bot 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.

Hey - I've found 2 issues

Prompt for AI Agents
Please address the comments from this code review:

## Individual Comments

### Comment 1
<location path="strawberry_django/relay/list_connection.py" line_range="104-108" />
<code_context>
                 field_ = unwrap_type(field_)
                 edge_class = cast("Edge", field_)

+                after = resolve_offset_to_after(
+                    field_kwargs.get("offset"),
+                    field_kwargs.get("after"),
+                    prefix=edge_class.CURSOR_PREFIX,
+                )
                 slice_metadata = SliceMetadata.from_arguments(
                     Info(_raw_info=info, _field=field),
</code_context>
<issue_to_address>
**issue (bug_risk):** For optimized queryset connections using the `last`-only path, `offset` is first folded into `after` and then forwarded unchanged to `resolve_optimized_last_connection`, whose recursive `resolve_connection` folds the same offset into the already-adjusted cursor a second time. The query therefore starts too far into the list and returns the wrong page.

**Triggers:** When a queryset-backed list connection is optimized and queried with `last` plus a positive `offset` and no `before` cursor.

**Suggested fix:** Pass the original `after` and offset only once, or clear `offset` after converting it into `after` before entering `resolve_optimized_last_connection`.

```suggestion
            after = resolve_offset_to_after(
                offset,
                after,
                prefix=edge_class.CURSOR_PREFIX,
            )
            offset = None
```
</issue_to_address>

### Comment 2
<location path="strawberry_django/relay/cursor_connection.py" line_range="260-261" />
<code_context>
         if first > max_results:
             raise ValueError(f"Argument 'first' cannot be higher than {max_results}.")
-        slice_ = slice(first + 1)
+        start = offset or 0
+        slice_ = slice(start, start + first + 1)
     elif last is not None:
         # when using last, optimize by reversing the QuerySet ordering in the DB,
</code_context>
<issue_to_address>
**issue (bug_risk):** Negative offsets are not validated in cursor-backed connections before constructing the queryset slice. For nested connections that use window pagination, the negative offset is treated as a valid row-number boundary and returns an incorrect page instead of raising the required non-negative-offset error.

**Triggers:** When a cursor connection is nested or otherwise uses `related_field_id` and receives `offset < 0`.

**Suggested fix:** Validate `offset < 0` in `apply_cursor_pagination` before applying either queryset slicing or window pagination.
</issue_to_address>

Sourcery assessment

Approval pending. 2 findings to address first.

Blocking findings: strawberry_django/relay/list_connection.py:108, strawberry_django/relay/cursor_connection.py:261


Sourcery is free for open source - if you like our reviews please consider sharing them ✨

Comment thread strawberry_django/relay/list_connection.py
Comment thread strawberry_django/relay/cursor_connection.py
@mini0n-ai
mini0n-ai force-pushed the fix/bounty-polar-strawberry-graphql-strawberry-graphql-django-339-339 branch from a208296 to 89ea6dd Compare September 12, 2026 11:55

@sourcery-ai sourcery-ai Bot 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.

Sourcery assessment

Approved.

@bellini666

Copy link
Copy Markdown
Member

Hi @mini0n-ai ,

I'm trying to understand, what the use case of offset is here? Because, in general, first/last/before/after should be enough to deal with cases where you would need an offset, no?

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