search

strawberry-graphql / strawberry-sqlalchemy

A SQLAlchemy Integration for strawberry-graphql
MIT License
91 stars 26 forks source link

SQLAlchemy JSON support #40

Closed Ckk3 closed 1 year ago

Ckk3 commented 1 year ago

Description

This PR introduces native SQLAlchemy JSON conversion support. While it's unclear whether this is an actual issue or a deliberate design choice to exclude scalar types, I've made the necessary changes to enhance the functionality.

Resuming the changes, I've added {sqlalchemy.JSON: strawberry.scalars.JSON} in _default_sqlalchemy_type_to_strawberry_type_map.

I'd like to extend my gratitude to @wisdomG for their assistance with Issue #22 .

Additionally, please note that some tests required adjustments due to automatic changes introduced by pre-commit. I am using devcontainers and followed all guides in CONTRIBUTING.md for this development.

Types of Changes

Issues Fixed or Closed by This PR

Checklist

botberry commented 1 year ago

Thanks for adding the RELEASE.md file!

Here's a preview of the changelog:

Native SQLAlchemy JSON Conversion Support. Added native support for SQLAlchemy JSON conversions. Now, you'll find that sqlalchemy.JSON is converted to strawberry.scalars.JSON for enhanced compatibility.

TimDumol commented 1 year ago

Description

This PR introduces native SQLAlchemy JSON conversion support. While it's unclear whether this is an actual issue or a deliberate design choice to exclude scalar types, I've made the necessary changes to enhance the functionality.

Resuming the changes, I've added {sqlalchemy.JSON: strawberry.scalars.JSON} in _default_sqlalchemy_type_to_strawberry_type_map.

I'd like to extend my gratitude to @wisdomG for their assistance with Issue #22 .

Additionally, please note that some tests required adjustments due to automatic changes introduced by pre-commit. I am using devcontainers and followed all guides in CONTRIBUTING.md for this development.

Types of Changes

  • [ ] Core
  • [ ] Bugfix
  • [x] New feature
  • [ ] Enhancement/optimization
  • [x] Documentation

Issues Fixed or Closed by This PR

Checklist

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

FWIW the reason why JSON was not originally included when I first wrote the library was because Strawberry didn't have a built-in JSON scalar type at that time, and not as a specific design decision

Ckk3 commented 1 year ago

Description

This PR introduces native SQLAlchemy JSON conversion support. While it's unclear whether this is an actual issue or a deliberate design choice to exclude scalar types, I've made the necessary changes to enhance the functionality. Resuming the changes, I've added {sqlalchemy.JSON: strawberry.scalars.JSON} in _default_sqlalchemy_type_to_strawberry_type_map. I'd like to extend my gratitude to @wisdomG for their assistance with Issue #22 . Additionally, please note that some tests required adjustments due to automatic changes introduced by pre-commit. I am using devcontainers and followed all guides in CONTRIBUTING.md for this development.

Types of Changes

  • [ ] Core
  • [ ] Bugfix
  • [x] New feature
  • [ ] Enhancement/optimization
  • [x] Documentation

Issues Fixed or Closed by This PR

Checklist

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

FWIW the reason why JSON was not originally included when I first wrote the library was because Strawberry didn't have a built-in JSON scalar type at that time, and not as a specific design decision

Ok I understand, please let me know if the code needs any corrections, I'm available!

codspeed-hq[bot] commented 1 year ago

CodSpeed Performance Report

Merging #40 will not alter performance

Comparing Ckk3:native-json-support (62889a1) with main (8ffa1f4)

Summary

✅ 1 untouched benchmarks

codecov-commenter commented 1 year ago

Codecov Report

Merging #40 (62889a1) into main (8ffa1f4) will increase coverage by 0.53%. The diff coverage is 100.00%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #40 +/- ## ========================================== + Coverage 76.84% 77.37% +0.53% ========================================== Files 10 10 Lines 717 725 +8 Branches 107 108 +1 ========================================== + Hits 551 561 +10 + Misses 136 135 -1 + Partials 30 29 -1 ```
Ckk3 commented 1 year ago

Resolved conflicts with main.