Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
# Notification Follow Preference SDK Fix

## Problem

`POST /incident/create` accepts `assigned_to.notify.follow_preference` as a
tri-state value in the backend:

- omitted: use each responder's personal notification preference;
- `true`: explicitly use personal preference;
- `false`: use `personal_channels` from the request.

The OpenAPI schema currently models the field as an optional, non-nullable
boolean. The SDK generator therefore emits:

```go
FollowPreference bool `json:"follow_preference,omitempty"`
```

Go's JSON encoder omits `false`, so SDK callers cannot send the value required
to force `personal_channels`.

## Design

Model `follow_preference` as a nullable boolean in the OpenAPI 3.1 source:

```json
"type": ["boolean", "null"]
```

The existing generator already maps nullable request scalars to pointers. The
generated SDK field becomes:

```go
FollowPreference *bool `json:"follow_preference,omitempty"`
```

Callers use `flashduty.Bool(false)` to send an explicit false value. A nil
pointer remains omitted.

The same request contract is used by incident creation and responder addition,
so both request schemas must be corrected. The incident assignment schema must
also expose `assigned_to.notify`, which the backend accepts through the shared
assignment structure.

## Source And Generation Flow

1. Correct the English and Chinese on-call OpenAPI source in
`flashduty-docs`.
2. Regenerate the consolidated bilingual OpenAPI artifacts there.
3. Sync those artifacts into `go-flashduty`.
4. Run the SDK generator; do not edit `models_gen.go` directly.

## Compatibility

Changing `FollowPreference` from `bool` to `*bool` is a source-level change for
callers that initialize this field. Release it as the next v0 minor version,
not as a patch release.

Removing `omitempty` is rejected because it would silently send `false` for a
notification override that only specifies `template_id`, changing that request
from personal-preference delivery to an empty explicit channel override.

## Verification

- Generator test: an optional nullable request boolean generates `*bool` with
`omitempty`.
- Wire test: `flashduty.Bool(false)` serializes as
`"follow_preference":false`.
- Wire test: nil omits `follow_preference`.
- Cover both incident creation and responder addition.
- Regenerate twice and confirm no diff on the second run.
- Run `make check` in `go-flashduty`.

## Success Criteria

An SDK request with `PersonalChannels: []string{"sms"}` and
`FollowPreference: flashduty.Bool(false)` sends both fields, allowing the
backend to select SMS explicitly.
109 changes: 109 additions & 0 deletions flashduty_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package flashduty

import (
"context"
"encoding/json"
"errors"
"io"
"net/http"
Expand Down Expand Up @@ -133,6 +134,114 @@ func TestResetPostMortemContentSendsZeroExpectedRevision(t *testing.T) {
}
}

func TestIncidentNotificationOverridePreservesExplicitFalse(t *testing.T) {
c, _ := NewClient("KEY", WithBaseURL("https://api.flashcat.cloud"), WithLogger(noopLogger{}))

tests := []struct {
name string
path string
body any
notifyPath []string
}{
{
name: "create incident",
path: "/incident/create",
body: &CreateIncidentRequest{
IncidentSeverity: "Critical",
AssignedTo: CreateIncidentRequestAssignedTo{
PersonIDs: []int64{1},
Notify: CreateIncidentRequestAssignedToNotify{
FollowPreference: Bool(false),
PersonalChannels: []string{"sms"},
},
},
},
notifyPath: []string{"assigned_to", "notify"},
},
{
name: "add responder",
path: "/incident/responder/add",
body: &AddIncidentResponderRequest{
IncidentID: "0123456789abcdef01234567",
PersonIDs: []int64{1},
Notify: AddIncidentResponderRequestNotify{
FollowPreference: Bool(false),
PersonalChannels: []string{"sms"},
},
},
notifyPath: []string{"notify"},
},
{
name: "assign incident",
path: "/incident/assign",
body: &AssignIncidentRequest{
IncidentID: "0123456789abcdef01234567",
AssignedTo: AssignedTo{
PersonIDs: []int64{1},
Notify: AssignedToNotify{
FollowPreference: Bool(false),
PersonalChannels: []string{"sms"},
},
},
},
notifyPath: []string{"assigned_to", "notify"},
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
req, err := c.newRequest(context.Background(), http.MethodPost, tt.path, tt.body)
if err != nil {
t.Fatal(err)
}
body, err := io.ReadAll(req.Body)
if err != nil {
t.Fatal(err)
}

var payload map[string]any
if err := json.Unmarshal(body, &payload); err != nil {
t.Fatal(err)
}
notify := payload
for _, key := range tt.notifyPath {
value, ok := notify[key].(map[string]any)
if !ok {
t.Fatalf("%s is missing from request body: %s", key, body)
}
notify = value
}
follow, ok := notify["follow_preference"]
if !ok || follow != false {
t.Fatalf("follow_preference = %#v, present = %t, body = %s", follow, ok, body)
}
})
}
}

func TestIncidentNotificationOverrideOmitsUnsetPreference(t *testing.T) {
c, _ := NewClient("KEY", WithBaseURL("https://api.flashcat.cloud"), WithLogger(noopLogger{}))
req, err := c.newRequest(context.Background(), http.MethodPost, "/incident/create", &CreateIncidentRequest{
IncidentSeverity: "Critical",
AssignedTo: CreateIncidentRequestAssignedTo{
PersonIDs: []int64{1},
Notify: CreateIncidentRequestAssignedToNotify{
PersonalChannels: []string{"sms"},
},
},
})
if err != nil {
t.Fatal(err)
}
body, err := io.ReadAll(req.Body)
if err != nil {
t.Fatal(err)
}
if strings.Contains(string(body), `"follow_preference"`) {
t.Fatalf("nil FollowPreference must be omitted from the wire, got body = %s", body)
}
}

func TestNewRequestAppliesHookAndHeaders(t *testing.T) {
c, _ := NewClient("KEY",
WithRequestHeaders(map[string][]string{"X-Static": {"s"}}),
Expand Down
19 changes: 19 additions & 0 deletions internal/cmd/gen/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,25 @@ func TestEmitStructRequiredNullableRequestScalarOmitsNil(t *testing.T) {
}
}

func TestEmitStructOptionalNullableRequestBoolPreservesFalse(t *testing.T) {
g := newTestGen(map[string]any{})
g.reqGoNames["NotifyRequest"] = true

schema := map[string]any{
"type": "object",
"properties": map[string]any{
"follow_preference": map[string]any{
"type": []any{"boolean", "null"},
},
},
}

src := g.emitStruct("NotifyRequest", schema)
if !strings.Contains(src, `FollowPreference *bool `+"`"+`json:"follow_preference,omitempty" toon:"follow_preference,omitempty"`+"`") {
t.Fatalf("optional nullable request bool must preserve explicit false; got:\n%s", src)
}
}

func TestMergeAllOfKeepsRequiredRequestFields(t *testing.T) {
g := newTestGen(map[string]any{
"BaseRequest": map[string]any{
Expand Down
32 changes: 28 additions & 4 deletions models_gen.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

39 changes: 35 additions & 4 deletions openapi/openapi.en.json
Original file line number Diff line number Diff line change
Expand Up @@ -31156,8 +31156,11 @@
"description": "Override the notification channels used for this assignment.",
"properties": {
"follow_preference": {
"type": "boolean",
"description": "When true, fall back to each responder's personal preference."
"type": [
"boolean",
"null"
],
"description": "When false, use `personal_channels`; when true or omitted, use each responder's personal preference."
},
"personal_channels": {
"type": "array",
Expand Down Expand Up @@ -31256,6 +31259,31 @@
"maxItems": 100,
"description": "Email recipients, used by integrations such as ServiceNow."
},
"notify": {
"type": "object",
"description": "Override the notification channels used for this assignment.",
"properties": {
"follow_preference": {
"type": [
"boolean",
"null"
],
"description": "When false, use `personal_channels`; when true or omitted, use each responder's personal preference."
},
"personal_channels": {
"type": "array",
"items": {
"type": "string"
},
"description": "Channels to use (e.g. `voice`, `sms`, `email`)."
},
"template_id": {
"type": "string",
"pattern": "^[0-9a-fA-F]{24}$",
"description": "Notification template ID (MongoDB ObjectID)."
}
}
},
"escalate_rule_name": {
"type": "string",
"description": "Escalation rule display name, filled by the server."
Expand Down Expand Up @@ -33632,8 +33660,11 @@
"description": "Optional notification override. Defaults to following each person's personal preference.",
"properties": {
"follow_preference": {
"type": "boolean",
"description": "When true, fall back to each responder's personal preference."
"type": [
"boolean",
"null"
],
"description": "When false, use `personal_channels`; when true or omitted, use each responder's personal preference."
},
"personal_channels": {
"type": "array",
Expand Down
Loading