-
Notifications
You must be signed in to change notification settings - Fork 1
ref: mapper api overhaul #147
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
base: main
Are you sure you want to change the base?
Changes from 7 commits
16d303b
abf6ba0
18ba6e5
5afbf12
912c82e
f972983
9fcd3ee
01276e4
dff45dc
701b55c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -3,7 +3,7 @@ | |||||||||||||||||
| from __future__ import annotations | ||||||||||||||||||
|
|
||||||||||||||||||
| from dataclasses import dataclass, field | ||||||||||||||||||
| from typing import TYPE_CHECKING, Any | ||||||||||||||||||
| from typing import TYPE_CHECKING, Any, Literal | ||||||||||||||||||
|
|
||||||||||||||||||
| from strawchemy.dto.inspectors import SQLAlchemyGraphQLInspector | ||||||||||||||||||
| from strawchemy.repository.strawberry import StrawchemySyncRepository | ||||||||||||||||||
|
|
@@ -43,14 +43,18 @@ class StrawchemyConfig: | |||||||||||||||||
| """Override default filters with custom filters.""" | ||||||||||||||||||
| execution_options: dict[str, Any] | None = None | ||||||||||||||||||
| """SQLAlchemy execution options for strawberry operations.""" | ||||||||||||||||||
| pagination_default_limit: int = 100 | ||||||||||||||||||
| """Default pagination limit when `pagination=True`.""" | ||||||||||||||||||
| pagination: bool = False | ||||||||||||||||||
| """Enable/disable pagination on list resolvers.""" | ||||||||||||||||||
| default_id_field_name: str = "id" | ||||||||||||||||||
| """Name for primary key fields arguments on primary key resolvers.""" | ||||||||||||||||||
| deterministic_ordering: bool = True | ||||||||||||||||||
| """Force deterministic ordering for list resolvers.""" | ||||||||||||||||||
| pagination: Literal["all"] | None = None | ||||||||||||||||||
| """Enable/disable pagination on list resolvers.""" | ||||||||||||||||||
| order_by: Literal["all"] | None = None | ||||||||||||||||||
| """Enable/disable order by on list resolvers.""" | ||||||||||||||||||
| pagination_default_limit: int = 100 | ||||||||||||||||||
| """Default pagination limit when `pagination=True`.""" | ||||||||||||||||||
| pagination_default_offset: int = 0 | ||||||||||||||||||
| """Default pagination offset when `pagination=True`.""" | ||||||||||||||||||
|
Comment on lines
+62
to
+65
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Inline docstrings reference outdated API. The docstrings on lines 55 and 57 mention 🔎 Proposed fix pagination_default_limit: int = 100
- """Default pagination limit when `pagination=True`."""
+ """Default pagination limit when pagination is enabled."""
pagination_default_offset: int = 0
- """Default pagination offset when `pagination=True`."""
+ """Default pagination offset when pagination is enabled."""📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
Comment on lines
+52
to
+65
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fix documentation errors in field docstrings. There are two issues in the docstrings:
📝 Proposed fix include: IncludeFields = "all"
"""Globally included fields."""
exclude: FieldIterable | None = None
- """Globally included fields."""
+ """Globally excluded fields."""
pagination: IncludeFields | None = None
"""Enable/disable pagination on list resolvers."""
order_by: IncludeFields | None = None
"""Enable/disable order by on list resolvers."""
distinct_on: IncludeFields | None = None
- """Enable/disable order by onelist resolvers."""
+ """Enable/disable distinct on for list resolvers."""
pagination_default_limit: int = 100
"""Default pagination limit when `pagination=True`."""
pagination_default_offset: int = 0
"""Default pagination offset when `pagination=True`."""🤖 Prompt for AI Agents |
||||||||||||||||||
|
|
||||||||||||||||||
| inspector: SQLAlchemyGraphQLInspector = field(init=False) | ||||||||||||||||||
|
|
||||||||||||||||||
|
|
||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,7 +7,7 @@ | |
| from enum import Enum | ||
| from typing import TYPE_CHECKING, Any, Literal, TypeAlias, final, get_type_hints | ||
|
|
||
| from typing_extensions import override | ||
| from typing_extensions import Self, override | ||
|
|
||
| from strawchemy.utils.annotation import get_annotations | ||
|
|
||
|
|
@@ -23,15 +23,15 @@ | |
| "DTOScope", | ||
| "DTOSkip", | ||
| "DTOUnset", | ||
| "ExcludeFields", | ||
| "FieldIterable", | ||
| "IncludeFields", | ||
| "Purpose", | ||
| "PurposeConfig", | ||
| ) | ||
|
|
||
| DTOScope: TypeAlias = Literal["global", "dto"] | ||
| IncludeFields: TypeAlias = "list[str] | set[str] | Literal['all']" | ||
| ExcludeFields: TypeAlias = "list[str] | set[str]" | ||
| FieldIterable: TypeAlias = "list[str] | set[str] | frozenset[str] | tuple[str, ...]" | ||
| IncludeFields: TypeAlias = "FieldIterable | Literal['all']" | ||
|
|
||
|
|
||
| @final | ||
|
|
@@ -160,7 +160,7 @@ class DTOConfig: | |
| """Configure the DTO for "read" or "write" operations.""" | ||
| include: IncludeFields = field(default_factory=set) | ||
| """Explicitly include fields from the generated DTO.""" | ||
| exclude: ExcludeFields = field(default_factory=set) | ||
| exclude: FieldIterable = field(default_factory=set) | ||
| """Explicitly exclude fields from the generated DTO. Implies `include="all"`.""" | ||
| partial: bool | None = None | ||
| """Make all field optional.""" | ||
|
|
@@ -185,11 +185,34 @@ def __post_init__(self) -> None: | |
| if self.exclude: | ||
| self.include = "all" | ||
|
|
||
| @classmethod | ||
| def from_include(cls, include: IncludeFields | None = None, purpose: Purpose = Purpose.READ) -> Self: | ||
| """Create a DTOConfig from an include specification. | ||
|
|
||
| Factory method for creating a DTOConfig with a simplified interface, converting | ||
| an `IncludeFields` specification into a complete configuration object. This is | ||
| useful for building configs when only the include/exclude specification matters. | ||
|
|
||
| Args: | ||
| include: The field inclusion specification. Can be: | ||
| - None: Include no fields (converted to empty set) | ||
| - "all": Include all fields | ||
| - list or set of field names: Include only these specific fields | ||
| Defaults to None. | ||
| purpose: The purpose of the DTO being configured (READ, WRITE, or COMPLETE). | ||
| Defaults to Purpose.READ. | ||
|
|
||
| Returns: | ||
| A new DTOConfig instance with the specified include and purpose settings. | ||
| All other configuration parameters use their defaults. | ||
| """ | ||
| return cls(purpose, include=set() if include is None else include) | ||
|
Comment on lines
+199
to
+220
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick | 🔵 Trivial Consider the past suggestion to make A previous review recommended making the 🔎 Reference to past reviewThe past review suggested: @classmethod
def from_include(cls, *, include: IncludeFields | None = None, purpose: Purpose = Purpose.READ) -> Self:This would require callers to explicitly use 🤖 Prompt for AI Agents |
||
|
|
||
| def copy_with( | ||
| self, | ||
| purpose: Purpose | type[DTOUnset] = DTOUnset, | ||
| include: IncludeFields | None = None, | ||
| exclude: ExcludeFields | None = None, | ||
| exclude: FieldIterable | None = None, | ||
| partial: bool | None | type[DTOUnset] = DTOUnset, | ||
| unset_sentinel: Any | type[DTOUnset] = DTOUnset, | ||
| type_overrides: Mapping[Any, Any] | type[DTOUnset] = DTOUnset, | ||
|
|
@@ -265,3 +288,24 @@ def alias(self, name: str) -> str | None: | |
| if self.alias_generator is not None: | ||
| return self.alias_generator(name) | ||
| return None | ||
|
|
||
| def is_field_included(self, name: str) -> bool: | ||
| """Check if a field should be included based on this configuration. | ||
|
|
||
| Evaluates field inclusion using the following rules: | ||
| 1. If include="all": the field is included unless explicitly excluded | ||
| 2. If include is a specific list/set: the field is included only if named | ||
| 3. If include is empty: the field is never included | ||
| 4. Regardless of include, the field is excluded if it's in the exclude set | ||
|
|
||
| This method is used during DTO factory operations to determine which fields | ||
| from the source model should be included in the generated DTO. | ||
|
|
||
| Args: | ||
| name: The field name to check for inclusion. | ||
|
|
||
| Returns: | ||
| True if the field should be included based on the include/exclude rules, | ||
| False otherwise. | ||
| """ | ||
| return (name in self.include or self.include == "all") and name not in self.exclude | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix hyphenation in configuration table.
"snake cased names" should be hyphenated as "snake-cased names" for proper English grammar.
🔎 Proposed fix
📝 Committable suggestion
🧰 Tools
🪛 LanguageTool
[grammar] ~2105-~2105: Use a hyphen to join words.
Context: ... | Automatically convert snake cased names to camel case in GraphQL sch...
(QB_NEW_EN_HYPHEN)
🤖 Prompt for AI Agents