Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: support node_id values containing colons #3025

Merged
merged 1 commit into from
Aug 10, 2023

Conversation

bellini666
Copy link
Member

The node_id can be anything depending on how the user is handling it, so colons can appear in it, specially when it is some kind of encrypted value.

This changes how we split the GlobalID string to only split the first colon in it, meaning that the remaining of the string will be considered as a whole for the node_id.

@bellini666 bellini666 self-assigned this Aug 10, 2023
@botberry
Copy link
Member

botberry commented Aug 10, 2023

Thanks for adding the RELEASE.md file!

Here's a preview of the changelog:


Add support for extra colons in the GlobalID string.

Before, the string SomeType:some:value would produce raise an error saying that
it was expected the string to be splited in 2 parts when doing .split(":").

Now we are using .split(":", 1), meaning that the example above will consider
SomeType to be the type name, and some:value to be the node_id.


Here's the preview release card for twitter:

Here's the tweet text:

🆕 Release (next) is out! Thanks to Thiago Bellini Ribeiro for the PR 👏

Get it here 👉 https://github.com/strawberry-graphql/strawberry/releases/tag/(next)

@codecov
Copy link

codecov bot commented Aug 10, 2023

Codecov Report

Merging #3025 (bfc1f63) into main (7adcbfe) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3025   +/-   ##
=======================================
  Coverage   96.53%   96.53%           
=======================================
  Files         468      468           
  Lines       29112    29116    +4     
  Branches     3582     3582           
=======================================
+ Hits        28104    28108    +4     
  Misses        827      827           
  Partials      181      181           

@codspeed-hq
Copy link

codspeed-hq bot commented Aug 10, 2023

CodSpeed Performance Report

Merging #3025 will not alter performance

Comparing bellini666:node_id_with_colons (bfc1f63) with main (7adcbfe)

Summary

✅ 12 untouched benchmarks

The node_id can be anything depending on how the user is handling it,
so colons can appear in it, specially when it is some kind of encrypted
value.

This changes how we split the `GlobalID` string to only split the
first colon in it, meaning that the remaining of the string will be
considered as a whole for the node_id.
@patrick91
Copy link
Member

:yay: :D

@bellini666 bellini666 merged commit e6c93f6 into strawberry-graphql:main Aug 10, 2023
57 checks passed
@bellini666 bellini666 deleted the node_id_with_colons branch August 10, 2023 15:26
etripier pushed a commit to Greenbax/strawberry that referenced this pull request Oct 25, 2023
…l#3025)

The node_id can be anything depending on how the user is handling it,
so colons can appear in it, specially when it is some kind of encrypted
value.

This changes how we split the `GlobalID` string to only split the
first colon in it, meaning that the remaining of the string will be
considered as a whole for the node_id.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants