add missing page_info / cursors for Association Proxy logic #241

Ckk3 · 2025-04-25T17:08:50Z

Note: This PR supersedes #232 because the original contributor did not allow edits from maintainers, and we needed to add additional tests to proceed with the merge. All credits remain to the original author for the core fix.

Original description:

Description

association_proxy_resolver_for has a few code blocks that were not updated for the relay changes. Association proxy mapping would fail due do constructors missing either page_info or cursor args.

Types of Changes

Issues Fixed or Closed by This PR

N/a

Checklist

My code follows the code style of this project.
My change requires a change to the documentation.
I have updated the documentation accordingly.
I have read the CONTRIBUTING document.
I have added tests to cover my changes.
I have tested the changes and verified that they work and don't break anything (as well as I can manage).

I haven't added tests as it looks like this code is originally untested.

Summary by Sourcery

Bug Fixes:

Fix association proxy mapping failures due to missing page_info or cursor arguments in constructors.

Summary by Sourcery

Bug Fixes:

Ensure association proxy resolvers return valid relay connections, including page_info and edge cursor details, even for empty results.

Summary by Sourcery

Fix association proxy resolvers to always return valid relay connections with correct page_info and cursor fields, including for empty results, and add comprehensive tests for association proxy logic.

Bug Fixes:

Fix association proxy mapping failures by ensuring relay connections include page_info and cursor fields, even for empty results.

Enhancements:

Improve error messages for unsupported association proxy targets and related exceptions.

Documentation:

Add release notes describing the patch and its impact.

Tests:

Add extensive tests for association proxy behavior, including edge cases and nullable relationships.

…fixrelay

sourcery-ai · 2025-04-25T17:08:56Z

Reviewer's Guide

This PR fixes association proxy mapping failures by ensuring that relay connection constructors always receive the required page_info and cursor arguments, even for empty results. It refactors resolver logic for association proxies, improves error messages for unsupported targets, and adds comprehensive tests for association proxy behavior, including edge cases. Minor code style and test improvements are also included.

File-Level Changes

Change	Details	Files
Fix association proxy relay connection construction to always provide page_info and cursor.	Refactor resolver logic to use a shared _resolve_connection_edges method for constructing relay connections. Ensure page_info and cursor fields are always set, even for empty connections. Update association proxy resolver to handle None and empty cases gracefully.	`src/strawberry_sqlalchemy_mapper/mapper.py`
Improve error messages for unsupported association proxy targets and related exceptions.	Clarify and expand exception messages for unsupported association proxies and descriptors.	`src/strawberry_sqlalchemy_mapper/exc.py`
Add comprehensive tests for association proxy logic, including edge cases and relay connection output.	Add new test_association_proxy.py with extensive tests for association proxy mapping, relay connection output, and error handling. Update conftest.py to provide a mapper fixture for tests. Refactor test_mapper.py for code style and assertion improvements.	`tests/test_association_proxy.py` `tests/conftest.py` `tests/test_mapper.py`
Document the patch and its impact on relay connection handling.	Add RELEASE.md describing the patch and its effect on association proxy relay connections.	`RELEASE.md`

Tips and commands

Interacting with Sourcery

Trigger a new review: Comment @sourcery-ai review on the pull request.
Continue discussions: Reply directly to Sourcery's review comments.
Generate a GitHub issue from a review comment: Ask Sourcery to create an
issue from a review comment by replying to it. You can also reply to a
review comment with @sourcery-ai issue to create an issue from it.
Generate a pull request title: Write @sourcery-ai anywhere in the pull
request title to generate a title at any time. You can also comment
@sourcery-ai title on the pull request to (re-)generate the title at any time.
Generate a pull request summary: Write @sourcery-ai summary anywhere in
the pull request body to generate a PR summary at any time exactly where you
want it. You can also comment @sourcery-ai summary on the pull request to
(re-)generate the summary at any time.
Generate reviewer's guide: Comment @sourcery-ai guide on the pull
request to (re-)generate the reviewer's guide at any time.
Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
pull request to resolve all Sourcery comments. Useful if you've already
addressed all the comments and don't want to see them anymore.
Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
request to dismiss all existing Sourcery reviews. Especially useful if you
want to start fresh with a new review - don't forget to comment
@sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

Enable or disable review features such as the Sourcery-generated pull request
summary, the reviewer's guide, and others.
Change the review language.
Add, remove or edit custom review instructions.
Adjust other review settings.

Getting Help

Contact our support team for questions or feedback.
Visit our documentation for detailed guides and information.
Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

codecov-commenter · 2025-05-10T22:34:30Z

Codecov Report

Attention: Patch coverage is 97.61905% with 7 lines in your changes missing coverage. Please review.

Project coverage is 89.94%. Comparing base (e19bdb8) to head (cfc8c6c).

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #241      +/-   ##
==========================================
+ Coverage   86.05%   89.94%   +3.88%     
==========================================
  Files          16       17       +1     
  Lines        1671     1939     +268     
  Branches      136      141       +5     
==========================================
+ Hits         1438     1744     +306     
+ Misses        174      123      -51     
- Partials       59       72      +13

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

codspeed-hq · 2025-05-10T22:34:57Z

CodSpeed Performance Report

Merging #241 will not alter performance

_{Comparing Ckk3:tyler-fixrelay (cfc8c6c) with main (e19bdb8)}

Summary

✅ 1 untouched benchmarks

…ction

Ckk3 · 2025-05-11T14:20:17Z

@sourcery-ai review

Ckk3 · 2025-05-11T14:20:28Z

@sourcery-ai summary

sourcery-ai

Hey @Ckk3 - I've reviewed your changes and they look great!

Here's what I looked at during the review

🟡 General issues: 2 issues found
🟢 Security: all looks good
🟡 Testing: 1 issue found
🟢 Documentation: all looks good

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

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

src/strawberry_sqlalchemy_mapper/mapper.py

tests/test_association_proxy.py

botberry · 2025-05-11T23:05:55Z

Thanks for adding the RELEASE.md file!

Here's a preview of the changelog:

Ensure association proxy resolvers return valid relay connections, including page_info and edge cursor details, even for empty results.

Thanks to https://github.com/tylernisonoff for the original PR.

Ckk3 · 2025-05-11T23:06:19Z

Ready for review, @patrick91 @bellini666 @fruitymedley !

Ckk3 · 2025-05-11T23:07:50Z

@sourcery-ai review

sourcery-ai

Hey @Ckk3 - I've reviewed your changes and they look great!

Here's what I looked at during the review

🟢 General issues: all looks good
🟢 Security: all looks good
🟢 Testing: all looks good
🟢 Documentation: all looks good

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

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

src/strawberry_sqlalchemy_mapper/mapper.py

sourcery-ai

Hey @Ckk3 - I've reviewed your changes and they look great!

Here's what I looked at during the review

🟢 General issues: all looks good
🟢 Security: all looks good
🟢 Testing: all looks good
🟢 Documentation: all looks good

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

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

src/strawberry_sqlalchemy_mapper/mapper.py

Ckk3 · 2025-05-13T14:21:36Z

Thanks @tylernisonoff 😊

tylernisonoff and others added 4 commits January 22, 2025 08:51

add missing page_info / cursors for relay

fc80f6d

add RELEASE.md

3decff7

add missing cursor

47b1d71

Merge remote-tracking branch 'strawberry-sqlalchemy/main' into tyler-…

65b484a

…fixrelay

Ckk3 mentioned this pull request Apr 25, 2025

add missing page_info / cursors for Association Proxy logic #232

Closed

11 tasks

Ckk3 self-assigned this Apr 25, 2025

Ckk3 added 2 commits April 25, 2025 18:10

run formatter and update release

b5d7a59

added tests to proxy_association, still missing a test case

e7e98f6

Ckk3 added 7 commits May 11, 2025 00:35

Add a better messages in the exception, add new test with keysetconne…

27fb776

…ction

add _resolve_connection_edges to a staticmethod and run pre-commit

f0e6a57

update Release md

7e72d7e

Run pre-commit in exc.py

dc7aa61

run pre-commit in conftest.py

dca18c8

run pre-commit in test_association_proxy.py

3b05959

run pre-commit in test_mapper

e9a4232

sourcery-ai bot reviewed May 11, 2025

View reviewed changes

src/strawberry_sqlalchemy_mapper/mapper.py Show resolved Hide resolved

src/strawberry_sqlalchemy_mapper/mapper.py Show resolved Hide resolved

tests/test_association_proxy.py Outdated Show resolved Hide resolved

Ckk3 added 3 commits May 11, 2025 19:06

Fix tests on windows error

8971938

add a new test to null relationship

52e91d6

refactor test_association_proxy for better understanding

e4f546c

Ckk3 marked this pull request as ready for review May 11, 2025 23:05

botberry added bot:has-release-file bot:release-type-patch labels May 11, 2025

Ckk3 requested review from bellini666 and patrick91 May 11, 2025 23:06

Ckk3 requested review from novag and removed request for novag May 11, 2025 23:06

Ckk3 requested a review from fruitymedley May 11, 2025 23:06

sourcery-ai bot reviewed May 11, 2025

View reviewed changes

src/strawberry_sqlalchemy_mapper/mapper.py Show resolved Hide resolved

sourcery-ai bot reviewed May 11, 2025

View reviewed changes

src/strawberry_sqlalchemy_mapper/mapper.py Show resolved Hide resolved

patrick91 approved these changes May 13, 2025

View reviewed changes

update release.md to add tylernisonoff credits

cfc8c6c

Ckk3 merged commit 74c41b7 into strawberry-graphql:main May 13, 2025
19 checks passed

Uh oh!

add missing page_info / cursors for Association Proxy logic #241

add missing page_info / cursors for Association Proxy logic #241

Uh oh!

Conversation

Ckk3 commented Apr 25, 2025 • edited by sourcery-ai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Types of Changes

Issues Fixed or Closed by This PR

Checklist

Summary by Sourcery

Summary by Sourcery

Summary by Sourcery

Uh oh!

sourcery-ai bot commented Apr 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviewer's Guide

File-Level Changes

Interacting with Sourcery

Customizing Your Experience

Getting Help

Uh oh!

codecov-commenter commented May 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

codspeed-hq bot commented May 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

CodSpeed Performance Report

Merging #241 will not alter performance

Summary

Uh oh!

Ckk3 commented May 11, 2025

Uh oh!

Ckk3 commented May 11, 2025

Uh oh!

sourcery-ai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

botberry commented May 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Ckk3 commented May 11, 2025

Uh oh!

Ckk3 commented May 11, 2025

Uh oh!

sourcery-ai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sourcery-ai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Ckk3 commented May 13, 2025

Uh oh!

Uh oh!

Ckk3 commented Apr 25, 2025 •

edited by sourcery-ai bot

Loading

sourcery-ai bot commented Apr 25, 2025 •

edited

Loading

codecov-commenter commented May 10, 2025 •

edited

Loading

codspeed-hq bot commented May 10, 2025 •

edited

Loading

botberry commented May 11, 2025 •

edited

Loading