Feature/rulesets: End to End assignment creation (KHO-172, KHO-173) #162
Conversation
CamPlume1
changed the title
| @@ -139,6 +139,15 @@ WHERE student_work_id = $3 | |||
| return formatted[0], nil | |||
| } | |||
|
|
|||
| // func (db *DB) GetWorkFromRepoName(ctx context.Context, repoName string) (*models.StudentWork, error){ | |||
| // var studentWork models.StudentWork | |||
| // err := db.connPool.QueryRow(ctx, `SELECT * FROM student_works WHERE repo_name = $1`, repoName).Scan(&studentWork) | |||
There was a problem hiding this comment.
Leaving-> This is functional but throws an error because it's unused. No reason to delete working code
There was a problem hiding this comment.
YAGNI.
Also, this is already implemented below in GetWorkByRepoName.
…y-classroom-plugin into feature/rulesets
| @@ -139,6 +139,15 @@ WHERE student_work_id = $3 | |||
| return formatted[0], nil | |||
| } | |||
|
|
|||
| // func (db *DB) GetWorkFromRepoName(ctx context.Context, repoName string) (*models.StudentWork, error){ | |||
| // var studentWork models.StudentWork | |||
| // err := db.connPool.QueryRow(ctx, `SELECT * FROM student_works WHERE repo_name = $1`, repoName).Scan(&studentWork) | |||
There was a problem hiding this comment.
YAGNI.
Also, this is already implemented below in GetWorkByRepoName.
|
|
||
| func CriticalGithubError() APIError { | ||
| return NewAPIError(http.StatusInternalServerError, fmt.Errorf("critical Out of State Error: Github Integration")) | ||
| } |
There was a problem hiding this comment.
How is this different than GitHubAPI error?
There was a problem hiding this comment.
I want to have this one in the intermediary stage before I can properly implement the recovery process.
In my mind this is an error that should not be thrown in any case, and probably represents a panic state
backend/internal/handlers/classrooms/assignments/assignments.go
Outdated
Show resolved
Hide resolved
| // Retrieve assignment deadline from DB | ||
| deadline, err := s.store.GetAssignmentDueDateByRepoName(c.Context(), *pushEvent.Repo.Name) | ||
| if err == nil { | ||
| // There is a deadline |
There was a problem hiding this comment.
This is a faulty way to look for a deadline. If another error happens, the code will mistakenly proceed.
IMO, you're better off defining a function GetAssignmentByBaseRepoName and handling the logic for whether there is/isn't a due date here.
backend/internal/storage/storage.go
Outdated
| @@ -31,6 +31,7 @@ type Works interface { | |||
| GetWorks(ctx context.Context, classroomID int, assignmentID int) ([]*models.StudentWorkWithContributors, error) | |||
| GetWork(ctx context.Context, classroomID int, assignmentID int, studentWorkID int) (*models.PaginatedStudentWorkWithContributors, error) | |||
| CreateStudentWork(ctx context.Context, assignmentOutlineID int32, gitHubUserID int64, repoName string, workState models.WorkState, dueDate *time.Time) (models.StudentWork, error) | |||
| GetAssignmentDueDateByRepoName(ctx context.Context, repoName string) (*time.Time, error) | |||
There was a problem hiding this comment.
This is under the wrong type.
| @@ -50,3 +52,15 @@ func (db *DB) GetAssignmentTemplateByID(ctx context.Context, templateID int64) ( | |||
|
|
|||
| return assignmentTemplate, nil | |||
| } | |||
|
|
|||
| func (db *DB) GetAssignmentDueDateByRepoName(ctx context.Context, repoName string) (*time.Time, error){ | |||
There was a problem hiding this comment.
See comment above about generalizing this return Assignment object and doing field extraction in service layer.
| @@ -1,6 +1,7 @@ | |||
| package organizations | |||
|
|
|||
| import ( | |||
| //"fmt" | |||
Checklist