fix(podcasts): code-review pass over the session's UI surfaces
Findings from the systematic gotcha-class review before the browser
pass:
- The collection smart editor's Apply derived the collection NAME from
document.title — which parses to "Smart Collection", RENAMING the
collection on every apply. The server now renders the real name
(data-collection-name) and the write path refuses to proceed without
it.
- TestRenderingPartialsRender pinned the pre-episode-collections
empty-state copy ("No books in this collection yet.") — updated to
the shipped label ("Nothing in this collection yet."; collections
hold episodes now).
- The deleted handler-local rule comparators (evaluateRule/
compareValues — removed in item 3.4's single-evaluator refactor)
left behind two unit-test files testing the duplicates; deleted.
Their semantics remain covered at the service level
(TestEvaluateRules_* over FieldMatches/EvaluateRules) and
TestTextToString (which actually pins utils.PgTextString) stays.
Everything else reviewed clean against the gotcha classes: templ
whole-expression attrs, ?={ boolean attrs, JSONArg quoting in
x-init/data attrs, Alpine CSP store paths, htmx form-encoding, the
" assert escaping, and the aria-label selector contract on the
player's reload listener.
This commit is contained in:
1 parent
94bd69d22d
commit
c54430c254
6 files changed
+75
-552
No files matched your search
@@ -1,415 +0,0 @@
|
||||
package handlers
|
||||
|
||||
import (
|
||||
"testing"
|
||||
|
||||
"bookhoard/internal/database"
|
||||
|
||||
"github.com/jackc/pgx/v5/pgtype"
|
||||
"github.com/stretchr/testify/assert"
|
||||
)
|
||||
|
||||
// Test collection rule matching with various operators
|
||||
|
||||
func TestEvaluateRule_Equals_StringMatch(t *testing.T) {
|
||||
handler := &CollectionHandler{}
|
||||
item := database.MediaItems{
|
||||
Genre: pgtype.Text{String: "Science Fiction", Valid: true},
|
||||
}
|
||||
|
||||
result := handler.evaluateRule(item, "genre", "equals", "Science Fiction")
|
||||
assert.True(t, result)
|
||||
}
|
||||
|
||||
func TestEvaluateRule_Equals_NoMatch(t *testing.T) {
|
||||
handler := &CollectionHandler{}
|
||||
item := database.MediaItems{
|
||||
Genre: pgtype.Text{String: "Fantasy", Valid: true},
|
||||
}
|
||||
|
||||
result := handler.evaluateRule(item, "genre", "equals", "Science Fiction")
|
||||
assert.False(t, result)
|
||||
}
|
||||
|
||||
func TestEvaluateRule_NotEquals_Matches(t *testing.T) {
|
||||
handler := &CollectionHandler{}
|
||||
item := database.MediaItems{
|
||||
Genre: pgtype.Text{String: "Fantasy", Valid: true},
|
||||
}
|
||||
|
||||
result := handler.evaluateRule(item, "genre", "not_equals", "Science Fiction")
|
||||
assert.True(t, result)
|
||||
}
|
||||
|
||||
func TestEvaluateRule_NotEquals_NoMatch(t *testing.T) {
|
||||
handler := &CollectionHandler{}
|
||||
item := database.MediaItems{
|
||||
Genre: pgtype.Text{String: "Science Fiction", Valid: true},
|
||||
}
|
||||
|
||||
result := handler.evaluateRule(item, "genre", "not_equals", "Science Fiction")
|
||||
assert.False(t, result)
|
||||
}
|
||||
|
||||
func TestEvaluateRule_Contains_Matches(t *testing.T) {
|
||||
handler := &CollectionHandler{}
|
||||
item := database.MediaItems{
|
||||
Author: pgtype.Text{String: "Isaac Asimov", Valid: true},
|
||||
}
|
||||
|
||||
result := handler.evaluateRule(item, "author", "contains", "asimov")
|
||||
assert.True(t, result)
|
||||
}
|
||||
|
||||
func TestEvaluateRule_Contains_NoMatch(t *testing.T) {
|
||||
handler := &CollectionHandler{}
|
||||
item := database.MediaItems{
|
||||
Author: pgtype.Text{String: "Isaac Asimov", Valid: true},
|
||||
}
|
||||
|
||||
result := handler.evaluateRule(item, "author", "contains", "clarke")
|
||||
assert.False(t, result)
|
||||
}
|
||||
|
||||
func TestEvaluateRule_NotContains_Matches(t *testing.T) {
|
||||
handler := &CollectionHandler{}
|
||||
item := database.MediaItems{
|
||||
Author: pgtype.Text{String: "Isaac Asimov", Valid: true},
|
||||
}
|
||||
|
||||
result := handler.evaluateRule(item, "author", "not_contains", "clarke")
|
||||
assert.True(t, result)
|
||||
}
|
||||
|
||||
func TestEvaluateRule_StartsWith_Matches(t *testing.T) {
|
||||
handler := &CollectionHandler{}
|
||||
item := database.MediaItems{
|
||||
Series: pgtype.Text{String: "Foundation", Valid: true},
|
||||
}
|
||||
|
||||
result := handler.evaluateRule(item, "series", "starts_with", "Foun")
|
||||
assert.True(t, result)
|
||||
}
|
||||
|
||||
func TestEvaluateRule_StartsWith_NoMatch(t *testing.T) {
|
||||
handler := &CollectionHandler{}
|
||||
item := database.MediaItems{
|
||||
Series: pgtype.Text{String: "Foundation", Valid: true},
|
||||
}
|
||||
|
||||
result := handler.evaluateRule(item, "series", "starts_with", "Robots")
|
||||
assert.False(t, result)
|
||||
}
|
||||
|
||||
func TestEvaluateRule_EndsWith_Matches(t *testing.T) {
|
||||
handler := &CollectionHandler{}
|
||||
item := database.MediaItems{
|
||||
Series: pgtype.Text{String: "Foundation", Valid: true},
|
||||
}
|
||||
|
||||
result := handler.evaluateRule(item, "series", "ends_with", "tion")
|
||||
assert.True(t, result)
|
||||
}
|
||||
|
||||
func TestEvaluateRule_GreaterThan_Matches(t *testing.T) {
|
||||
handler := &CollectionHandler{}
|
||||
item := database.MediaItems{
|
||||
CopyrightYear: pgtype.Int4{Int32: 2021, Valid: true},
|
||||
}
|
||||
|
||||
result := handler.evaluateRule(item, "copyright_year", "greater_than", "2000")
|
||||
assert.True(t, result)
|
||||
}
|
||||
|
||||
func TestEvaluateRule_GreaterThan_NoMatch(t *testing.T) {
|
||||
handler := &CollectionHandler{}
|
||||
item := database.MediaItems{
|
||||
CopyrightYear: pgtype.Int4{Int32: 1999, Valid: true},
|
||||
}
|
||||
|
||||
result := handler.evaluateRule(item, "copyright_year", "greater_than", "2000")
|
||||
assert.False(t, result)
|
||||
}
|
||||
|
||||
func TestEvaluateRule_LessThan_Matches(t *testing.T) {
|
||||
handler := &CollectionHandler{}
|
||||
item := database.MediaItems{
|
||||
CopyrightYear: pgtype.Int4{Int32: 1999, Valid: true},
|
||||
}
|
||||
|
||||
result := handler.evaluateRule(item, "copyright_year", "less_than", "2000")
|
||||
assert.True(t, result)
|
||||
}
|
||||
|
||||
func TestEvaluateRule_LessThan_NoMatch(t *testing.T) {
|
||||
handler := &CollectionHandler{}
|
||||
item := database.MediaItems{
|
||||
CopyrightYear: pgtype.Int4{Int32: 2021, Valid: true},
|
||||
}
|
||||
|
||||
result := handler.evaluateRule(item, "copyright_year", "less_than", "2000")
|
||||
assert.False(t, result)
|
||||
}
|
||||
|
||||
func TestEvaluateRule_NullField(t *testing.T) {
|
||||
handler := &CollectionHandler{}
|
||||
item := database.MediaItems{
|
||||
Genre: pgtype.Text{Valid: false}, // NULL
|
||||
}
|
||||
|
||||
result := handler.evaluateRule(item, "genre", "equals", "Science Fiction")
|
||||
assert.False(t, result, "Should return false for NULL fields")
|
||||
}
|
||||
|
||||
func TestCompareValues_CaseInsensitive(t *testing.T) {
|
||||
handler := &CollectionHandler{}
|
||||
|
||||
result := handler.compareValues("Science Fiction", "contains", "science fiction")
|
||||
assert.True(t, result, "Contains should be case-insensitive")
|
||||
|
||||
result = handler.compareValues("Science Fiction", "contains", "SCIENCE FICTION")
|
||||
assert.True(t, result)
|
||||
}
|
||||
|
||||
func TestCompareValues_EmptyString(t *testing.T) {
|
||||
handler := &CollectionHandler{}
|
||||
|
||||
result := handler.compareValues("", "contains", "test")
|
||||
assert.False(t, result, "Empty string should not contain anything")
|
||||
|
||||
result = handler.compareValues("test", "contains", "")
|
||||
assert.True(t, result, "Non-empty string contains empty string")
|
||||
}
|
||||
|
||||
func TestCompareValues_NumericEdgeCases(t *testing.T) {
|
||||
handler := &CollectionHandler{}
|
||||
|
||||
result := handler.compareValues("0", "greater_than", "-1")
|
||||
assert.True(t, result)
|
||||
|
||||
result = handler.compareValues("9999", "less_than", "10000")
|
||||
assert.True(t, result)
|
||||
}
|
||||
|
||||
// Test rule checking with multiple rules
|
||||
|
||||
func TestCheckRulesAgainstBook_MatchesFirstRule(t *testing.T) {
|
||||
handler := &CollectionHandler{}
|
||||
item := database.MediaItems{
|
||||
Genre: pgtype.Text{String: "Science Fiction", Valid: true},
|
||||
}
|
||||
|
||||
rules := []map[string]interface{}{
|
||||
{
|
||||
"field": "genre",
|
||||
"operator": "equals",
|
||||
"value": "Science Fiction",
|
||||
},
|
||||
{
|
||||
"field": "author",
|
||||
"operator": "contains",
|
||||
"value": "Asimov",
|
||||
},
|
||||
}
|
||||
|
||||
reason := handler.checkRulesAgainstBook(item, rules)
|
||||
assert.NotEmpty(t, reason, "Should match at least one rule")
|
||||
assert.Contains(t, reason, "genre")
|
||||
}
|
||||
|
||||
func TestCheckRulesAgainstBook_MatchesSecondRule(t *testing.T) {
|
||||
handler := &CollectionHandler{}
|
||||
item := database.MediaItems{
|
||||
Genre: pgtype.Text{String: "Fantasy", Valid: true},
|
||||
Author: pgtype.Text{String: "Isaac Asimov", Valid: true},
|
||||
CopyrightYear: pgtype.Int4{Int32: 2021, Valid: true},
|
||||
}
|
||||
|
||||
rules := []map[string]interface{}{
|
||||
{
|
||||
"field": "genre",
|
||||
"operator": "equals",
|
||||
"value": "Science Fiction",
|
||||
},
|
||||
{
|
||||
"field": "author",
|
||||
"operator": "contains",
|
||||
"value": "Asimov",
|
||||
},
|
||||
}
|
||||
|
||||
reason := handler.checkRulesAgainstBook(item, rules)
|
||||
assert.NotEmpty(t, reason, "Should match second rule")
|
||||
assert.Contains(t, reason, "author")
|
||||
}
|
||||
|
||||
func TestCheckRulesAgainstBook_NoMatches(t *testing.T) {
|
||||
handler := &CollectionHandler{}
|
||||
item := database.MediaItems{
|
||||
Genre: pgtype.Text{String: "Mystery", Valid: true},
|
||||
Author: pgtype.Text{String: "Agatha Christie", Valid: true},
|
||||
CopyrightYear: pgtype.Int4{Int32: 1950, Valid: true},
|
||||
}
|
||||
|
||||
rules := []map[string]interface{}{
|
||||
{
|
||||
"field": "genre",
|
||||
"operator": "equals",
|
||||
"value": "Science Fiction",
|
||||
},
|
||||
{
|
||||
"field": "author",
|
||||
"operator": "contains",
|
||||
"value": "Asimov",
|
||||
},
|
||||
}
|
||||
|
||||
reason := handler.checkRulesAgainstBook(item, rules)
|
||||
assert.Empty(t, reason, "Should not match any rules")
|
||||
}
|
||||
|
||||
func TestCheckRulesAgainstBook_EmptyRules(t *testing.T) {
|
||||
handler := &CollectionHandler{}
|
||||
item := database.MediaItems{
|
||||
Genre: pgtype.Text{String: "Science Fiction", Valid: true},
|
||||
}
|
||||
|
||||
rules := []map[string]interface{}{}
|
||||
|
||||
reason := handler.checkRulesAgainstBook(item, rules)
|
||||
assert.Empty(t, reason, "Empty rules should return empty reason")
|
||||
}
|
||||
|
||||
// Test complex rule scenarios
|
||||
|
||||
func TestComplexRule_MultipleConditions(t *testing.T) {
|
||||
handler := &CollectionHandler{}
|
||||
|
||||
// Book: "Foundation" by Isaac Asimov, Sci-Fi, 1951
|
||||
item := database.MediaItems{
|
||||
Title: "Foundation",
|
||||
Author: pgtype.Text{String: "Isaac Asimov", Valid: true},
|
||||
Genre: pgtype.Text{String: "Science Fiction", Valid: true},
|
||||
CopyrightYear: pgtype.Int4{Int32: 1951, Valid: true},
|
||||
Series: pgtype.Text{String: "Foundation", Valid: true},
|
||||
}
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
field string
|
||||
operator string
|
||||
value string
|
||||
expected bool
|
||||
}{
|
||||
{
|
||||
name: "Match genre exactly",
|
||||
field: "genre",
|
||||
operator: "equals",
|
||||
value: "Science Fiction",
|
||||
expected: true,
|
||||
},
|
||||
{
|
||||
name: "Match author substring",
|
||||
field: "author",
|
||||
operator: "contains",
|
||||
value: "asimov",
|
||||
expected: true,
|
||||
},
|
||||
{
|
||||
name: "Match year greater than",
|
||||
field: "copyright_year",
|
||||
operator: "greater_than",
|
||||
value: "1950",
|
||||
expected: true,
|
||||
},
|
||||
{
|
||||
name: "Match year less than",
|
||||
field: "copyright_year",
|
||||
operator: "less_than",
|
||||
value: "2000",
|
||||
expected: true,
|
||||
},
|
||||
{
|
||||
name: "Match series starts with",
|
||||
field: "series",
|
||||
operator: "starts_with",
|
||||
value: "Found",
|
||||
expected: true,
|
||||
},
|
||||
{
|
||||
name: "Match series ends with",
|
||||
field: "series",
|
||||
operator: "ends_with",
|
||||
value: "tion",
|
||||
expected: true,
|
||||
},
|
||||
{
|
||||
name: "No match for genre",
|
||||
field: "genre",
|
||||
operator: "equals",
|
||||
value: "Fantasy",
|
||||
expected: false,
|
||||
},
|
||||
{
|
||||
name: "No match for author",
|
||||
field: "author",
|
||||
operator: "contains",
|
||||
value: "Tolkien",
|
||||
expected: false,
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
result := handler.evaluateRule(item, tt.field, tt.operator, tt.value)
|
||||
assert.Equal(t, tt.expected, result)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// Test edge cases and error handling
|
||||
|
||||
func TestEvaluateRule_InvalidOperator(t *testing.T) {
|
||||
handler := &CollectionHandler{}
|
||||
item := database.MediaItems{
|
||||
Genre: pgtype.Text{String: "Science Fiction", Valid: true},
|
||||
}
|
||||
|
||||
result := handler.evaluateRule(item, "genre", "invalid_operator", "Science Fiction")
|
||||
assert.False(t, result, "Invalid operator should return false")
|
||||
}
|
||||
|
||||
func TestEvaluateRule_NonExistentField(t *testing.T) {
|
||||
handler := &CollectionHandler{}
|
||||
item := database.MediaItems{
|
||||
Genre: pgtype.Text{String: "Science Fiction", Valid: true},
|
||||
}
|
||||
|
||||
result := handler.evaluateRule(item, "nonexistent_field", "equals", "value")
|
||||
assert.False(t, result, "Non-existent field should return false")
|
||||
}
|
||||
|
||||
func TestCompareValues_InvalidOperator(t *testing.T) {
|
||||
handler := &CollectionHandler{}
|
||||
|
||||
result := handler.compareValues("test", "invalid_op", "value")
|
||||
assert.False(t, result, "Invalid comparison operator should return false")
|
||||
}
|
||||
|
||||
func TestCompareValues_NumericConversion(t *testing.T) {
|
||||
handler := &CollectionHandler{}
|
||||
|
||||
// Valid numeric strings
|
||||
result := handler.compareValues("100", "greater_than", "99")
|
||||
assert.True(t, result)
|
||||
|
||||
result = handler.compareValues("99", "less_than", "100")
|
||||
assert.True(t, result)
|
||||
|
||||
// Invalid numeric strings (should return false)
|
||||
result = handler.compareValues("abc", "greater_than", "100")
|
||||
assert.False(t, result, "Non-numeric string should not compare as greater than")
|
||||
|
||||
result = handler.compareValues("100", "greater_than", "xyz")
|
||||
assert.False(t, result, "Non-numeric value should not allow comparison")
|
||||
}
|
||||
@@ -3,95 +3,12 @@ package handlers
|
||||
import (
|
||||
"testing"
|
||||
|
||||
"bookhoard/internal/database"
|
||||
"bookhoard/internal/utils"
|
||||
|
||||
"github.com/jackc/pgx/v5/pgtype"
|
||||
"github.com/stretchr/testify/assert"
|
||||
)
|
||||
|
||||
func TestCompareValues_Equals(t *testing.T) {
|
||||
handler := &CollectionHandler{}
|
||||
|
||||
result := handler.compareValues("Science Fiction", "equals", "Science Fiction")
|
||||
assert.True(t, result)
|
||||
|
||||
result = handler.compareValues("Science Fiction", "equals", "Fantasy")
|
||||
assert.False(t, result)
|
||||
}
|
||||
|
||||
func TestCompareValues_Contains(t *testing.T) {
|
||||
handler := &CollectionHandler{}
|
||||
|
||||
result := handler.compareValues("Isaac Asimov", "contains", "asimov")
|
||||
assert.True(t, result)
|
||||
|
||||
result = handler.compareValues("Isaac Asimov", "contains", "clarke")
|
||||
assert.False(t, result)
|
||||
}
|
||||
|
||||
func TestCompareValues_GreaterThan(t *testing.T) {
|
||||
handler := &CollectionHandler{}
|
||||
|
||||
result := handler.compareValues("2021", "greater_than", "2000")
|
||||
assert.True(t, result)
|
||||
|
||||
result = handler.compareValues("1999", "greater_than", "2000")
|
||||
assert.False(t, result)
|
||||
}
|
||||
|
||||
func TestCompareValues_NotEquals(t *testing.T) {
|
||||
handler := &CollectionHandler{}
|
||||
|
||||
result := handler.compareValues("Science Fiction", "not_equals", "Fantasy")
|
||||
assert.True(t, result)
|
||||
|
||||
result = handler.compareValues("Science Fiction", "not_equals", "Science Fiction")
|
||||
assert.False(t, result)
|
||||
}
|
||||
|
||||
func TestEvaluateRule_Genre(t *testing.T) {
|
||||
item := database.MediaItems{
|
||||
Genre: pgtype.Text{String: "Science Fiction", Valid: true},
|
||||
}
|
||||
|
||||
handler := &CollectionHandler{}
|
||||
|
||||
result := handler.evaluateRule(item, "genre", "equals", "Science Fiction")
|
||||
assert.True(t, result)
|
||||
|
||||
result = handler.evaluateRule(item, "genre", "equals", "Fantasy")
|
||||
assert.False(t, result)
|
||||
}
|
||||
|
||||
func TestEvaluateRule_Author(t *testing.T) {
|
||||
item := database.MediaItems{
|
||||
Author: pgtype.Text{String: "Isaac Asimov", Valid: true},
|
||||
}
|
||||
|
||||
handler := &CollectionHandler{}
|
||||
|
||||
result := handler.evaluateRule(item, "author", "contains", "asimov")
|
||||
assert.True(t, result)
|
||||
|
||||
result = handler.evaluateRule(item, "author", "contains", "clarke")
|
||||
assert.False(t, result)
|
||||
}
|
||||
|
||||
func TestEvaluateRule_CopyrightYear(t *testing.T) {
|
||||
item := database.MediaItems{
|
||||
CopyrightYear: pgtype.Int4{Int32: 2021, Valid: true},
|
||||
}
|
||||
|
||||
handler := &CollectionHandler{}
|
||||
|
||||
result := handler.evaluateRule(item, "copyright_year", "greater_than", "2000")
|
||||
assert.True(t, result)
|
||||
|
||||
result = handler.evaluateRule(item, "copyright_year", "less_than", "2000")
|
||||
assert.False(t, result)
|
||||
}
|
||||
|
||||
func TestTextToString(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
|
||||
Reference in new issue
Block a user