Fetching multiple new assignments from tests generates a single assignment about quickfeed

[meling]

@JosteinLindhom To debug this, you will want to look in assignments/assignments_parser.go. Suggest to add the problematic assignment.yml example as a test case to figure out why it isn't providing parse error in the log, ref. Line 30:

err := yaml.Unmarshal(contents, &newAssignment)

(I think my recollection is that this function doesn't return an error for a yaml field (assignmentid) that is missing a corresponding field in the struct. Which is should according to the documentation.)

[meling]

If my recollection mentioned above is correct, then this is really annoying. Would be great if you can confirm this. Moreover, if that is the case, then readTestsRepositoryContent will return partially initialized assignments ([]*qf.Assignment), which may be written to the database. We should prevent "partially" filled assignments from being written to the database.

Regarding the particular issue of having to invoke UpdateAssignments; the database code for this method has always been a bit problematic I think. I suggest to add some more test cases to try to trigger the particular scenario with "partial" assignments, and see if there is a problem with the implementation of said method. It looks like we have only a simple test TestUpdateAssignment for this in gormdb_assignment_test.go.

[JosteinLindhom]

Jotting down my findings after taking a cursory look:

Seems like yaml.Unmarshal(...) returns an error if the field has the incorrect type, e.g. order: "hello".

yaml.UnmarshalStrict(...) will return an error if an included yaml field does not correspond to any struct fields.
However, we include fields such as title and effort for other tooling to generate README.md

As for the issue with UpdateAssignments, I've found these issues with db.UpdateAssignments:

• If any one assignment does not have Order > 0, the entire transaction is rolled back.

This explains the issue where no assignments were added when assignmentid was used instead of order

return db.conn.Transaction(func(tx *gorm.DB) error {
	for _, v := range assignments {
		if err := check(tx, v); err != nil {
			return err // if any assignment fails the check, rollback and stops processing assignments
		}
		...
	}
}

• If any one assignment does not already exist, that assignment is created and the function returns.

This explains why I had to invoke the method three times:

return db.conn.Transaction(func(tx *gorm.DB) error {
	for _, v := range assignments {
	        ...
		var assignment qf.Assignment
		if tx.Model(&qf.Assignment{}).FirstOrInit(&assignment,
			&qf.Assignment{
				CourseID: v.CourseID,
				Order:    v.Order,
			},
		).RowsAffected == 0 {
			// Zero rows affected indicates that the assignment does not exist
			return tx.Model(&qf.Assignment{}).Create(v).Error // commits or rolls back transaction. no further assignments processed
		}
		...
	}
}

[meling]

PR #1002 makes the most trivial fix for a bad assignment.yml file, which is the main thing we want to fix here, so that we can more easily discover why and which assignment.yml file failed to parse correctly. We can also avoid multiple assignments in a single transaction, avoiding the other problem.

@JosteinLindhom Can you please add some tests for this stuff. Just commit to the same PR.

PS: Seems like something doesn't like that I used go 1.20 for GitHub workflows... (didn't have time to debug why... I hate workflows.)