mirror of
https://github.com/lukaszraczylo/claude-mnemonic.git
synced 2026-10-12 04:55:18 +00:00
fix(db): stop writing a column that does not exist when editing an observation (#69)
UpdateObservation set updated_at_epoch, but the observations table has no such column, so every PUT /api/observations/{id} failed with HTTP 500 (no such column: updated_at_epoch). Remove the assignment; nothing reads an edited-at time. An update that changes nothing now returns the observation, as intended. UpdateObservation had no tests: add store and handler tests.
Co-authored-by: Claude Sonnet 5.5 <[email protected]>
This commit is contained in:
co-authored by
Claude Sonnet 5.5
parent
67de0f0c84
commit
055103cb3b
@@ -252,9 +252,6 @@ func (s *ObservationStore) UpdateObservation(ctx context.Context, id int64, upda
|
|||||||
updates["scope"] = sql.NullString{String: *update.Scope, Valid: true}
|
updates["scope"] = sql.NullString{String: *update.Scope, Valid: true}
|
||||||
}
|
}
|
||||||
|
|
||||||
// Add updated_at timestamp
|
|
||||||
updates["updated_at_epoch"] = sql.NullInt64{Int64: time.Now().Unix(), Valid: true}
|
|
||||||
|
|
||||||
if len(updates) == 0 {
|
if len(updates) == 0 {
|
||||||
// Nothing to update, just return existing observation
|
// Nothing to update, just return existing observation
|
||||||
return toModelObservation(&dbObs), nil
|
return toModelObservation(&dbObs), nil
|
||||||
|
|||||||
@@ -0,0 +1,94 @@
|
|||||||
|
package gorm
|
||||||
|
|
||||||
|
import (
|
||||||
|
"context"
|
||||||
|
"testing"
|
||||||
|
|
||||||
|
"github.com/stretchr/testify/assert"
|
||||||
|
"github.com/stretchr/testify/require"
|
||||||
|
|
||||||
|
"github.com/lukaszraczylo/claude-mnemonic/pkg/models"
|
||||||
|
)
|
||||||
|
|
||||||
|
func storeForUpdate(t *testing.T) (*ObservationStore, int64, func()) {
|
||||||
|
t.Helper()
|
||||||
|
obsStore, store, cleanup := testObservationStore(t)
|
||||||
|
ctx := context.Background()
|
||||||
|
sessionID, err := NewSessionStore(store).CreateSDKSession(ctx, "claude-1", "test-project", "")
|
||||||
|
require.NoError(t, err)
|
||||||
|
id, _, err := obsStore.StoreObservation(ctx, "claude-1", "test-project", &models.ParsedObservation{
|
||||||
|
Type: models.ObsTypeDiscovery, Title: "Original title", Subtitle: "Original subtitle", Narrative: "Original narrative",
|
||||||
|
Facts: []string{"fact one"}, Concepts: []string{"cache"}, FilesRead: []string{"a.go"},
|
||||||
|
}, int(sessionID), 1)
|
||||||
|
require.NoError(t, err)
|
||||||
|
return obsStore, id, cleanup
|
||||||
|
}
|
||||||
|
|
||||||
|
func ptr[T any](v T) *T { return &v }
|
||||||
|
|
||||||
|
func TestUpdateObservation_ChangesOnlyTheGivenFields(t *testing.T) {
|
||||||
|
obsStore, id, cleanup := storeForUpdate(t)
|
||||||
|
defer cleanup()
|
||||||
|
ctx := context.Background()
|
||||||
|
|
||||||
|
got, err := obsStore.UpdateObservation(ctx, id, &ObservationUpdate{Title: ptr("New title"), Narrative: ptr("New narrative")})
|
||||||
|
require.NoError(t, err)
|
||||||
|
assert.Equal(t, "New title", got.Title.String)
|
||||||
|
assert.Equal(t, "New narrative", got.Narrative.String)
|
||||||
|
assert.Equal(t, "Original subtitle", got.Subtitle.String, "a field that was not given stays")
|
||||||
|
assert.Equal(t, []string{"fact one"}, []string(got.Facts))
|
||||||
|
|
||||||
|
stored, err := obsStore.GetObservationByID(ctx, id)
|
||||||
|
require.NoError(t, err)
|
||||||
|
assert.Equal(t, "New title", stored.Title.String, "and it is stored, not only returned")
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestUpdateObservation_ReplacesListsAndScope(t *testing.T) {
|
||||||
|
obsStore, id, cleanup := storeForUpdate(t)
|
||||||
|
defer cleanup()
|
||||||
|
ctx := context.Background()
|
||||||
|
|
||||||
|
got, err := obsStore.UpdateObservation(ctx, id, &ObservationUpdate{
|
||||||
|
Facts: &[]string{"a", "b"}, Concepts: &[]string{}, FilesRead: &[]string{"x.go"}, FilesModified: &[]string{"y.go"}, Scope: ptr("global"),
|
||||||
|
})
|
||||||
|
require.NoError(t, err)
|
||||||
|
assert.Equal(t, []string{"a", "b"}, []string(got.Facts))
|
||||||
|
assert.Empty(t, got.Concepts, "an empty list clears the field")
|
||||||
|
assert.Equal(t, []string{"x.go"}, []string(got.FilesRead))
|
||||||
|
assert.Equal(t, []string{"y.go"}, []string(got.FilesModified))
|
||||||
|
assert.Equal(t, models.ScopeGlobal, got.Scope)
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestUpdateObservation_NothingToChangeReturnsTheObservation(t *testing.T) {
|
||||||
|
obsStore, id, cleanup := storeForUpdate(t)
|
||||||
|
defer cleanup()
|
||||||
|
|
||||||
|
got, err := obsStore.UpdateObservation(context.Background(), id, &ObservationUpdate{})
|
||||||
|
require.NoError(t, err)
|
||||||
|
assert.Equal(t, "Original title", got.Title.String)
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestUpdateObservation_Refusals(t *testing.T) {
|
||||||
|
obsStore, id, cleanup := storeForUpdate(t)
|
||||||
|
defer cleanup()
|
||||||
|
ctx := context.Background()
|
||||||
|
|
||||||
|
_, err := obsStore.UpdateObservation(ctx, id, nil)
|
||||||
|
assert.Error(t, err, "a nil update")
|
||||||
|
_, err = obsStore.UpdateObservation(ctx, 999999, &ObservationUpdate{Title: ptr("x")})
|
||||||
|
require.Error(t, err)
|
||||||
|
assert.Contains(t, err.Error(), "not found")
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestUpdateObservation_KeepsTheSearchIndexInStep(t *testing.T) {
|
||||||
|
obsStore, id, cleanup := storeForUpdate(t)
|
||||||
|
defer cleanup()
|
||||||
|
ctx := context.Background()
|
||||||
|
|
||||||
|
_, err := obsStore.UpdateObservation(ctx, id, &ObservationUpdate{Title: ptr("Zebrafish enclosure")})
|
||||||
|
require.NoError(t, err)
|
||||||
|
var n int64
|
||||||
|
require.NoError(t, obsStore.db.WithContext(ctx).Raw(
|
||||||
|
`SELECT COUNT(*) FROM observations_fts WHERE observations_fts MATCH 'zebrafish'`).Scan(&n).Error)
|
||||||
|
assert.EqualValues(t, 1, n, "the full-text index sees the new title")
|
||||||
|
}
|
||||||
@@ -0,0 +1,59 @@
|
|||||||
|
package worker
|
||||||
|
|
||||||
|
import (
|
||||||
|
"bytes"
|
||||||
|
"encoding/json"
|
||||||
|
"fmt"
|
||||||
|
"net/http"
|
||||||
|
"net/http/httptest"
|
||||||
|
"testing"
|
||||||
|
|
||||||
|
"github.com/stretchr/testify/assert"
|
||||||
|
"github.com/stretchr/testify/require"
|
||||||
|
)
|
||||||
|
|
||||||
|
func doUpdateRequest(t *testing.T, svc *Service, method, target string, body any) *httptest.ResponseRecorder {
|
||||||
|
t.Helper()
|
||||||
|
var buf bytes.Buffer
|
||||||
|
if body != nil {
|
||||||
|
require.NoError(t, json.NewEncoder(&buf).Encode(body))
|
||||||
|
}
|
||||||
|
rec := httptest.NewRecorder()
|
||||||
|
svc.router.ServeHTTP(rec, httptest.NewRequest(method, target, &buf))
|
||||||
|
return rec
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestHandleUpdateObservation_EditsAndReturnsTheObservation(t *testing.T) {
|
||||||
|
svc, cleanup := testService(t)
|
||||||
|
defer cleanup()
|
||||||
|
id := createTestObservation(t, svc.observationStore, "proj_aaaaaa", "Old title", "Old narrative", []string{"cache"})
|
||||||
|
|
||||||
|
rec := doUpdateRequest(t, svc, http.MethodPut, fmt.Sprintf("/api/observations/%d", id),
|
||||||
|
map[string]any{"title": "New title", "facts": []string{"one", "two"}})
|
||||||
|
require.Equal(t, http.StatusOK, rec.Code, rec.Body.String())
|
||||||
|
var resp struct {
|
||||||
|
Observation struct {
|
||||||
|
Title string `json:"title"`
|
||||||
|
Narrative string `json:"narrative"`
|
||||||
|
Facts []string `json:"facts"`
|
||||||
|
} `json:"observation"`
|
||||||
|
}
|
||||||
|
require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &resp))
|
||||||
|
assert.Equal(t, "New title", resp.Observation.Title)
|
||||||
|
assert.Equal(t, "Old narrative", resp.Observation.Narrative)
|
||||||
|
assert.Equal(t, []string{"one", "two"}, resp.Observation.Facts)
|
||||||
|
|
||||||
|
rec = doUpdateRequest(t, svc, http.MethodGet, fmt.Sprintf("/api/observations/%d", id), nil)
|
||||||
|
require.Equal(t, http.StatusOK, rec.Code)
|
||||||
|
assert.Contains(t, rec.Body.String(), "New title", "the edit is stored")
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestHandleUpdateObservation_Refusals(t *testing.T) {
|
||||||
|
svc, cleanup := testService(t)
|
||||||
|
defer cleanup()
|
||||||
|
id := createTestObservation(t, svc.observationStore, "proj_aaaaaa", "A title", "A narrative", nil)
|
||||||
|
|
||||||
|
assert.Equal(t, http.StatusNotFound, doUpdateRequest(t, svc, http.MethodPut, "/api/observations/999999", map[string]any{"title": "x"}).Code)
|
||||||
|
assert.Equal(t, http.StatusBadRequest, doUpdateRequest(t, svc, http.MethodPut, fmt.Sprintf("/api/observations/%d", id), map[string]any{"scope": "galaxy"}).Code)
|
||||||
|
assert.Equal(t, http.StatusBadRequest, doUpdateRequest(t, svc, http.MethodPut, "/api/observations/abc", map[string]any{"title": "x"}).Code)
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user