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.

Sign up for GitHub

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

Jump to bottom

Fixes #32214 - Register query field with a resolver #8415

Closed
wants to merge 1 commit into from
Closed

Fixes #32214 - Register query field with a resolver #8415

wants to merge 1 commit into from

Conversation

xprazak2
Copy link
Contributor

@xprazak2 xprazak2 commented Mar 26, 2021 •
edited
Loading 

Foreman::Plugin.register :foreman_plugin do
  register_graphql_query_field :foo, '::Types::Foo', :field, { :resolver => ::Resolvers::Foos::Foo }
  # register with lambda instead of a string if type does not resolve to a contant
  register_graphql_query_field :bars, -> () { [::Types::Bar] }, :field, { :resolver => ::Resolvers::Bars }
end

@theforeman-bot
Copy link
Member

Issues: #32214

Fixes #32214 - Register query field with a resolver
3a9003c 
Foreman::Plugin.register :foreman_plugin do
  register_graphql_query_field :foo, '::Types::Foo', :field, { :resolver => ::Resolvers::Foos::Foo }
  # register with lambda instead of a string if type does not resolve to a contant
  register_graphql_query_field :bars, -> () { [::Types::Bar] }, :field, { :resolver => ::Resolvers::Bars }
end
@ezr-ondrej ezr-ondrej self-assigned this Apr 7, 2021
ezr-ondrej
ezr-ondrej reviewed Jun 12, 2021
View reviewed changes
Copy link
Member

@ezr-ondrej ezr-ondrej left a comment

Choose a reason for hiding this comment

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

I'm fine with this, it make sense I just do not see use-case, so I'm reluctant to merge as we don't have the need for it now.

Could we at least document this in developers docs? Are there even docs for defining plugin GraphQL fields from plugin?

app/registries/foreman/plugin/graphql_plugin_fields.rb
@@ -6,7 +6,11 @@ module GraphqlPluginFields
module ClassMethods
def realize_plugin_query_extensions(source = Foreman::Plugin.graphql_types_registry.plugin_query_fields)
source.map do |plugin_type|
send plugin_type[:field_type], plugin_type[:field_name], Foreman::Module.resolve(plugin_type[:type])
if plugin_type[:options].empty?
send plugin_type[:field_type], plugin_type[:field_name], Foreman::Module.resolve(plugin_type[:type])
Copy link
Member

Choose a reason for hiding this comment

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

Why are we not allowing lambda types without options?
And why do we even have case for options, can't we just send the empty options and handle them on receiver end?

@xprazak2
Copy link
Contributor Author

I'm fine with this, it make sense I just do not see use-case, so I'm reluctant to merge as we don't have the need for it now.

You are right, this is not needed anywhere just yet. I'll close for now and reopen when it is really needed. Thanks!

@xprazak2 xprazak2 closed this Jun 14, 2021
@xprazak2 xprazak2 mentioned this pull request Sep 24, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ezr-ondrej ezr-ondrej ezr-ondrej left review comments

Assignees

@ezr-ondrej ezr-ondrej

Labels
Needs testing Not yet reviewed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@xprazak2 @theforeman-bot @ezr-ondrej