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

Create editExerciseComment mutation and update some snapshots for tests #2731

Merged
merged 19 commits into from
Feb 1, 2023

Conversation

HS-90
Copy link
Collaborator

@HS-90 HS-90 commented Jan 30, 2023

Description: relates to #2400

This PR adds a graphQL mutation that will handle editing of exercise comments as part of the discussions feature in C0D3 Dojo.
To test:
Use an existing exerciseComment id or create a new one in graphQL like so:

  1. Go to 'api/graphql'
  2. write a query as follows:
mutation AddExerciseComment{   
       addExerciseComment(exerciseId:1, content:"Cool")
             { id 
              exerciseId 
              content   } 
}
  1. You should receive a response such as:
{   "data": {
        "addExerciseComment": { 
            "id": 3, 
            "exerciseId": 1, 
            "content": "Cool" }   
                  }
 }

image

  1. Remember/save the id that is generated/assigned to the newly created exerciseComment

  2. Use this id to run a query such as:

mutation EditExerciseComment{   
       editExerciseComment( id: 3, content: "I edited this comment")
             { id 
              createdAt
              exerciseId
              parentId
              content   } 
}

If successful, there should be a result similar to the following:
editted exercise comment

@vercel
Copy link

vercel bot commented Jan 30, 2023

@HS-90 is attempting to deploy a commit to the c0d3-prod Team on Vercel.

A member of the Team first needs to authorize it.

@vercel
Copy link

vercel bot commented Jan 30, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated
c0d3-app ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Feb 1, 2023 at 8:56AM (UTC)

@flacial
Copy link
Member

flacial commented Jan 31, 2023

It shouldn't update any snapshots because they were updated and merged in this PR. Did you merge upstream changes and update the packages?

if (exerciseComment?.authorId !== authorId)
throw new Error('Comment is not by user')

return prisma.exerciseComment.update({
Copy link
Member

@flacial flacial Jan 31, 2023

Choose a reason for hiding this comment

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

It would be nice to include the date too. So in the app, we can show when was the comment last edited

Copy link
Collaborator Author

@HS-90 HS-90 Jan 31, 2023

Choose a reason for hiding this comment

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

@flacial Actually, I though prisma updates that automatically in the "updatedAt" field. If that's the case we wont need to manually update the time right?

Copy link
Member

Choose a reason for hiding this comment

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

You're right.

@@ -44,3 +45,28 @@ export const addExerciseComment = async (
data: { authorId, content, exerciseId, parentId, userPic }
})
}

export const editExerciseComment = async (
Copy link
Member

@flacial flacial Jan 31, 2023

Choose a reason for hiding this comment

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

Can't we wrap this with WithUserContainer higher order function because it checks if the user session exists?

@HS-90
Copy link
Collaborator Author

HS-90 commented Jan 31, 2023

It shouldn't update any snapshots because they were updated and merged in this PR. Did you merge upstream changes and update the packages?

I did update my fork and did 'git pull' in my local repo before submitting this PR. I had to update the snapshots in my local with 'yarn test -u' otherwise the snapshots failed. I'll have to look further into why it's failing the test.

@HS-90 HS-90 added the dont-merge Not ready for merge label Jan 31, 2023
@HS-90 HS-90 closed this Jan 31, 2023
@codecov
Copy link

codecov bot commented Jan 31, 2023

Codecov Report

Merging #2731 (39e003e) into master (7cec79a) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            master     #2731   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          189       189           
  Lines         3555      3562    +7     
  Branches       984       985    +1     
=========================================
+ Hits          3555      3562    +7     
Impacted Files Coverage Δ
graphql/resolvers/exerciseCommentCrud.ts 100.00% <100.00%> (ø)

@anthonykhoa
Copy link
Collaborator

Thanks for the detailed test instructions. LGTM! 👍

@HS-90 HS-90 merged commit 7981b2e into garageScript:master Feb 1, 2023
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.

4 participants