Skip to content

Commit

Permalink
Added assignment syncing to the LMS user syncing procedure.
Browse files Browse the repository at this point in the history
  • Loading branch information
eriq-augustine committed Dec 31, 2023
1 parent 7767da9 commit 12534b5
Show file tree
Hide file tree
Showing 15 changed files with 294 additions and 76 deletions.
14 changes: 14 additions & 0 deletions _docs/assignments.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
# Autograder Assignments

## LMS Syncing Mechanics

When syncing assignment information from an LMS,
only fields not set in the assignment config will be brought over from the LMS.

Matching autograder assignments with LMS assignments are first done via the LMS id set in the assignment's config.
If no LMS is is set, then an attempt is made to match assignments via their name (if the name is not empty).
A name match is made only if an autograder assignment matches one and only one LMS assignment.

## Duplicate Assignments

Assignments in the same course may not share the same ID, name, or LMS ID.
2 changes: 1 addition & 1 deletion api/lms/routes.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import (

var routes []*core.Route = []*core.Route{
core.NewAPIRoute(core.NewEndpoint(`lms/user/get`), HandleUserGet),
core.NewAPIRoute(core.NewEndpoint(`lms/sync/users`), HandleSyncUsers),
core.NewAPIRoute(core.NewEndpoint(`lms/sync`), HandleSync),
core.NewAPIRoute(core.NewEndpoint(`lms/upload/scores`), HandleUploadScores),
};

Expand Down
19 changes: 11 additions & 8 deletions api/lms/sync_users.go → api/lms/sync.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,35 +2,38 @@ package lms

import (
"github.com/eriq-augustine/autograder/api/core"
"github.com/eriq-augustine/autograder/model"
"github.com/eriq-augustine/autograder/procedures"
)

type SyncUsersRequest struct {
type SyncRequest struct {
core.APIRequestCourseUserContext
core.MinRoleAdmin

DryRun bool `json:"dry-run"`
SkipEmails bool `json:"skip-emails"`
}

type SyncUsersResponse struct {
core.SyncUsersInfo
type SyncResponse struct {
Users *core.SyncUsersInfo `json:"users"`
Assignments *model.AssignmentSyncResult `json:"assignments"`
}

func HandleSyncUsers(request *SyncUsersRequest) (*SyncUsersResponse, *core.APIError) {
func HandleSync(request *SyncRequest) (*SyncResponse, *core.APIError) {
if (request.Course.GetLMSAdapter() == nil) {
return nil, core.NewBadRequestError("-403", &request.APIRequest, "Course is not linked to an LMS.").
Add("course", request.Course.GetID());
}

result, err := procedures.SyncAllLMSUsers(request.Course, request.DryRun, !request.SkipEmails);
result, err := procedures.SyncLMS(request.Course, request.DryRun, !request.SkipEmails);
if (err != nil) {
return nil, core.NewInternalError("-404", &request.APIRequestCourseUserContext,
"Failed to sync LMS users.").Err(err);
"Failed to sync LMS information.").Err(err);
}

response := SyncUsersResponse{
SyncUsersInfo: *core.NewSyncUsersInfo(result),
response := SyncResponse{
Users: core.NewSyncUsersInfo(result.UserSync),
Assignments: result.AssignmentSync,
};

return &response, nil;
Expand Down
6 changes: 3 additions & 3 deletions api/lms/sync_users_test.go → api/lms/sync_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import (
"github.com/eriq-augustine/autograder/util"
)

func TestAPISyncUsers(test *testing.T) {
func TestLMSSync(test *testing.T) {
testCases := []struct{ role model.UserRole; dryRun bool; skipEmails bool; permError bool }{
{model.RoleOther, false, true, true},
{model.RoleStudent, false, true, true},
Expand All @@ -28,7 +28,7 @@ func TestAPISyncUsers(test *testing.T) {
"skip-emails": testCase.skipEmails,
};

response := core.SendTestAPIRequestFull(test, core.NewEndpoint(`lms/sync/users`), fields, nil, testCase.role);
response := core.SendTestAPIRequestFull(test, core.NewEndpoint(`lms/sync`), fields, nil, testCase.role);
if (!response.Success) {
if (testCase.permError) {
expectedLocator := "-020";
Expand All @@ -44,7 +44,7 @@ func TestAPISyncUsers(test *testing.T) {
}

// Ensure the response can unmarshal.
var responseContent SyncUsersResponse;
var responseContent SyncResponse;
util.MustJSONFromString(util.MustToJSON(response.Content), &responseContent);
}
}
11 changes: 7 additions & 4 deletions cmd/lms-sync-users/main.go → cmd/lms-sync/main.go
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
package main

import (
"fmt"

"github.com/alecthomas/kong"
"github.com/rs/zerolog/log"

"github.com/eriq-augustine/autograder/config"
"github.com/eriq-augustine/autograder/db"
"github.com/eriq-augustine/autograder/procedures"
"github.com/eriq-augustine/autograder/util"
)

var args struct {
Expand All @@ -18,7 +21,7 @@ var args struct {

func main() {
kong.Parse(&args,
kong.Description("Sync local users with matching LMS users."),
kong.Description("Sync course information from a course's LMS."),
);

err := config.HandleConfigArgs(args.ConfigArgs);
Expand All @@ -31,10 +34,10 @@ func main() {

course := db.MustGetCourse(args.Course);

result, err := procedures.SyncAllLMSUsers(course, args.DryRun, !args.SkipEmails);
result, err := procedures.SyncLMS(course, args.DryRun, !args.SkipEmails);
if (err != nil) {
log.Fatal().Err(err).Msg("Failed to sync LMS users.");
log.Fatal().Err(err).Msg("Failed to sync LMS.");
}

result.PrintReport();
fmt.Println(util.MustToJSONIndent(result));
}
8 changes: 4 additions & 4 deletions cmd/users/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ func (this *AddUser) Run(course *model.Course) error {
}

fmt.Println("Add Report:");
result.PrintReport();
fmt.Println(util.MustToJSONIndent(result));

if (this.SyncLMS) {
result, err = procedures.SyncLMSUserEmail(course, this.Email, this.DryRun, this.SendEmail);
Expand All @@ -61,7 +61,7 @@ func (this *AddUser) Run(course *model.Course) error {
}

fmt.Println("\nLMS sync report:");
result.PrintReport();
fmt.Println(util.MustToJSONIndent(result));
}

return nil;
Expand Down Expand Up @@ -89,7 +89,7 @@ func (this *AddTSV) Run(course *model.Course) error {

if (this.DryRun) {
fmt.Println("Doing a dry run, users file will not be written to.");
result.PrintReport();
fmt.Println(util.MustToJSONIndent(result));
}

if (this.SyncLMS) {
Expand All @@ -105,7 +105,7 @@ func (this *AddTSV) Run(course *model.Course) error {

if (this.DryRun) {
fmt.Println("LMS sync report:");
result.PrintReport();
fmt.Println(util.MustToJSONIndent(result));
}
}

Expand Down
13 changes: 13 additions & 0 deletions common/timestamp.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,19 @@ func MustTimestampFromString(text string) Timestamp {
return instance;
}

func (this Timestamp) Validate() error {
if (this.IsZero()) {
return nil;
}

_, err := this.Time();
if (err != nil) {
return err;
}

return nil;
}

func (this Timestamp) IsZero() bool {
return (string(this) == "");
}
Expand Down
6 changes: 3 additions & 3 deletions db/database.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,14 +44,14 @@ type Backend interface {
// Returns (nil, nil) if the course does not exist.
GetCourse(courseID string) (*model.Course, error);

// Load a course into the databse and return the course.
// Load a course into the database and return the course.
// This implies loading a course directory from a config and saving it in the db.
// Will search for and load any assignments, users, and submissions
// located in the same directory tree.
// Override any existing settings for this course.
LoadCourse(path string) (*model.Course, error);

// Explicitly save a course.
// Explicitly save a course (which includes all course assignments).
SaveCourse(course *model.Course) error;

// Dump a database course to the standard directory layout
Expand All @@ -74,7 +74,7 @@ type Backend interface {
// Remove a user.
// Do nothing and return nil if the user does not exist.
RemoveUser(course *model.Course, email string) error;

// Remove a submission.
// Return a bool indicating whether the submission exists or not and an error if there is one.
RemoveSubmission(assignment *model.Assignment, email string, submissionID string) (bool, error);
Expand Down
16 changes: 12 additions & 4 deletions model/assignment.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ type Assignment struct {
Name string `json:"name"`
SortID string `json:"sort-id,omitempty"`

DueDate common.Timestamp `json:"due-date,omitempty"`
MaxPoints float64 `json:"max-points,omitempty"`

LMSID string `json:"lms-id,omitempty"`
LatePolicy *LateGradingPolicy `json:"late-policy,omitempty"`

Expand Down Expand Up @@ -86,16 +89,21 @@ func (this *Assignment) Validate() error {
return fmt.Errorf("No course found for assignment.")
}

if (this.Name == "") {
this.Name = this.ID;
}

var err error;
this.ID, err = common.ValidateID(this.ID);
if (err != nil) {
return err;
}

err = this.DueDate.Validate();
if (err != nil) {
return fmt.Errorf("Due date is not a valid timestamp: '%w'.", err);
}

if (this.MaxPoints < 0.0) {
return fmt.Errorf("Max points cannot be negative: %f.", this.MaxPoints);
}

this.imageLock = &sync.Mutex{};

// Inherit submission limit from course or leave nil.
Expand Down
9 changes: 3 additions & 6 deletions model/assignment_load.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,10 @@ func ReadAssignmentConfig(course *Course, path string) (*Assignment, error) {
return nil, fmt.Errorf("Failed to validate assignment config (%s): '%w'.", path, err);
}

otherAssignment := course.GetAssignment(assignment.GetID());
if (otherAssignment != nil) {
return nil, fmt.Errorf(
"Found multiple assignments with the same ID ('%s'): ['%s', '%s'].",
assignment.GetID(), otherAssignment.GetSourceDir(), assignment.GetSourceDir());
err = course.AddAssignment(&assignment);
if (err != nil) {
return nil, fmt.Errorf("Failed to add assignment to course (%s): '%w'.", path, err);
}
course.Assignments[assignment.GetID()] = &assignment;

return &assignment, nil;
}
25 changes: 25 additions & 0 deletions model/course.go
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,31 @@ func (this *Course) Validate() error {
return nil;
}

func (this *Course) AddAssignment(assignment *Assignment) error {
for _, otherAssignment := range this.Assignments {
if (assignment.GetID() == otherAssignment.GetID()) {
return fmt.Errorf(
"Found multiple assignments with the same ID ('%s'): ['%s', '%s'].",
assignment.GetID(), otherAssignment.GetSourceDir(), assignment.GetSourceDir());
}

if ((assignment.GetName() != "") && (assignment.GetName() == otherAssignment.GetName())) {
return fmt.Errorf(
"Found multiple assignments with the same name ('%s'): ['%s', '%s'].",
assignment.GetName(), otherAssignment.GetID(), assignment.GetID());
}

if ((assignment.GetLMSID() != "") && (assignment.GetLMSID() == otherAssignment.GetLMSID())) {
return fmt.Errorf(
"Found multiple assignments with the same LMS ID ('%s'): ['%s', '%s'].",
assignment.GetLMSID(), otherAssignment.GetID(), assignment.GetID());
}
}

this.Assignments[assignment.GetID()] = assignment;
return nil;
}

// Returns: (successful image names, map[imagename]error).
func (this *Course) BuildAssignmentImages(force bool, quick bool, options *docker.BuildOptions) ([]string, map[string]error) {
goodImageNames := make([]string, 0, len(this.Assignments));
Expand Down
53 changes: 22 additions & 31 deletions model/sync.go
Original file line number Diff line number Diff line change
@@ -1,11 +1,29 @@
package model

import (
"fmt"
)

type LMSSyncResult struct {
UserSync *UserSyncResult `json:"user-sync"`
AssignmentSync *AssignmentSyncResult `json:"assignment-sync"`
}

type AssignmentSyncResult struct {
SyncedAssignments []AssignmentInfo `json:"synced-assignments"`
AmbiguousMatches []AssignmentInfo `json:"ambiguous-matches"`
NonMatchedAssignments []AssignmentInfo `json:"non-matched-assignments"`
UnchangedAssignments []AssignmentInfo `json:"unchanged-assignments"`
}

type AssignmentInfo struct {
ID string `json:"id"`
Name string `json:"name"`
}

func NewAssignmentSyncResult() *AssignmentSyncResult {
return &AssignmentSyncResult{
SyncedAssignments: make([]AssignmentInfo, 0),
AmbiguousMatches: make([]AssignmentInfo, 0),
NonMatchedAssignments: make([]AssignmentInfo, 0),
UnchangedAssignments: make([]AssignmentInfo, 0),
};
}

type UserSyncResult struct {
Expand Down Expand Up @@ -47,33 +65,6 @@ func (this *UserSyncResult) Count() int {
return len(this.Add) + len(this.Mod) + len(this.Del);
}

func (this *UserSyncResult) PrintReport() {
groups := []struct{operation string; users []*User}{
{"Added", this.Add},
{"Modified", this.Mod},
{"Deleted", this.Del},
{"Skipped", this.Skip},
{"Unchanged", this.Unchanged},
};

for i, group := range groups {
if (i != 0) {
fmt.Println();
}

fmt.Printf("%s %d users.\n", group.operation, len(group.users));
for _, user := range group.users {
fmt.Println(" " + user.ToRow(", "));
}
}

fmt.Println();
fmt.Printf("Generated %d passwords.\n", len(this.ClearTextPasswords));
for email, pass := range this.ClearTextPasswords {
fmt.Printf(" %s, %s\n", email, pass);
}
}

func (this *UserSyncResult) AddResolveResult(resolveResult *UserResolveResult) {
if (resolveResult == nil) {
return;
Expand Down
Loading

0 comments on commit 12534b5

Please sign in to comment.