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

Add transformations phase #3

Merged
merged 6 commits into from
Sep 10, 2024
Merged

Add transformations phase #3

merged 6 commits into from
Sep 10, 2024

Conversation

twist900
Copy link
Member

@twist900 twist900 commented Sep 9, 2024

Identifies input nodes with transformation metadata and applies
them to the values. These transformations can be applied to both single-value
nodes and lists of items.

New transformations can be added in the lib/transforms/ directory, like
AbsintheHelpers.Transforms.ToInteger, or within your own project,
as long as they implement the same behaviour.

Example Usage

To add this phase to your pipeline, add the following to your router:

forward "/graphql",
    to: Absinthe.Plug,
    init_opts: [
      schema: MyProject.Schema,
      pipeline: {__MODULE__, :absinthe_pipeline},
    ]

def absinthe_pipeline(config, opts) do
  config
  |> Absinthe.Plug.default_pipeline(opts)
  |> AbsintheHelpers.Phase.ApplyTransforms.add_to_pipeline(opts)
end

To define a custom transformation on a schema field:

  alias AbsintheHelpers.Transforms.ToInteger
  alias AbsintheHelpers.Transforms.Trim
  alias AbsintheHelpers.Transforms.Increment

  field :employee_id, :id do
    meta transforms: [Trim, ToInteger, {Increment, 3}]
  end

or on a list:

  field :employee_ids, non_null(list_of(non_null(:id))) do
    meta transforms: [Trim, ToInteger, {Increment, 3}]
  end

To define a custom transformation on a schema arg:

  field(:create_booking, :string) do
    arg(:employee_id, non_null(:id),
      __private__: [meta: [transforms: [Trim, ToInteger, {Increment, 3}]]]
    )

    resolve(&TestResolver.run/3)
  end

In this case, both the Trim, ToInteger, and Increment will be applied
to the employee_id field.

@twist900 twist900 self-assigned this Sep 9, 2024
@twist900 twist900 force-pushed the add-transformations-phase branch from c529806 to f852e33 Compare September 9, 2024 12:35
@twist900 twist900 force-pushed the add-transformations-phase branch 4 times, most recently from b721be7 to d93e928 Compare September 9, 2024 12:54
Comment on lines 153 to 157
transform_camelized =
transform_name
|> Atom.to_string()
|> Macro.camelize()

transform_module =
String.to_existing_atom("Elixir.AbsintheHelpers.Transforms.#{transform_camelized}Transform")

apply(transform_module, :call, [input | transform_args])

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚳

please no - I hate this pattern.

We can use full module names in as the parameters like

        meta transforms: [AbsintheHelpers.Transforms.ToInteger]

That won't be a problem with with alias, and will help a lot with the code navigation

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see your point, but this is what allows us to define custom transforms directly in the project that uses this lib. See PR/module doc:

New transformations can be added in the lib/transforms/ directory, like
AbsintheHelpers.Transforms.ToIntegerTransform, or within your own project,
as long as they follow the convention. For example, you could define a new
:increment transformation tag and create a corresponding
AbsintheHelpers.Transforms.IncrementTransform to increment numeric input
values.

Maybe we can leave it until we figure out how to use dynamic meta to directly pass custom transform functions to this phase, see here

Copy link

@mpmiszczyk mpmiszczyk Sep 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like you are making my point.

If I were to pass the module name (aliased), then I could define simply MyApp.Transforms.Increment and pass it there (as long at is will follow the behaviour). And I don't need to create module with "namespace" of the helpers lib.

You can not just override the module from deps - that's an compilation warning, and most apps won't build.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice, updating it now 👍

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done ✅ , I like it! thanks e2eb945

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

going to update the docs now

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

docs updated ✅

Comment on lines 160 to 157

apply(transform_module, :call, [input | transform_args])

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe that's an bug

Suggested change
apply(transform_module, :call, [input | transform_args])
apply(transform_module, :call, [input, transform_args])

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice one, now fixed here

@twist900 twist900 force-pushed the add-transformations-phase branch from d93e928 to 7f5ef87 Compare September 9, 2024 12:58

@behaviour AbsintheHelpers.Transform

def call(%Input.Value{data: data} = item, _opts) do

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should that fail if used on any different type?

Also, it will fail with 500

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done ✅

@twist900 twist900 force-pushed the add-transformations-phase branch from c7c1002 to 5fd6c7c Compare September 9, 2024 13:37
@twist900 twist900 requested a review from mpmiszczyk September 9, 2024 14:08
@twist900 twist900 force-pushed the add-transformations-phase branch 3 times, most recently from 85d943b to 6d0c492 Compare September 9, 2024 14:35
@twist900 twist900 force-pushed the add-transformations-phase branch 2 times, most recently from 0449da7 to 32644ff Compare September 9, 2024 15:13
@twist900 twist900 force-pushed the add-transformations-phase branch from 32644ff to 9f85f6f Compare September 9, 2024 17:52
@twist900 twist900 force-pushed the add-transformations-phase branch from a6fb6d6 to 951ecb7 Compare September 10, 2024 09:29
@twist900 twist900 merged commit ebc395b into main Sep 10, 2024
4 checks passed
@twist900 twist900 deleted the add-transformations-phase branch September 10, 2024 09:36
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.

2 participants