-
Notifications
You must be signed in to change notification settings - Fork 2.2k
feat: Add ListAcceptedAssignments and GetAssignmentGrades methods to Classroom API #3732
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
Conversation
Fixes: #3684. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #3732 +/- ##
==========================================
- Coverage 91.11% 91.11% -0.01%
==========================================
Files 187 187
Lines 16686 16697 +11
==========================================
+ Hits 15204 15213 +9
- Misses 1295 1296 +1
- Partials 187 188 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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.
Thank you, @jferrl!
LGTM.
Awaiting second LGTM+Approval from any other contributor to this repo before merging.
cc: @stevehipwell - @alexandear
github/classroom.go
Outdated
Passing *bool `json:"passing,omitempty"` | ||
CommitCount *int `json:"commit_count,omitempty"` | ||
Grade *string `json:"grade,omitempty"` | ||
Students []*User `json:"students,omitempty"` |
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 it's better to create a separate struct ClassroomUser
and use it instead of the User
?
This struct will contain only 4 fields while the User
has more than 20 fields:
"title": "Simple Classroom User",
"description": "A GitHub user simplified for Classroom.",
"type": "object",
"properties": {
"id": {
"type": "integer",
"examples": [
1
]
},
"login": {
"type": "string",
"examples": [
"octocat"
]
},
"avatar_url": {
"type": "string",
"format": "uri",
"examples": [
"https://github.com/images/error/octocat_happy.gif"
]
},
"html_url": {
"type": "string",
"format": "uri",
"examples": [
"https://github.com/octocat"
]
}
},
"required": [
"id",
"login",
"avatar_url",
"html_url"
]
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.
Done!
AssignmentName *string `json:"assignment_name,omitempty"` | ||
AssignmentURL *string `json:"assignment_url,omitempty"` | ||
StarterCodeURL *string `json:"starter_code_url,omitempty"` | ||
GithubUsername *string `json:"github_username,omitempty"` |
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.
GithubUsername *string `json:"github_username,omitempty"` | |
GitHubUsername *string `json:"github_username,omitempty"` |
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.
No, we decided years ago that in this repo, we are going to strictly follow the capitalization of the JSON tag, so that GithubUsername
in this case is correct.
If they had named the field git_hub_username
, then you would be right, but they don't, so we don't.
I'll see if I can find the conversation.
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.
Here's one instance, but am still trying to find the original decision with the repo's original author.
#2851 (comment)
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 this is what I'm thinking of, although I'm pretty sure there was a separate discussion specifically about Github
vs GitHub
.
#605
Can we improve also PR's title? The PR is definitely not a "chore" but a "feature". |
Thank you, @alexandear! |
Fixes: #3684.
Summary
This PR adds support for two missing GitHub Classroom API endpoints by implementing the
ListAcceptedAssignments
andGetAssignmentGrades
methods in theClassroomService
.Changes
New Methods
ListAcceptedAssignments
- Lists accepted assignments for a specific assignmentGetAssignmentGrades
- Retrieves assignment grades for a specific assignmentAPI Coverage
This completes the implementation of all 6 available GitHub Classroom API endpoints:
GET /assignments/{assignment_id}
→GetAssignment()
GET /assignments/{assignment_id}/accepted_assignments
→ListAcceptedAssignments()
← NEWGET /assignments/{assignment_id}/grades
→GetAssignmentGrades()
← NEWGET /classrooms
→ListClassrooms()
GET /classrooms/{classroom_id}
→GetClassroom()
GET /classrooms/{classroom_id}/assignments
→ListClassroomAssignments()