Skip to content

Add skip attribute to input objects #463

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

Open
mwilliammyers opened this issue Nov 20, 2019 · 6 comments
Open

Add skip attribute to input objects #463

mwilliammyers opened this issue Nov 20, 2019 · 6 comments
Assignees
Labels
enhancement Improvement of existing features or bugfix

Comments

@mwilliammyers
Copy link

What do you think about adding a #[graphql(skip)] attribute for deriving GraphQLInputObject like GraphQLObject?

I often find myself wanting to skip fields for input objects (e.g. a database id that should be auto generated and not set by users).

Describe the solution you'd like
It would be awesome to be able to do something like:

#[derive(juniper::GraphQLScalarValue)]
struct Id(String); // or `uuid::Uuid` etc.

impl Default for Id {
    fn default() -> Self {
        Id::generate()
    }
}
#[derive(serde::Serialize, juniper::GraphQLInputObject)]
struct UserInput {
    #[graphql(skip)]
    #[serde(default)]
    id: Id,
    
    name: String,
}

Describe alternatives you've considered

  • Creating a separate struct with the same fields + the additional skipped fields and using that for the db. This is what I am currently doing and it is rather cumbersome and brittle. I would prefer to not have to use proc-macros to keep the fields of the two in sync (and to auto-generate From impls because that seems overkill and my project already takes way too long to compile on very fast hardware.
  • Preprocessing the documents in the database to auto generate ids and set the id field. I would love to do this but Elasticsearch doesn't support this (see [this] issue (Ingest processor cannot access _id on autogenerated id elastic/elasticsearch#41163) and this issue).

Additional context
In reality I would use it more like this with the elastic crate:

#[derive(serde::Serialize, juniper::GraphQLInputObject, elastic_derive::ElasticType)]
struct UserInput {
    #[graphql(skip)]
    #[elastic(id)]
    #[serde(default)]
    id: Id,
    
    name: String,
}

#[derive(serde::Deserialize, juniper::GraphQLObject, elastic_derive::ElasticType)]
struct User {
    // GraphQL API consumers should be able to *view* but not set the id field.
    #[elastic(id)]
    id: Id,
    
    name: String,
}

This would tell elastic to use the id field as the document ID and would create a document that looks like:

{
  "_index": "users",
  "_type": "_doc",
  "_id": "31cfad4a-814d-4e0b-bea9-52ceaefa8f96",
  "_source": {
    "id": "31cfad4a-814d-4e0b-bea9-52ceaefa8f96",
    "name": "Bob"
  }
}
@mwilliammyers mwilliammyers added the enhancement Improvement of existing features or bugfix label Nov 20, 2019
@LegNeato
Copy link
Member

Sure, this seems like something we should do for consistency.

@mwilliammyers
Copy link
Author

Ok. I’ll see if I can open a PR for it.

@LegNeato
Copy link
Member

Sounds good! You should probably hold off until we merge the async branch onto master (extremely soon)

@nebnes
Copy link

nebnes commented Dec 14, 2020

@mwilliammyers I am looking for add #[graphql(skip)] on GraphQLInputObject too.
Have you news about this possible feature ?

@ilslv
Copy link
Member

ilslv commented Apr 1, 2022

@tyranron should we add this feature as a part of #[derive(GraphQLInputObject)] or push users to create another struct with From impl? I'm in favour of this, mainly because we already allow skip on #[derive(GraphQLObject)], so Rust structs are already may not be describing pure GraphQL schema. Also approach with From impl isn't very refactoring friendly, as adding a field to a input object wouldn't raise error for missing field on From target.

@tyranron
Copy link
Member

tyranron commented Apr 1, 2022

@ilslv I don't really like the idea of pushing users to something. We have already possibility to provide From for another struct. Having #[graphql(ignore)] in addition to that will be quite handy (imagine PhantomData or PhantomPinned field for whatever purpose).

Of course, we should require the Default impl for the skipped fields.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improvement of existing features or bugfix
Projects
None yet
Development

No branches or pull requests

5 participants