-
Notifications
You must be signed in to change notification settings - Fork 2
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
Write mutations to: create a community and join a community #41
Conversation
dc7efde
to
4d49262
Compare
): Promise<Maybe<UserDocument>> => { | ||
if (!token) return null; | ||
|
||
// TODO: Maybe it should be a crime |
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.
lol
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.
Drastic measures are sometimes necessary lmao
packages/server/src/modules/community/mutations/createCommunityMutation.ts
Outdated
Show resolved
Hide resolved
packages/server/src/modules/community/mutations/createCommunityMutation.ts
Outdated
Show resolved
Hide resolved
ctx: GraphQLContext, | ||
) => { | ||
// TODO: In some way, you can pass it to a middleware. But IDK how to do it yet. | ||
if (!ctx.user) { |
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.
packages/server/src/modules/community/mutations/createCommunityMutation.ts
Outdated
Show resolved
Hide resolved
packages/server/src/modules/community/mutations/createCommunityMutation.ts
Outdated
Show resolved
Hide resolved
resolve: async ({ communityId }) => | ||
await CommunityModel.findOne({ _id: communityId }), | ||
}, | ||
me: { |
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.
create the auth auth
middleware based on this https://github.com/daniloab/rbaf-graphql-api/blob/master/src/server/app.ts#L18
then, expose me to the query type, and should be enough. This will help you to avoid declaring me on mutation output fields.
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.
I don't understand exactly what you suggest here, how can I do it?
const community = await CommunityModel.findOne({ name: communityId }); | ||
|
||
if (!community) { | ||
throw new Error("This community doesn't exist. Please, try again."); |
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.
maybe you can return errors on graphql output mutation
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.
I was thinking about it, but for now, I intend to keep throw because I can more objectively handle the error on the front-end.
But I guess that it's one of the ways that I think to implement error handling on the "server-side". I'm just waiting to answer my question here to decide how I deal with errors.
community.updateOne({ | ||
$addToSet: { members: [...community.members, ctx.user._id] }, | ||
}), | ||
ctx.user.updateOne({ |
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.
prefer to get the user itself from MongoDB and then update it.
implement dataloaders to help you with this.
} | ||
|
||
interface UserDocument extends User, Document { | ||
export interface UserDocument extends User, Document { |
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.
IUser could be cool
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.
IMO the I
prefix on interfaces is unnecessary because in all modern editors (and IDEs) you can know that UserDocument
is an interface, I think that this prefix isn't necessary.
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.
cool
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.
awesome pr
4d49262
to
49c02f5
Compare
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.
merge this and send the changes into small prs
Sure! I'll create issues with the rest of the changes! |
Description
With this PR, we are adding these mutations:
communityCreate
andcommunityJoin
that will be responsible to create and join communities based oncommunityId
.communityCreate
To run this mutation, you just run it:
communityJoin
To run this mutation, you just run it:
Observations
koa-graphql
to^v0.12.0
;koa-router
to@koa/router
(is the same package);user
object to context from each request;