diff --git a/internal/models/taxonomy.go b/internal/models/taxonomy.go index aa9ab61..e59e7cf 100644 --- a/internal/models/taxonomy.go +++ b/internal/models/taxonomy.go @@ -54,10 +54,14 @@ const ( ) // TaxonomyScope identifies the feedback field that a taxonomy run covers. +// +// SourceID is optional: feedback_records.source_id is nullable, so feedback may have no +// attributed source. An empty SourceID is the canonical "no source" bucket and matches +// feedback records whose source_id is NULL or blank. type TaxonomyScope struct { TenantID string `json:"tenant_id" validate:"required,no_null_bytes,min=1,max=255"` SourceType string `json:"source_type" validate:"required,no_null_bytes,min=1,max=255"` - SourceID string `json:"source_id" validate:"required,no_null_bytes,min=1,max=255"` + SourceID string `json:"source_id" validate:"omitempty,no_null_bytes,max=255"` FieldID string `json:"field_id" validate:"required,no_null_bytes,min=1,max=255"` } @@ -116,12 +120,17 @@ type CreateTaxonomyRunResponse struct { } // ListTaxonomyRunsFilters scopes taxonomy run history queries. +// +// SourceID is a tri-state filter: nil means "no source_id filter", while a non-nil +// pointer (including a pointer to "") filters by that exact value. An empty string +// scopes history to the canonical "no source" bucket — runs whose source_id is "" — +// which a plain string filter cannot express (it cannot tell "unset" from "empty"). type ListTaxonomyRunsFilters struct { - TenantID string `form:"tenant_id" validate:"required,no_null_bytes,min=1,max=255"` - SourceType string `form:"source_type" validate:"omitempty,no_null_bytes,min=1,max=255"` - SourceID string `form:"source_id" validate:"omitempty,no_null_bytes,min=1,max=255"` - FieldID string `form:"field_id" validate:"omitempty,no_null_bytes,min=1,max=255"` - Limit int `form:"limit" validate:"omitempty,min=1,max=100"` + TenantID string `form:"tenant_id" validate:"required,no_null_bytes,min=1,max=255"` + SourceType string `form:"source_type" validate:"omitempty,no_null_bytes,min=1,max=255"` + SourceID *string `form:"source_id" validate:"omitempty,no_null_bytes,max=255"` + FieldID string `form:"field_id" validate:"omitempty,no_null_bytes,min=1,max=255"` + Limit int `form:"limit" validate:"omitempty,min=1,max=100"` } // ListTaxonomyRunsResponse contains taxonomy run history. diff --git a/internal/repository/taxonomy_repository.go b/internal/repository/taxonomy_repository.go index 755e44a..a23f88b 100644 --- a/internal/repository/taxonomy_repository.go +++ b/internal/repository/taxonomy_repository.go @@ -57,7 +57,7 @@ func (r *TaxonomyRepository) ListFieldOptions( SELECT fr.tenant_id, fr.source_type, - fr.source_id, + COALESCE(NULLIF(btrim(fr.source_id), ''), ''), COALESCE(MAX(fr.source_name) FILTER (WHERE fr.source_name IS NOT NULL AND btrim(fr.source_name) <> ''), ''), fr.field_id, COALESCE(MAX(fr.field_label) FILTER (WHERE fr.field_label IS NOT NULL AND btrim(fr.field_label) <> ''), ''), @@ -66,12 +66,10 @@ func (r *TaxonomyRepository) ListFieldOptions( FROM feedback_records fr LEFT JOIN embeddings e ON e.feedback_record_id = fr.id AND e.model = $2 WHERE fr.tenant_id = $1 - AND fr.source_id IS NOT NULL - AND btrim(fr.source_id) <> '' AND fr.value_text IS NOT NULL AND btrim(fr.value_text) <> '' - GROUP BY fr.tenant_id, fr.source_type, fr.source_id, fr.field_id - ORDER BY fr.source_type, fr.source_id, fr.field_id`, + GROUP BY fr.tenant_id, fr.source_type, COALESCE(NULLIF(btrim(fr.source_id), ''), ''), fr.field_id + ORDER BY fr.source_type, COALESCE(NULLIF(btrim(fr.source_id), ''), ''), fr.field_id`, tenantID, embeddingModel, ) if err != nil { @@ -127,7 +125,7 @@ func (r *TaxonomyRepository) CountScopeInput( LEFT JOIN embeddings e ON e.feedback_record_id = fr.id AND e.model = $5 WHERE fr.tenant_id = $1 AND fr.source_type = $2 - AND fr.source_id = $3 + AND NULLIF(btrim(fr.source_id), '') IS NOT DISTINCT FROM NULLIF(btrim($3), '') AND fr.field_id = $4 AND fr.value_text IS NOT NULL AND btrim(fr.value_text) <> ''`, @@ -358,9 +356,21 @@ func (r *TaxonomyRepository) ListRuns( query += fmt.Sprintf(" AND %s = $%d", column, len(args)) } + // addFilterPtr is the tri-state variant: nil skips the filter, while a non-nil + // pointer filters by the exact stored value, including "" for the canonical + // "no source" bucket (which addFilter would drop as an absent filter). + addFilterPtr := func(column string, value *string) { + if value == nil { + return + } + + args = append(args, *value) + query += fmt.Sprintf(" AND %s = $%d", column, len(args)) + } + addFilter("source_type", filters.SourceType) - addFilter("source_id", filters.SourceID) addFilter("field_id", filters.FieldID) + addFilterPtr("source_id", filters.SourceID) args = append(args, limit) query += fmt.Sprintf(" ORDER BY created_at DESC, id DESC LIMIT $%d", len(args)) @@ -392,7 +402,7 @@ func (r *TaxonomyRepository) GetRunInput( INNER JOIN embeddings e ON e.feedback_record_id = fr.id AND e.model = $6 WHERE fr.tenant_id = $1 AND fr.source_type = $2 - AND fr.source_id = $3 + AND NULLIF(btrim(fr.source_id), '') IS NOT DISTINCT FROM NULLIF(btrim($3), '') AND fr.field_id = $4 AND fr.value_text IS NOT NULL AND btrim(fr.value_text) <> '' diff --git a/internal/service/id_validation.go b/internal/service/id_validation.go index 801158c..25eb2fa 100644 --- a/internal/service/id_validation.go +++ b/internal/service/id_validation.go @@ -36,6 +36,22 @@ func normalizeRequiredIdentifier(fieldName, value string) (string, error) { return "", huberrors.NewValidationError(fieldName, fieldName+" is required and cannot be empty") } + return normalizeIdentifierValue(fieldName, normalized) +} + +// normalizeOptionalIdentifier trims an identifier and validates it when present. +// An empty (or blank) value is allowed and normalizes to "", which callers treat as the +// canonical "no value" bucket (e.g. taxonomy's "no source" scope). +func normalizeOptionalIdentifier(fieldName, value string) (string, error) { + normalized := strings.TrimSpace(value) + if normalized == "" { + return "", nil + } + + return normalizeIdentifierValue(fieldName, normalized) +} + +func normalizeIdentifierValue(fieldName, normalized string) (string, error) { if strings.ContainsRune(normalized, '\x00') { return "", huberrors.NewValidationError(fieldName, fieldName+" must not contain NULL bytes") } diff --git a/internal/service/taxonomy_service.go b/internal/service/taxonomy_service.go index 0b9454c..a74674a 100644 --- a/internal/service/taxonomy_service.go +++ b/internal/service/taxonomy_service.go @@ -453,7 +453,8 @@ func normalizeTaxonomyScope(scope models.TaxonomyScope) (models.TaxonomyScope, e return models.TaxonomyScope{}, err } - sourceID, err := normalizeRequiredIdentifier("source_id", scope.SourceID) + // source_id is optional: an empty value is the canonical "no source" scope. + sourceID, err := normalizeOptionalIdentifier("source_id", scope.SourceID) if err != nil { return models.TaxonomyScope{}, err } diff --git a/migrations/010_taxonomy_schema.sql b/migrations/010_taxonomy_schema.sql index 9796ea4..23d509d 100644 --- a/migrations/010_taxonomy_schema.sql +++ b/migrations/010_taxonomy_schema.sql @@ -1,6 +1,16 @@ -- +goose up -- Taxonomy generation artifacts are stored as run-scoped Hub data. Keep them -- separate from feedback_records so repeated generation never mutates source feedback. +-- +-- Source scope and the "no source" bucket: +-- feedback_records.source_id is nullable (see 001_initial_schema.sql), so feedback can +-- exist with no attributed source. Taxonomy must still cover those records. Rather than +-- making source_id nullable inside the composite PK/FK/UNIQUE constraints below (NULL is +-- never equal to NULL, which breaks key matching), taxonomy canonicalizes "no source" to +-- the empty string ''. The Go layer always sends a string, never NULL, so source_id stays +-- NOT NULL here and '' is a valid, comparable key value. Repository queries that read +-- feedback_records use null-safe matching (NULLIF(btrim(source_id), '') IS NOT DISTINCT +-- FROM ...) so the '' taxonomy scope matches NULL/blank feedback rows. ALTER TABLE feedback_records ADD CONSTRAINT feedback_records_id_tenant_unique UNIQUE USING INDEX feedback_records_id_tenant_uidx; @@ -45,7 +55,7 @@ CREATE TABLE taxonomy_runs ( updated_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), CONSTRAINT taxonomy_runs_tenant_id_required CHECK (btrim(tenant_id) <> ''), CONSTRAINT taxonomy_runs_source_type_required CHECK (btrim(source_type) <> ''), - CONSTRAINT taxonomy_runs_source_id_required CHECK (btrim(source_id) <> ''), + -- source_id intentionally allows '' (the canonical "no source" bucket); see header note. CONSTRAINT taxonomy_runs_field_id_required CHECK (btrim(field_id) <> ''), CONSTRAINT taxonomy_runs_record_count_nonnegative CHECK (record_count >= 0), CONSTRAINT taxonomy_runs_embedding_count_nonnegative CHECK (embedding_count >= 0), @@ -157,7 +167,7 @@ CREATE TABLE taxonomy_active_runs ( ON DELETE CASCADE, CONSTRAINT taxonomy_active_runs_tenant_id_required CHECK (btrim(tenant_id) <> ''), CONSTRAINT taxonomy_active_runs_source_type_required CHECK (btrim(source_type) <> ''), - CONSTRAINT taxonomy_active_runs_source_id_required CHECK (btrim(source_id) <> ''), + -- source_id intentionally allows '' (the canonical "no source" bucket); see header note. CONSTRAINT taxonomy_active_runs_field_id_required CHECK (btrim(field_id) <> ''), UNIQUE (run_id) ); @@ -185,7 +195,7 @@ CREATE TABLE taxonomy_node_events ( FOREIGN KEY (node_id, run_id) REFERENCES taxonomy_nodes(id, run_id) ON DELETE CASCADE, CONSTRAINT taxonomy_node_events_tenant_id_required CHECK (btrim(tenant_id) <> ''), CONSTRAINT taxonomy_node_events_source_type_required CHECK (btrim(source_type) <> ''), - CONSTRAINT taxonomy_node_events_source_id_required CHECK (btrim(source_id) <> ''), + -- source_id intentionally allows '' (the canonical "no source" bucket); see header note. CONSTRAINT taxonomy_node_events_field_id_required CHECK (btrim(field_id) <> ''), CONSTRAINT taxonomy_node_events_actor_id_required CHECK (btrim(actor_id) <> '') ); diff --git a/tests/taxonomy_no_source_test.go b/tests/taxonomy_no_source_test.go new file mode 100644 index 0000000..b9f5efd --- /dev/null +++ b/tests/taxonomy_no_source_test.go @@ -0,0 +1,168 @@ +package tests + +import ( + "context" + "os" + "testing" + + "github.com/google/uuid" + "github.com/jackc/pgx/v5/pgxpool" + "github.com/stretchr/testify/require" + + "github.com/formbricks/hub/internal/config" + "github.com/formbricks/hub/internal/models" + "github.com/formbricks/hub/internal/repository" + "github.com/formbricks/hub/pkg/database" +) + +func setupTaxonomyNoSourceTestDB(t *testing.T) *pgxpool.Pool { + t.Helper() + + databaseURL := os.Getenv("DATABASE_URL") + if databaseURL == "" { + databaseURL = defaultTestDatabaseURL + } + + t.Setenv("DATABASE_URL", databaseURL) + + cfg, err := config.Load() + require.NoError(t, err) + + db, err := database.NewPostgresPool(context.Background(), cfg.Database.URL, + database.WithPoolConfig(cfg.Database.PoolConfig()), + ) + require.NoError(t, err) + + t.Cleanup(db.Close) + + return db +} + +// TestTaxonomyNoSourceScope verifies that feedback records with a NULL/blank source_id +// participate in taxonomy through the canonical "no source" bucket (empty-string scope). +func TestTaxonomyNoSourceScope(t *testing.T) { + ctx := context.Background() + db := setupTaxonomyNoSourceTestDB(t) + repo := repository.NewTaxonomyRepository(db) + + tenantID := "taxonomy-nosource-" + uuid.NewString() + sourceType := "formbricks" + fieldID := "feedback" + + const embeddingModel = "text-embedding-test" + + t.Cleanup(func() { + _, _ = db.Exec(ctx, `DELETE FROM taxonomy_runs WHERE tenant_id = $1`, tenantID) + _, _ = db.Exec(ctx, `DELETE FROM feedback_records WHERE tenant_id = $1`, tenantID) + }) + + // Two feedback records with no attributed source: one NULL, one blank. + for _, srcID := range []any{nil, " "} { + _, err := db.Exec(ctx, ` + INSERT INTO feedback_records ( + source_type, source_id, field_id, field_label, field_type, + value_text, tenant_id, submission_id + ) + VALUES ($1, $2, $3, $4, $5::field_type_enum, $6, $7, $8)`, + sourceType, srcID, fieldID, "Feedback", "text", + "Login was confusing", tenantID, "submission-"+uuid.NewString(), + ) + require.NoError(t, err) + } + + // Discovery surfaces the records as a single "no source" bucket with empty SourceID. + options, err := repo.ListFieldOptions(ctx, tenantID, embeddingModel) + require.NoError(t, err) + + var noSource *models.TaxonomyFieldOption + + for i := range options { + if options[i].SourceType == sourceType && options[i].FieldID == fieldID { + noSource = &options[i] + } + } + + require.NotNil(t, noSource, "expected a discovered field option for the no-source bucket") + require.Empty(t, noSource.SourceID, "no-source bucket must expose an empty source_id") + require.Equal(t, 2, noSource.RecordCount, "NULL and blank source_id must collapse into one bucket") + + // Counting the empty-source scope matches both NULL and blank feedback rows. + scope := models.TaxonomyScope{ + TenantID: tenantID, + SourceType: sourceType, + SourceID: "", + FieldID: fieldID, + } + + recordCount, _, _, err := repo.CountScopeInput(ctx, scope, embeddingModel) + require.NoError(t, err) + require.Equal(t, 2, recordCount, "empty-source scope must null-safe match NULL/blank source rows") + + // A taxonomy run can be created for the empty-source scope and is found by the + // in-progress guard (empty string is a valid, comparable key in taxonomy tables). + run, created, err := repo.CreateRunIfAvailable(ctx, repository.CreateTaxonomyRunParams{ + TaxonomyScope: scope, + RecordCount: recordCount, + EmbeddingCount: 0, + }) + require.NoError(t, err) + require.True(t, created) + require.Empty(t, run.SourceID) + + _, createdAgain, err := repo.CreateRunIfAvailable(ctx, repository.CreateTaxonomyRunParams{ + TaxonomyScope: scope, + }) + require.NoError(t, err) + require.False(t, createdAgain, "a second run for the same empty-source scope must reuse the in-progress run") +} + +// TestTaxonomyListRunsSourceIDTriState verifies the tri-state source_id filter on run +// history: nil returns every source, a pointer to "" scopes to the no-source bucket, and +// a pointer to a concrete value scopes to that source. +func TestTaxonomyListRunsSourceIDTriState(t *testing.T) { + ctx := context.Background() + db := setupTaxonomyNoSourceTestDB(t) + repo := repository.NewTaxonomyRepository(db) + + tenantID := "taxonomy-listfilter-" + uuid.NewString() + sourceType := "formbricks" + fieldID := "feedback" + sourcedID := "survey-" + uuid.NewString() + + t.Cleanup(func() { + _, _ = db.Exec(ctx, `DELETE FROM taxonomy_runs WHERE tenant_id = $1`, tenantID) + }) + + // One run with no source ("") and one with a concrete source. + for _, sourceID := range []string{"", sourcedID} { + _, created, err := repo.CreateRunIfAvailable(ctx, repository.CreateTaxonomyRunParams{ + TaxonomyScope: models.TaxonomyScope{ + TenantID: tenantID, + SourceType: sourceType, + SourceID: sourceID, + FieldID: fieldID, + }, + }) + require.NoError(t, err) + require.True(t, created) + } + + ptr := func(s string) *string { return &s } + + // nil filter: both runs. + all, err := repo.ListRuns(ctx, models.ListTaxonomyRunsFilters{TenantID: tenantID}) + require.NoError(t, err) + require.Len(t, all, 2) + + // pointer to "": only the no-source run. + noSource, err := repo.ListRuns(ctx, models.ListTaxonomyRunsFilters{TenantID: tenantID, SourceID: ptr("")}) + require.NoError(t, err) + require.Len(t, noSource, 1) + require.Empty(t, noSource[0].SourceID) + + // pointer to a concrete value: only that sourced run. + sourced, err := repo.ListRuns(ctx, models.ListTaxonomyRunsFilters{TenantID: tenantID, SourceID: ptr(sourcedID)}) + require.NoError(t, err) + require.Len(t, sourced, 1) + require.Equal(t, sourcedID, sourced[0].SourceID) +}