Janady13/missionmatch / internal/modules/org/settings_handler_test.go

1package org 2 3import ( 4 "encoding/json" 5 "reflect" 6 "strings" 7 "testing" 8) 9 10// THE REGRESSION. Every field on the wire must reach the update, and this has now failed TWICE in the same 11// way: a settings field was added to the store, to validation and to the SQL, and the handler's request 12// struct was not updated, so the API accepted the value, returned 200, and changed nothing. Silent success is 13// the worst failure mode a settings screen can have. 14// 15// Comparing the two structs field by field catches it for every field that will ever be added. 16func TestEveryPatchFieldReachesTheUpdate(t *testing.T) { 17 req := reflect.TypeOf(patchRequest{}) 18 upd := reflect.TypeOf(patchRequest{}.toUpdate()) 19 updFields := map[string]bool{} 20 for i := 0; i < upd.NumField(); i++ { 21 updFields[upd.Field(i).Name] = true 22 } 23 for i := 0; i < req.NumField(); i++ { 24 f := req.Field(i) 25 // UNEXPORTED FIELDS ARE BOOKKEEPING, NOT SETTINGS. encoding/json cannot decode into one, so it can never 26 // carry a value from the wire and has nothing to reach on the update. defaultBurdenPresent is the case 27 // this admits: it records whether a KEY appeared, which is how an explicit null is told apart from an 28 // absent field, and both of those decode to the same nil pointer without it. 29 if !f.IsExported() { 30 continue 31 } 32 if !updFields[f.Name] { 33 t.Errorf("patchRequest.%s has no matching field on orgsettings.Update, it can never be applied", f.Name) 34 } 35 } 36 // And the other direction: a field the store can update but the API cannot express is a setting nobody 37 // can reach. 38 reqFields := map[string]bool{} 39 for i := 0; i < req.NumField(); i++ { 40 reqFields[req.Field(i).Name] = true 41 } 42 for i := 0; i < upd.NumField(); i++ { 43 name := upd.Field(i).Name 44 if !reqFields[name] { 45 t.Errorf("orgsettings.Update.%s is not exposed on patchRequest, no caller can ever set it", name) 46 } 47 } 48} 49 50// A non nil value on the wire must actually arrive on the update. The reflection test above proves the fields 51// line up; this proves the mapping function copies them rather than dropping one. 52func TestPatchMappingCopiesValues(t *testing.T) { 53 body := `{"staff_record_scope":"own","manager_record_scope":"team","unowned_records_visible":false, 54 "rate_visibility":"team","work_visibility":"private","smtp_port":587, 55 "email_delivery_enabled":true,"time_zone":"UTC","claim_expiry_hours":24}` 56 var in patchRequest 57 if err := json.NewDecoder(strings.NewReader(body)).Decode(&in); err != nil { 58 t.Fatalf("decode: %v", err) 59 } 60 u := in.toUpdate() 61 uv := reflect.ValueOf(u) 62 rv := reflect.ValueOf(in) 63 rt := rv.Type() 64 for i := 0; i < rt.NumField(); i++ { 65 f := rt.Field(i) 66 name := f.Name 67 got := rv.Field(i) 68 // Same rule as above, and it must come before IsNil, which panics on a bool. 69 if !f.IsExported() { 70 continue 71 } 72 if got.Kind() == reflect.Ptr && got.IsNil() { 73 continue // absent from the body, nothing to carry 74 } 75 target := uv.FieldByName(name) 76 if !target.IsValid() || target.IsNil() { 77 t.Errorf("%s was set on the wire and is nil on the update, the value was dropped", name) 78 continue 79 } 80 // DefaultBurdenPct is the one field that deliberately changes TYPE in transit: a decimal string on the 81 // wire becomes a decimal behind a double pointer on the update, because a wrap must never round trip 82 // through a JSON float. DeepEqual would compare a string with a number and always disagree, so its 83 // carrying is proven by name below rather than by structural equality here. 84 if name == "DefaultBurdenPct" { 85 continue 86 } 87 if !reflect.DeepEqual(got.Elem().Interface(), target.Elem().Interface()) { 88 t.Errorf("%s changed value in transit: %v became %v", name, got.Elem(), target.Elem()) 89 } 90 } 91} 92 93// THE WRAP HAS THREE STATES ON THE WIRE and they mean different things, so each is pinned. The one that 94// matters most is the middle case: a PATCH that does not mention the wrap must leave it alone. The interface 95// sends a subset of fields on every save, so getting this wrong would erase an organization's cost basis the 96// next time somebody changed an unrelated setting, and every margin in the product would quietly become a 97// spread again. 98func TestTheWrapTellsAbsentApartFromCleared(t *testing.T) { 99 decode := func(body string) patchRequest { 100 var in patchRequest 101 if err := json.NewDecoder(strings.NewReader(body)).Decode(&in); err != nil { 102 t.Fatalf("decode %s: %v", body, err) 103 } 104 return in 105 } 106 107 // Absent. Leave the wrap alone. 108 if u := decode(`{"claim_expiry_hours":24}`).toUpdate(); u.DefaultBurdenPct != nil { 109 t.Error("a patch that never mentioned the wrap would have changed it") 110 } 111 112 // Explicit null. Clear it. 113 u := decode(`{"default_burden_pct":null}`).toUpdate() 114 if u.DefaultBurdenPct == nil { 115 t.Fatal("an explicit null did not reach the update, so the wrap could never be removed") 116 } 117 if *u.DefaultBurdenPct != nil { 118 t.Error("an explicit null did not clear the wrap") 119 } 120 121 // A value. Set it, exactly, with no float in the path. 122 u = decode(`{"default_burden_pct":"0.3525"}`).toUpdate() 123 if u.DefaultBurdenPct == nil || *u.DefaultBurdenPct == nil { 124 t.Fatal("a wrap on the wire did not reach the update") 125 } 126 if got := (*u.DefaultBurdenPct).String(); got != "0.3525" { 127 t.Errorf("wrap = %s, want 0.3525 exactly", got) 128 } 129} 130 131// A wrap that is not a number is REFUSED rather than treated as a clear. burdenUpdate cannot tell a typo from 132// an erasure on its own, so the handler asks parseBurden first, and this is the guard on that ordering. 133func TestAMalformedWrapIsRefusedNotTreatedAsAClear(t *testing.T) { 134 var in patchRequest 135 if err := json.NewDecoder(strings.NewReader(`{"default_burden_pct":"35%"}`)).Decode(&in); err != nil { 136 t.Fatalf("decode: %v", err) 137 } 138 if err := in.parseBurden(); err == nil { 139 t.Fatal("a wrap of \"35%\" was accepted, and it would have erased the real one") 140 } 141} 142