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

Allow "kind of" middlewares for extending user model before returning it to the client #995

Open
nsine opened this issue Jun 4, 2020 · 5 comments

Comments

@nsine
Copy link
Contributor

nsine commented Jun 4, 2020

Feature request

Is your feature request related to a problem? Please describe.

Will try to explain what i mean because not sure how clearly describe in the header 😅

On our project we use GraphQL, MongoDb and Typegoose/Mongoose as ORM. We don't use raw mongodb objects anywhere as we expect everything is wrapped to Mongoose model. In any resolver that expect user as a parent parameter - we expect it's mongoose document so we can use virtual getters/methods on it. However that's not true with accounts-js as all it's resolvers return raw mongodb object.
And e.g. if I have mongoose model

class User {
  ...
  emails: EmailEntry[];
  get isEmailVerified() {
    return this.emails[0].verified;
  }
}

and a graphql type

type User {
  ....
  isEmailVerified: Boolean!
}

if i query a user with default getUser mutation i won't be able to query isEmailVerified field as the user is raw object, not mongoose model

Describe the solution you'd like

Currently like a workaround i manually wrap user into mongoose in each resolver

const resolvers = {
  User: {
    isEmailVerified: (user) => User.hydrate(user).isEmailVerified,
  }
}

But wondering if that's a useful feature to allow customize how result User object should look like.
E.g. i pass somewhere modifyUser = (user) => User.hydrate(user) to automatically wrap it to what i need.

I didn't came to some exact solution as i'm not sure to which part of layered architecture we can add such middleware.

  • can it also be useful for rest api, not only for graphql
  • will it be architecture-correct to create another DatabaseInterface like database-mongo but database-mongoose (seems to be too much overhead)

Let me know what do you think about this (i'm sure this is useful for graphql+mongoose users). And if you have any ideas how we can implement that. I can help with implementation afterwards.

Thanks ❤️

@pradel
Copy link
Member

pradel commented Jun 4, 2020

I see, well the first thing that is coming to my mind for a generic solution would be to actually fetch the user yourself inside the graphql context. Right now we inject the user (doing a db call) inside the context, I think a good option would be to actually skip this call and just inject the user id to the context. Then you can fetch your user as you want so with mongoose in your case.

It could look like this:

const server = new ApolloServer({
    schema,
    context: async ({ req, connection }: any) => {
       const accountJsContext = await accountsGraphQL.context({ req }, { skipUserLoadOrWhateverOptionName: true });

       let user;
       if (accountJsContext.userId) {
         user = await myMongooseUserModel.findOneById(accountJsContext.userId)
       }

       return {
        ...accountJsContext,
        user
      };
    })
  });

The same behavior can also be applied to rest :)

What do you think?

@pradel
Copy link
Member

pradel commented Jun 4, 2020

will it be architecture-correct to create another DatabaseInterface like database-mongo but database-mongoose (seems to be too much overhead)

This is an option but definitely looks like too much overhead :D

@nsine
Copy link
Contributor Author

nsine commented Jun 11, 2020

@pradel yeah, for getting correct mongoose model in context we already do sth like this

export async function getWrappedAccountsContext(session) {
  const accountsContext = await accounts.context(session);
  return {
    ...accountsContext,
    user: accountsContext.user ? User.hydrate(accountsContext.user) : null,
  };
}

And calling it inside context creator fn.

My question was more about user models which are returned from queries/mutations. E.g. getUser returns raw mongodb object and we have to override that

Query: {
    // Override this mutation to return mongoose model which we already have in context
    getUser(_, __, { user }) {
      return user || null;
    },
  },

or authenticate/createUser methods which return raw object and you have to again overwrite mutations and write sth like

const res =await AccountsMutation.authenticate(...);
if (res.user) res.user = User.hydrate(res.user);
return res

@pradel
Copy link
Member

pradel commented Jul 7, 2020

@nsine okay I guess it's something not easy to do currently and probably the best way to achieve this would be to have a mongoose database adapter. But that would require to maintain one more database adapter. I was taking a look and this is the choice other projects did.

@darkbasic
Copy link
Member

Another option would be to create a directive that hydrates the user.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants