Skip to content

Commit 901020c

Browse files
steipetesteipete
committed
fix: normalize classroom assignees + lint cleanup (#74) (thanks @salmonumbrella)
1 parent 7cf8b91 commit 901020c

11 files changed

Lines changed: 131 additions & 46 deletions

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
- Gmail: include `gmail.settings.sharing` scope for filter operations to avoid 403 insufficientPermissions. (#69) — thanks @ryanh-ai.
88
- Gmail: resync on stale history 404s and skip missing message fetches without masking non-404 failures. (#70) — thanks @antons.
9+
- Classroom: normalize assignee updates + fix grade update masks. (#74) — thanks @salmonumbrella.
910

1011
### Build
1112

‎internal/cmd/classroom_announcements.go‎

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -317,15 +317,13 @@ func (c *ClassroomAnnouncementsAssigneesCmd) Run(ctx context.Context, flags *Roo
317317
return usage("empty announcementId")
318318
}
319319

320-
req := &classroom.ModifyAnnouncementAssigneesRequest{}
321-
if mode := strings.TrimSpace(c.Mode); mode != "" {
322-
req.AssigneeMode = strings.ToUpper(mode)
323-
}
324-
if len(c.AddStudents) > 0 || len(c.RemoveStudents) > 0 {
325-
req.ModifyIndividualStudentsOptions = &classroom.ModifyIndividualStudentsOptions{
326-
AddStudentIds: c.AddStudents,
327-
RemoveStudentIds: c.RemoveStudents,
328-
}
320+
mode, opts, err := normalizeAssigneeMode(c.Mode, c.AddStudents, c.RemoveStudents)
321+
if err != nil {
322+
return usage(err.Error())
323+
}
324+
req := &classroom.ModifyAnnouncementAssigneesRequest{
325+
AssigneeMode: mode,
326+
ModifyIndividualStudentsOptions: opts,
329327
}
330328
if req.AssigneeMode == "" && req.ModifyIndividualStudentsOptions == nil {
331329
return usage("no assignee changes specified")

‎internal/cmd/classroom_coursework.go‎

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -446,15 +446,13 @@ func (c *ClassroomCourseworkAssigneesCmd) Run(ctx context.Context, flags *RootFl
446446
return usage("empty courseworkId")
447447
}
448448

449-
req := &classroom.ModifyCourseWorkAssigneesRequest{}
450-
if mode := strings.TrimSpace(c.Mode); mode != "" {
451-
req.AssigneeMode = strings.ToUpper(mode)
452-
}
453-
if len(c.AddStudents) > 0 || len(c.RemoveStudents) > 0 {
454-
req.ModifyIndividualStudentsOptions = &classroom.ModifyIndividualStudentsOptions{
455-
AddStudentIds: c.AddStudents,
456-
RemoveStudentIds: c.RemoveStudents,
457-
}
449+
mode, opts, err := normalizeAssigneeMode(c.Mode, c.AddStudents, c.RemoveStudents)
450+
if err != nil {
451+
return usage(err.Error())
452+
}
453+
req := &classroom.ModifyCourseWorkAssigneesRequest{
454+
AssigneeMode: mode,
455+
ModifyIndividualStudentsOptions: opts,
458456
}
459457
if req.AssigneeMode == "" && req.ModifyIndividualStudentsOptions == nil {
460458
return usage("no assignee changes specified")

‎internal/cmd/classroom_helpers.go‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,28 @@ func updateMask(fields []string) string {
118118
return strings.Join(fields, ",")
119119
}
120120

121+
func normalizeAssigneeMode(mode string, addStudents, removeStudents []string) (string, *classroom.ModifyIndividualStudentsOptions, error) {
122+
mode = strings.TrimSpace(mode)
123+
hasChanges := len(addStudents) > 0 || len(removeStudents) > 0
124+
if hasChanges {
125+
if mode == "" {
126+
mode = "INDIVIDUAL_STUDENTS"
127+
}
128+
mode = strings.ToUpper(mode)
129+
if mode != "INDIVIDUAL_STUDENTS" {
130+
return "", nil, fmt.Errorf("assignee mode must be INDIVIDUAL_STUDENTS when modifying individual students")
131+
}
132+
return mode, &classroom.ModifyIndividualStudentsOptions{
133+
AddStudentIds: addStudents,
134+
RemoveStudentIds: removeStudents,
135+
}, nil
136+
}
137+
if mode == "" {
138+
return "", nil, nil
139+
}
140+
return strings.ToUpper(mode), nil, nil
141+
}
142+
121143
func parseFloat(value string) (float64, error) {
122144
value = strings.TrimSpace(value)
123145
if value == "" {

‎internal/cmd/classroom_helpers_test.go‎

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -431,6 +431,87 @@ func TestUpdateMask(t *testing.T) {
431431
}
432432
}
433433

434+
func TestNormalizeAssigneeMode(t *testing.T) {
435+
tests := []struct {
436+
name string
437+
mode string
438+
add []string
439+
remove []string
440+
wantMode string
441+
wantOpts bool
442+
wantAdd []string
443+
wantRemove []string
444+
wantErrSubstr string
445+
}{
446+
{
447+
name: "no mode or students returns empty",
448+
wantMode: "",
449+
wantOpts: false,
450+
},
451+
{
452+
name: "mode only uppercases",
453+
mode: "all_students",
454+
wantMode: "ALL_STUDENTS",
455+
wantOpts: false,
456+
},
457+
{
458+
name: "students default mode",
459+
add: []string{"a", "b"},
460+
remove: []string{"c"},
461+
wantMode: "INDIVIDUAL_STUDENTS",
462+
wantOpts: true,
463+
wantAdd: []string{"a", "b"},
464+
wantRemove: []string{"c"},
465+
},
466+
{
467+
name: "students with explicit mode",
468+
mode: "INDIVIDUAL_STUDENTS",
469+
add: []string{"a"},
470+
wantMode: "INDIVIDUAL_STUDENTS",
471+
wantOpts: true,
472+
wantAdd: []string{"a"},
473+
},
474+
{
475+
name: "students with invalid mode errors",
476+
mode: "ALL_STUDENTS",
477+
add: []string{"a"},
478+
wantErrSubstr: "INDIVIDUAL_STUDENTS",
479+
},
480+
}
481+
482+
for _, tt := range tests {
483+
t.Run(tt.name, func(t *testing.T) {
484+
gotMode, gotOpts, err := normalizeAssigneeMode(tt.mode, tt.add, tt.remove)
485+
if tt.wantErrSubstr != "" {
486+
if err == nil {
487+
t.Fatal("expected error, got nil")
488+
}
489+
if !strings.Contains(err.Error(), tt.wantErrSubstr) {
490+
t.Fatalf("error %q does not contain %q", err.Error(), tt.wantErrSubstr)
491+
}
492+
return
493+
}
494+
if err != nil {
495+
t.Fatalf("unexpected error: %v", err)
496+
}
497+
if gotMode != tt.wantMode {
498+
t.Errorf("mode = %q, want %q", gotMode, tt.wantMode)
499+
}
500+
if (gotOpts != nil) != tt.wantOpts {
501+
t.Fatalf("opts nil = %v, want %v", gotOpts == nil, !tt.wantOpts)
502+
}
503+
if tt.wantOpts {
504+
if strings.Join(gotOpts.AddStudentIds, ",") != strings.Join(tt.wantAdd, ",") {
505+
t.Errorf("add = %v, want %v", gotOpts.AddStudentIds, tt.wantAdd)
506+
}
507+
if strings.Join(gotOpts.RemoveStudentIds, ",") != strings.Join(tt.wantRemove, ",") {
508+
t.Errorf("remove = %v, want %v", gotOpts.RemoveStudentIds, tt.wantRemove)
509+
}
510+
}
511+
})
512+
}
513+
}
514+
434515
func TestParseFloat(t *testing.T) {
435516
tests := []struct {
436517
name string

‎internal/cmd/classroom_submissions.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -288,15 +288,15 @@ func (c *ClassroomSubmissionsGradeCmd) Run(ctx context.Context, flags *RootFlags
288288
return usage(parseErr.Error())
289289
}
290290
sub.DraftGrade = grade
291-
fields = append(fields, "draft_grade")
291+
fields = append(fields, "draftGrade")
292292
}
293293
if strings.TrimSpace(c.Assigned) != "" {
294294
grade, parseErr := parseFloat(c.Assigned)
295295
if parseErr != nil {
296296
return usage(parseErr.Error())
297297
}
298298
sub.AssignedGrade = grade
299-
fields = append(fields, "assigned_grade")
299+
fields = append(fields, "assignedGrade")
300300
}
301301
if len(fields) == 0 {
302302
return usage("no grades specified")

‎internal/cmd/gmail_watch_server.go‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -468,10 +468,7 @@ func isStaleHistoryError(err error) bool {
468468
func isNotFoundAPIError(err error) bool {
469469
var gerr *googleapi.Error
470470
if errors.As(err, &gerr) {
471-
if gerr.Code != http.StatusNotFound {
472-
return false
473-
}
474-
return true
471+
return gerr.Code == http.StatusNotFound
475472
}
476473
return false
477474
}

‎internal/cmd/sheets_format.go‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -44,18 +44,15 @@ func (c *SheetsFormatCmd) Run(ctx context.Context, flags *RootFlags) error {
4444
}
4545

4646
var format sheets.CellFormat
47-
if err := json.Unmarshal([]byte(c.FormatJSON), &format); err != nil {
47+
if err = json.Unmarshal([]byte(c.FormatJSON), &format); err != nil {
4848
return fmt.Errorf("invalid format JSON: %w", err)
4949
}
5050

51-
normalizedFields, formatJSONPaths, err := normalizeFormatMask(formatFields)
52-
if err != nil {
53-
return err
54-
}
51+
normalizedFields, formatJSONPaths := normalizeFormatMask(formatFields)
5552
if normalizedFields != "" {
5653
formatFields = normalizedFields
5754
}
58-
if err := applyForceSendFields(&format, formatJSONPaths); err != nil {
55+
if err = applyForceSendFields(&format, formatJSONPaths); err != nil {
5956
return err
6057
}
6158

‎internal/cmd/sheets_format_fields.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,10 @@ import (
88
"google.golang.org/api/sheets/v4"
99
)
1010

11-
func normalizeFormatMask(mask string) (string, []string, error) {
11+
func normalizeFormatMask(mask string) (string, []string) {
1212
parts := splitFieldMask(mask)
1313
if len(parts) == 0 {
14-
return "", nil, nil
14+
return "", nil
1515
}
1616

1717
normalized := make([]string, 0, len(parts))
@@ -40,7 +40,7 @@ func normalizeFormatMask(mask string) (string, []string, error) {
4040
}
4141
}
4242

43-
return strings.Join(normalized, ","), formatJSONPaths, nil
43+
return strings.Join(normalized, ","), formatJSONPaths
4444
}
4545

4646
func applyForceSendFields(format *sheets.CellFormat, formatPaths []string) error {

‎internal/cmd/sheets_format_fields_test.go‎

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -55,10 +55,7 @@ func hasString(values []string, target string) bool {
5555
}
5656

5757
func TestNormalizeFormatMask(t *testing.T) {
58-
normalized, paths, err := normalizeFormatMask("textFormat.bold, userEnteredFormat.textFormat.italic, userEnteredValue")
59-
if err != nil {
60-
t.Fatalf("normalizeFormatMask: %v", err)
61-
}
58+
normalized, paths := normalizeFormatMask("textFormat.bold, userEnteredFormat.textFormat.italic, userEnteredValue")
6259
if normalized != "userEnteredFormat.textFormat.bold,userEnteredFormat.textFormat.italic,userEnteredValue" {
6360
t.Fatalf("unexpected normalized mask: %s", normalized)
6461
}
@@ -68,10 +65,7 @@ func TestNormalizeFormatMask(t *testing.T) {
6865
}
6966

7067
func TestNormalizeFormatMask_UserEnteredFormatOnly(t *testing.T) {
71-
normalized, paths, err := normalizeFormatMask("userEnteredFormat")
72-
if err != nil {
73-
t.Fatalf("normalizeFormatMask: %v", err)
74-
}
68+
normalized, paths := normalizeFormatMask("userEnteredFormat")
7569
if normalized != "userEnteredFormat" {
7670
t.Fatalf("unexpected normalized mask: %s", normalized)
7771
}
@@ -81,10 +75,7 @@ func TestNormalizeFormatMask_UserEnteredFormatOnly(t *testing.T) {
8175
}
8276

8377
func TestNormalizeFormatMask_LeavesUnknowns(t *testing.T) {
84-
normalized, paths, err := normalizeFormatMask("note")
85-
if err != nil {
86-
t.Fatalf("normalizeFormatMask: %v", err)
87-
}
78+
normalized, paths := normalizeFormatMask("note")
8879
if normalized != "note" {
8980
t.Fatalf("unexpected normalized mask: %s", normalized)
9081
}

0 commit comments

Comments
 (0)