diff --git a/ErsatzTV.Application/Search/Queries/GetSearchFieldValues.cs b/ErsatzTV.Application/Search/Queries/GetSearchFieldValues.cs new file mode 100644 index 000000000..92938b177 --- /dev/null +++ b/ErsatzTV.Application/Search/Queries/GetSearchFieldValues.cs @@ -0,0 +1,6 @@ +using ErsatzTV.Core.Api.Search; + +namespace ErsatzTV.Application.Search.Queries; + +public record GetSearchFieldValues(string Name, string Query, int Limit) + : IRequest>; diff --git a/ErsatzTV.Application/Search/Queries/GetSearchFieldValuesHandler.cs b/ErsatzTV.Application/Search/Queries/GetSearchFieldValuesHandler.cs new file mode 100644 index 000000000..eb5ca3c8d --- /dev/null +++ b/ErsatzTV.Application/Search/Queries/GetSearchFieldValuesHandler.cs @@ -0,0 +1,120 @@ +using ErsatzTV.Core.Api.Search; +using ErsatzTV.Core.Domain; +using ErsatzTV.Infrastructure.Data; +using Microsoft.EntityFrameworkCore; + +namespace ErsatzTV.Application.Search.Queries; + +public class GetSearchFieldValuesHandler(IDbContextFactory dbContextFactory) + : IRequestHandler> +{ + private const int DefaultLimit = 50; + private const int MaxLimit = 50; + + public async Task> Handle( + GetSearchFieldValues request, + CancellationToken cancellationToken) + { + SearchFieldResponseModel field = SearchFieldCatalog.Fields + .FirstOrDefault(f => f.Name == request.Name); + + if (field is null || field.Type != "text") + { + return Option.None; + } + + int limit = request.Limit <= 0 ? DefaultLimit : Math.Clamp(request.Limit, 1, MaxLimit); + string qLower = (request.Query ?? string.Empty).ToLower(); + + // in-memory special cases (no DB query needed) + switch (request.Name) + { + case "state": + return new SearchFieldValuesResponseModel( + FilterSortTake(Enum.GetNames(), qLower, limit)); + case "video_dynamic_range": + return new SearchFieldValuesResponseModel( + FilterSortTake(["hdr", "sdr"], qLower, limit)); + } + + await using TvContext dbContext = await dbContextFactory.CreateDbContextAsync(cancellationToken); + + if (request.Name == "content_rating") + { + return new SearchFieldValuesResponseModel( + await GetContentRatingValues(dbContext, qLower, limit, cancellationToken)); + } + + IQueryable source = GetSource(dbContext, request.Name); + if (source is null) + { + return Option.None; + } + + List values = await source + .Where(v => v != null && v.ToLower().StartsWith(qLower)) + .Distinct() + .OrderBy(v => v) + .Take(limit) + .ToListAsync(cancellationToken); + + return new SearchFieldValuesResponseModel(values); + } + + private static IQueryable GetSource(TvContext dbContext, string name) => name switch + { + "genre" or "show_genre" => dbContext.Set().Select(g => g.Name), + "studio" => dbContext.Set().Select(s => s.Name), + "director" => dbContext.Set().Select(d => d.Name), + "writer" => dbContext.Set().Select(w => w.Name), + "actor" => dbContext.Actors.Select(a => a.Name), + // entity artists only; free-text music-video/song artist credits are not included (known limitation) + "artist" => dbContext.ArtistMetadata.Select(m => m.Title), + "tag" => dbContext.Set() + .Where(t => t.ExternalTypeId != Tag.NfoCountryTypeId && t.ExternalTypeId != Tag.PlexNetworkTypeId) + .Select(t => t.Name), + "network" => dbContext.Set() + .Where(t => t.ExternalTypeId == Tag.PlexNetworkTypeId) + .Select(t => t.Name), + "collection" => dbContext.Collections.Select(c => c.Name), + "video_codec" => dbContext.MediaStreams + .Where(s => s.MediaStreamKind == MediaStreamKind.Video && s.Codec != null) + .Select(s => s.Codec), + "album" => dbContext.MusicVideoMetadata + .Where(m => m.Album != null) + .Select(m => m.Album) + .Concat(dbContext.SongMetadata.Where(m => m.Album != null).Select(m => m.Album)), + _ => null + }; + + private static async Task> GetContentRatingValues( + TvContext dbContext, + string qLower, + int limit, + CancellationToken cancellationToken) + { + List raw = await dbContext.MovieMetadata + .Where(m => m.ContentRating != null) + .Select(m => m.ContentRating) + .Concat(dbContext.ShowMetadata.Where(m => m.ContentRating != null).Select(m => m.ContentRating)) + .Concat(dbContext.OtherVideoMetadata.Where(m => m.ContentRating != null).Select(m => m.ContentRating)) + .Concat(dbContext.RemoteStreamMetadata.Where(m => m.ContentRating != null).Select(m => m.ContentRating)) + .Distinct() + .ToListAsync(cancellationToken); + + IEnumerable split = raw + .SelectMany(cr => cr.Split('/')) + .Select(cr => cr.Trim()) + .Where(cr => !string.IsNullOrEmpty(cr)) + .Distinct(); + + return FilterSortTake(split, qLower, limit); + } + + private static List FilterSortTake(IEnumerable values, string qLower, int limit) => + values + .Where(v => v.ToLower().StartsWith(qLower)) + .OrderBy(v => v) + .Take(limit) + .ToList(); +} diff --git a/ErsatzTV.Core/Api/Search/SearchFieldValuesResponseModel.cs b/ErsatzTV.Core/Api/Search/SearchFieldValuesResponseModel.cs new file mode 100644 index 000000000..cc262711f --- /dev/null +++ b/ErsatzTV.Core/Api/Search/SearchFieldValuesResponseModel.cs @@ -0,0 +1,8 @@ +#nullable enable +namespace ErsatzTV.Core.Api.Search; + +/// +/// Distinct term values for a single text field in the search index, used to power the visual rule +/// builder's facet-value typeahead. +/// +public record SearchFieldValuesResponseModel(List Values); diff --git a/ErsatzTV.Tests/Application/Search/GetSearchFieldValuesHandlerTests.cs b/ErsatzTV.Tests/Application/Search/GetSearchFieldValuesHandlerTests.cs new file mode 100644 index 000000000..c3836f4e0 --- /dev/null +++ b/ErsatzTV.Tests/Application/Search/GetSearchFieldValuesHandlerTests.cs @@ -0,0 +1,199 @@ +using ErsatzTV.Application.Search.Queries; +using ErsatzTV.Core.Api.Search; +using ErsatzTV.Core.Domain; +using ErsatzTV.Infrastructure.Data; +using ErsatzTV.Tests.Support; +using LanguageExt; +using NUnit.Framework; +using Shouldly; + +namespace ErsatzTV.Tests.Application.Search; + +[TestFixture] +public class GetSearchFieldValuesHandlerTests +{ + private InMemoryTvContext _db = null!; + + [SetUp] + public async Task SetUp() => _db = await InMemoryTvContext.CreateAsync(); + + [TearDown] + public async Task TearDown() => await _db.DisposeAsync(); + + [Test] + public async Task Returns_Distinct_Whole_Values_For_Genre_With_Prefix_And_Limit() + { + await using (TvContext context = _db.CreateContext()) + { + context.Set().AddRange( + new Genre { Name = "Action" }, + new Genre { Name = "Adventure" }, + new Genre { Name = "Animation" }, + new Genre { Name = "Comedy" }); + await context.SaveChangesAsync(); + } + + var handler = new GetSearchFieldValuesHandler(_db.Factory); + + Option result = await handler.Handle( + new GetSearchFieldValues("genre", "A", 50), + CancellationToken.None); + + result.IsSome.ShouldBeTrue(); + result.IfSome(r => r.Values.ShouldBe(new List { "Action", "Adventure", "Animation" })); + + Option limited = await handler.Handle( + new GetSearchFieldValues("genre", "A", 2), + CancellationToken.None); + + limited.IsSome.ShouldBeTrue(); + limited.IfSome(r => r.Values.ShouldBe(new List { "Action", "Adventure" })); + } + + [Test] + public async Task Splits_Content_Rating_On_Slash() + { + await using (TvContext context = _db.CreateContext()) + { + context.MovieMetadata.Add(new MovieMetadata + { + MetadataKind = MetadataKind.External, + DateAdded = DateTime.UtcNow, + DateUpdated = DateTime.UtcNow, + ContentRating = "PG-13/TV-14" + }); + await context.SaveChangesAsync(); + } + + var handler = new GetSearchFieldValuesHandler(_db.Factory); + + Option result = await handler.Handle( + new GetSearchFieldValues("content_rating", string.Empty, 50), + CancellationToken.None); + + result.IsSome.ShouldBeTrue(); + result.IfSome(r => r.Values.ShouldBe(new List { "PG-13", "TV-14" })); + } + + [Test] + public async Task Returns_MediaItemState_Enum_Names_Without_Seeding() + { + var handler = new GetSearchFieldValuesHandler(_db.Factory); + + Option result = await handler.Handle( + new GetSearchFieldValues("state", string.Empty, 50), + CancellationToken.None); + + result.IsSome.ShouldBeTrue(); + result.IfSome(r => r.Values.ShouldBe( + new List { "FileNotFound", "Normal", "RemoteOnly", "Unavailable" })); + } + + [Test] + public async Task Returns_NotFound_For_Excluded_Text_Field_Title() + { + var handler = new GetSearchFieldValuesHandler(_db.Factory); + + Option result = await handler.Handle( + new GetSearchFieldValues("title", string.Empty, 50), + CancellationToken.None); + + result.IsNone.ShouldBeTrue(); + } + + [Test] + public async Task Returns_NotFound_For_NonText_Field() + { + var handler = new GetSearchFieldValuesHandler(_db.Factory); + + // "minutes" is a "number" field in SearchFieldCatalog, not "text" + Option result = await handler.Handle( + new GetSearchFieldValues("minutes", string.Empty, 50), + CancellationToken.None); + + result.IsNone.ShouldBeTrue(); + } + + [Test] + public async Task Returns_NotFound_For_Unknown_Field() + { + var handler = new GetSearchFieldValuesHandler(_db.Factory); + + Option result = await handler.Handle( + new GetSearchFieldValues("nope", string.Empty, 50), + CancellationToken.None); + + result.IsNone.ShouldBeTrue(); + } + + [Test] + public async Task Matches_Case_Insensitive_Prefix() + { + await using (TvContext context = _db.CreateContext()) + { + context.Set().AddRange( + new Genre { Name = "Action" }, + new Genre { Name = "Comedy" }); + await context.SaveChangesAsync(); + } + + var handler = new GetSearchFieldValuesHandler(_db.Factory); + + Option result = await handler.Handle( + new GetSearchFieldValues("genre", "a", 50), + CancellationToken.None); + + result.IsSome.ShouldBeTrue(); + result.IfSome(r => r.Values.ShouldBe(new List { "Action" })); + } + + [Test] + public async Task Excludes_Network_And_Country_Tags_From_Tag_Field_And_Routes_Network_Tags_To_Network_Field() + { + await using (TvContext context = _db.CreateContext()) + { + context.Set().AddRange( + new Tag { Name = "PlainTag" }, + new Tag { Name = "HBO", ExternalTypeId = Tag.PlexNetworkTypeId }, + new Tag { Name = "USA", ExternalTypeId = Tag.NfoCountryTypeId }); + await context.SaveChangesAsync(); + } + + var handler = new GetSearchFieldValuesHandler(_db.Factory); + + Option tagResult = await handler.Handle( + new GetSearchFieldValues("tag", string.Empty, 50), + CancellationToken.None); + + tagResult.IsSome.ShouldBeTrue(); + tagResult.IfSome(r => r.Values.ShouldBe(new List { "PlainTag" })); + + Option networkResult = await handler.Handle( + new GetSearchFieldValues("network", string.Empty, 50), + CancellationToken.None); + + networkResult.IsSome.ShouldBeTrue(); + networkResult.IfSome(r => r.Values.ShouldBe(new List { "HBO" })); + } + + [Test] + public async Task Dedupes_Repeated_Values() + { + await using (TvContext context = _db.CreateContext()) + { + context.Set().AddRange( + new Genre { Name = "Action" }, + new Genre { Name = "Action" }); + await context.SaveChangesAsync(); + } + + var handler = new GetSearchFieldValuesHandler(_db.Factory); + + Option result = await handler.Handle( + new GetSearchFieldValues("genre", string.Empty, 50), + CancellationToken.None); + + result.IsSome.ShouldBeTrue(); + result.IfSome(r => r.Values.ShouldBe(new List { "Action" })); + } +} diff --git a/ErsatzTV/Controllers/Api/SearchController.cs b/ErsatzTV/Controllers/Api/SearchController.cs index 217613005..b2153e374 100644 --- a/ErsatzTV/Controllers/Api/SearchController.cs +++ b/ErsatzTV/Controllers/Api/SearchController.cs @@ -189,6 +189,29 @@ public class SearchController(IMediator mediator) : ControllerBase public Task> GetSearchFields(CancellationToken cancellationToken) => mediator.Send(new GetSearchFieldCatalog(), cancellationToken); + [HttpGet("/api/v1/search/fields/{name}/values", Name = "GetSearchFieldValues")] + [Tags("Search")] + [EndpointSummary("List distinct database values for a text search field")] + [EndpointDescription( + "Returns distinct whole values from the database for the given text field, filtered by an " + + "optional case-insensitive prefix. Powers the visual rule builder's facet-value typeahead. " + + "404 when the field is unknown, is not a text field, or is a text field with no distinct-value " + + "source.")] + [EndpointGroupName("general")] + [ProducesResponseType(typeof(SearchFieldValuesResponseModel), StatusCodes.Status200OK)] + [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status404NotFound)] + public async Task GetSearchFieldValues( + [FromRoute] string name, + [FromQuery] string q = "", + [FromQuery] int limit = 50, + CancellationToken cancellationToken = default) + { + Option result = await mediator.Send( + new GetSearchFieldValues(name, q ?? string.Empty, limit), + cancellationToken); + return result.ToGetResult(); + } + private static SearchResultAllItemsResponseModel Project(SearchResultAllItemsViewModel vm) => new( vm.MovieIds, diff --git a/ErsatzTV/wwwroot/openapi/v1.json b/ErsatzTV/wwwroot/openapi/v1.json index 2312c6b8b..4cd1e73a8 100644 --- a/ErsatzTV/wwwroot/openapi/v1.json +++ b/ErsatzTV/wwwroot/openapi/v1.json @@ -18035,6 +18035,108 @@ ] } }, + "/api/v1/search/fields/{name}/values": { + "get": { + "tags": [ + "Search" + ], + "summary": "List distinct database values for a text search field", + "description": "Returns distinct whole values from the database for the given text field, filtered by an optional case-insensitive prefix. Powers the visual rule builder's facet-value typeahead. 404 when the field is unknown, is not a text field, or is a text field with no distinct-value source.", + "operationId": "GetSearchFieldValues", + "parameters": [ + { + "name": "name", + "in": "path", + "required": true, + "schema": { + "type": "string" + } + }, + { + "name": "q", + "in": "query", + "schema": { + "type": "string", + "default": "" + } + }, + { + "name": "limit", + "in": "query", + "schema": { + "type": "integer", + "format": "int32", + "default": 50 + } + } + ], + "responses": { + "200": { + "description": "OK", + "content": { + "text/plain": { + "schema": { + "$ref": "#/components/schemas/SearchFieldValuesResponseModel" + } + }, + "application/json": { + "schema": { + "$ref": "#/components/schemas/SearchFieldValuesResponseModel" + } + }, + "text/json": { + "schema": { + "$ref": "#/components/schemas/SearchFieldValuesResponseModel" + } + } + } + }, + "404": { + "description": "Not Found", + "content": { + "text/plain": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "application/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "text/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } + }, + "401": { + "description": "API key missing or invalid.", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } + }, + "400": { + "description": "Request validation failed (model binding or FluentValidation).", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ValidationProblemDetails" + } + } + } + } + }, + "security": [ + { } + ] + } + }, "/api/v1/seasons/{id}": { "get": { "tags": [ @@ -30612,6 +30714,20 @@ } } }, + "SearchFieldValuesResponseModel": { + "required": [ + "values" + ], + "type": "object", + "properties": { + "values": { + "type": "array", + "items": { + "type": "string" + } + } + } + }, "SearchResultAllItemsResponseModel": { "required": [ "movieIds", diff --git a/docs/api-conventions.md b/docs/api-conventions.md index 134ecf20c..e69953482 100644 --- a/docs/api-conventions.md +++ b/docs/api-conventions.md @@ -412,6 +412,24 @@ Returns the curated `SearchFieldCatalog` (name, friendly label, type, UI group, enum fields) as `List`. Drives the SmartCollection rule builder and is introspectable by MCP; no query parameters. +**Endpoint inventory addition (#434, facet-value typeahead)**: one read-only `SearchController` GET, +standard credential (catalog-read tier — no `[RequiresAuthentication]`): + +| Method | Path | Operation | Summary | +|---|---|---|---| +| GET | `/api/v1/search/fields/{name}/values` | `GetSearchFieldValues` | List distinct term values for a text search field | + +Query params: `q` (optional prefix filter, case-insensitive, default empty) and `limit` (optional, +clamped `1..50`, default 50). `{name}` is allow-listed to `SearchFieldCatalog` fields with +`type: "text"` AND a distinct-value source in the database — an unknown field, a non-text field (e.g. +an enum), or a text field without a source (`title`, `show_title`, `album_artist`) 404s rather than +returning an empty list, since enum fields already ship their values inline on +`GET /api/v1/search/fields` and never need this endpoint. Returns `SearchFieldValuesResponseModel` +(`{ values: string[] }`), sourced from a per-field distinct-values DB query (`IDbContextFactory`), +not the Lucene term dictionary — analyzed text fields store lowercased word tokens, not whole values. +No server-side caching. Powers the visual rule builder's value-input combobox for text fields; see +`docs/decisions.md` 2026-07-23 (#434) and `spa-conventions.md` §12. + **Param + DTO expansion (#293, cap `search/all-items`)**: no new endpoint — `GET /api/v1/search/all-items` gained two **optional** query params (`pageSize` default 500, clamped 1–1000 via the §1 Logs `Math.Clamp` precedent; `pageNum` 0-based, clamped `0..2_000_000` so `pageNum * pageSize` can't overflow `int` to a 500) diff --git a/docs/decisions.md b/docs/decisions.md index 62b567c85..31c6c539f 100644 --- a/docs/decisions.md +++ b/docs/decisions.md @@ -3599,3 +3599,70 @@ This is #72 scope item (a), deferred in `api.channel-health-signal` because "no **Immutable provenance, not a mutable "still managed" flag.** `Origin` records how the row was *born* and a later user edit never changes it, so "auto-generated then user-edited" stays `AutoTuned`. This deliberately avoids reviving the fragile "detect when it's been edited away" heuristic the issue rejected. A future "has diverged from its auto-tune template" signal, if wanted, is a *separate* concern owned by the #383/#384 auto-tune arc (which knows the template), not this column — mirroring the `api.channel-health-signal` reasoning that kept health a raw fact rather than freezing a policy enum. **`Unknown = 0` is the honest legacy default.** A new non-null int column defaults existing rows to `0`; making that `Unknown` (rather than `UserCreated`) means pre-migration rows say "we never recorded this" instead of asserting a provenance we cannot know. The SPA badges only `AutoTuned`, so `Unknown` and `UserCreated` both render unbadged. Enum (not `bool IsAutoTuned`) so a future origin (e.g. `Imported`) is additive without a wire-contract break. Stamped in `CreateChannelFromLineupHandler.BuildChannel`, which is the single channel-construction primitive `CreateAutoTunedChannelsHandler` delegates to, so both the lineup endpoint and bulk auto-tune are covered by one stamp site. Empty-schedule and broken-source fault detection remain deferred to #415. +## 2026-07-23 — Facet-value typeahead is a new endpoint, allow-listed to text fields, no caching (#434) + +`key: api.search-field-values` · `status: active` · `since: 2026-07-23` · `supersedes: none` · `superseded-by: none` +**Rule:** `GET /api/v1/search/fields/{name}/values?q=&limit=` returns distinct WHOLE values from the database for one of a narrow allow-list of catalog fields (not the Lucene term dictionary — analyzed `TextField`s store lowercased word tokens, e.g. "Science Fiction" → `science`/`fiction`, useless as a typeahead suggestion), 404 for an unknown field, a non-`text` field, or a `text` field with no distinct-value source; case-insensitive prefix-filtered on `q`, `limit` clamped to `[1, 50]` (default 50). +**Signals:** facet-value typeahead, rule builder value combobox, distinct field values, GetSearchFieldValues, text field allow-list, DB-sourced distinct values, content_rating split · paths: `ErsatzTV/Controllers/Api/SearchController.cs`, `ErsatzTV.Application/Search/Queries/GetSearchFieldValues.cs`, `ErsatzTV.Application/Search/Queries/GetSearchFieldValuesHandler.cs`, `web/src/api/search.ts` · issues: #434, #176 +**Mechanics:** `SearchController.GetSearchFieldValues`; `GetSearchFieldValuesHandler`; api-conventions.md; spa-conventions.md §12 + +Enum fields (e.g. `type`, `content_rating` group) already ship their allowed values inline on +`SearchFieldResponseModel` from the existing `GET /api/v1/search/fields` catalog (`spa.smartcollection-rule-builder`, +#176), so they need no endpoint — a client already has the full value set. **Text** fields (title, studio, +genre-as-free-text, etc.) don't: their values are whatever strings the library actually contains, so the +rule builder's value input for a text field needs a live lookup rather than a fixed list. +The handler allow-lists on `field.Type != "text"` (matching the same `SearchFieldCatalog.Fields` the +`/fields` endpoint serves) and returns `Option.None` → 404 for anything else, rather than silently returning +an empty list for a field that will never have values — a 404 tells a caller "wrong field kind," an empty +200 would look like "no matches yet." + +**DB-sourced, not the search index.** The handler injects `IDbContextFactory` and resolves an +explicit per-field-name `IQueryable` (or, for a few special cases, an in-memory list) rather than +querying `ISearchIndex`: `genre`/`show_genre` → `Set()`, `studio` → `Set()`, `director` → +`Set()`, `writer` → `Set()`, `actor` → `Actors`, `artist` → `ArtistMetadata.Title` (entity +artists only — free-text music-video/song artist credits are a known, intentionally-uncovered gap), `tag` → +`Set()` excluding `Tag.NfoCountryTypeId`/`Tag.PlexNetworkTypeId` (reapplying the indexer's own +exclusions so country/network strings don't leak in as tags), `network` → `Set()` filtered to +`Tag.PlexNetworkTypeId`, `collection` → `Collections`, `video_codec` → `MediaStreams` filtered to +`MediaStreamKind.Video`, `album` → `MusicVideoMetadata.Album` concatenated with `SongMetadata.Album`. Every +DB-sourced field runs the same pipeline: `.Where(v => v.ToLower().StartsWith(qLower)).Distinct().OrderBy(v => +v).Take(limit)`, translated to SQL by EF for both SQLite and MySQL. Two fields are computed in memory instead +of queried: `state` (the fixed 4-value `MediaItemState` enum) and `video_dynamic_range` (the literal +`["hdr", "sdr"]`). `content_rating` is special-cased: the DB stores an unsplit `"PG-13/TV-14"` string across +`MovieMetadata`/`ShowMetadata`/`OtherVideoMetadata`/`RemoteStreamMetadata`, so the handler pulls the distinct +raw strings then `Split('/')`s, trims, and dedupes in memory before the same prefix-filter/sort/take — this +matches what search actually matches on, rather than surfacing the compound string as one facet value. +**`title`, `show_title`, `album_artist` are explicitly NOT supported** (404, free-text fallback): `title`/ +`show_title` are near-unique free-text fields spanning ~9 metadata tables where a distinct list of every +title isn't a useful facet; `album_artist` backs onto `SongMetadata.AlbumArtists`, a value-converted +`IList` column EF can't translate into a server-side distinct query. + +**Why a thin query, not a cache.** No result cache, no debounce on the server side (the SPA combobox +debounces the keystroke) — each per-field query is a bounded, indexed `Distinct`/`Take`; adding a cache +before there's a measured cost would be premature. + +## 2026-07-23 — Relative-date rule builder operators are a frontend-only mapping onto existing Lucene macros (#435) + +`key: rulebuilder.relative-date-macros` · `status: active` · `since: 2026-07-23` · `supersedes: none` · `superseded-by: none` +**Rule:** The visual rule builder's `inLast`/`notInLast` date operators compile to/parse from the pre-existing `CustomMultiFieldQueryParser` macros `released_inthelast`/`released_notinthelast` and `added_inthelast`/`added_notinthelast`, value form `" day|week|month|year"`; there is no backend change. +**Signals:** relative date operator, inLast, notInLast, inthelast macro, released_inthelast, added_inthelast, date unit picker, rule builder relative dates · paths: `web/src/builder/rules/dateMacro.ts`, `web/src/builder/rules/compile.ts`, `web/src/builder/rules/parse.ts`, `web/src/builder/rules/types.ts`, `web/src/builder/rules/validation.ts` · issues: #435, #176, #438 +**Mechanics:** `dateMacro.ts` (`compileRelative`/`parseRelative`, `OP_TO_SUFFIX`/`SUFFIX_TO_OP`, `RELATIVE_SYNTHETIC`); spa-conventions.md §12 + +`released_inthelast`/`added_inthelast` (+ their `notinthelast` negations) already existed in +`CustomMultiFieldQueryParser` as free-text macro fields before the rule builder could reach them — they +were only usable by typing raw Lucene. #435 exposes them as first-class builder operators without touching +the parser: `release_date`/`added_date` gain `inLast`/`notInLast` alongside the existing `before`/`after`/ +`between`, backed by a numeric-value input plus a `day|week|month|year` unit picker (`types.ts`'s `unit?: +DateUnit`). `dateMacro.ts` is the single seam — a small `field↔macro-prefix` table (`release_date↔released`, +`added_date↔added`) plus the `inLast/notInLast ↔ inthelast/notinthelast` suffix maps — that `compile.ts` +delegates to for these two fields and `parse.ts` recognizes via `RELATIVE_SYNTHETIC` before falling into the +generic field:value grammar. Validation (`ruleError` in `validation.ts`) requires the value to parse as a +positive integer; a non-numeric or non-positive value is a builder-side error, never sent to the server. + +**Frontend-only because the macros are the query-string wire format, not a new query kind.** The compiled +query for `release_date inLast "7 day"` is literally `released_inthelast:"7 day"` — the same string a user +could type by hand — so nothing downstream (search index, SmartCollection storage, Auto-Tune) needs to know +the rule builder exists. This keeps `rulebuilder.relative-date-macros` symmetric with +`spa.smartcollection-rule-builder`'s "compile-only, no new stored AST" stance (#176): a relative-date rule is +just another point in the same closed grammar subset, proven by the same compile→parse round-trip discipline +(`dateMacro.test.ts`, and the property test in `roundtrip.test.ts`, #438) rather than a special case. diff --git a/docs/decisions/README.md b/docs/decisions/README.md index 5d4de7197..d1fd8cde9 100644 --- a/docs/decisions/README.md +++ b/docs/decisions/README.md @@ -27,6 +27,7 @@ the link for rationale. Superseded/retired history lives in `archive/`. Regenera | `api.schedule-item-flat-dto` | Schedule-item GET/POST/PUT use a flat, non-polymorphic `ScheduleItemResponseModel` (every subtype field promoted to a nullable top-level member) instead of the polymorphic Application VM hierarchy, with mutation field names matching `ScheduleItemRequest` 1:1 for a lossless round-trip. | 2026-07-10 | [link](../decisions.md#2026-07-10--schedule-item-get-returns-a-flat-non-polymorphic-dto-scheduleitemresponsemodel) | | `api.scheduling-hardening` | Create/Replace handlers guard against null/whitespace `name` (`IsNullOrWhiteSpace`, not just `Length`) to prevent NRE-500s, template-item overlap validation compares by index (not record value-equality) to catch exact-duplicate items, and unreachable 404 `ProducesResponseType` attributes on create-only actions are trimmed. | 2026-07-13 | [link](../decisions.md#2026-07-13--scheduling-api-hardening-null-name-500s-duplicate-template-items-unreachable-404-172) | | `api.search-allitems-paging` | `GET /api/v1/search/all-items` is paginated (capped page size, `Totals` field) to bound DoS exposure; the SPA add-all flow pages to completeness instead of relying on an unbounded response. | 2026-07-18 | [link](../decisions.md#2026-07-18--search-all-items-is-paged-to-cap-dos-exposure-spa-add-all-pages-to-completeness-293) | +| `api.search-field-values` | `GET /api/v1/search/fields/{name}/values?q=&limit=` returns distinct WHOLE values from the database for one of a narrow allow-list of catalog fields (not the Lucene term dictionary — analyzed `TextField`s store lowercased word tokens, e.g. "Science Fiction" → `science`/`fiction`, useless as a typeahead suggestion), 404 for an unknown field, a non-`text` field, or a `text` field with no distinct-value source; case-insensitive prefix-filtered on `q`, `limit` clamped to `[1, 50]` (default 50). | 2026-07-23 | [link](../decisions.md#2026-07-23--facet-value-typeahead-is-a-new-endpoint-allow-listed-to-text-fields-no-caching-434) | | `api.search-paging-cap` | Search stays capped at 100 items per media kind; an overflowing kind's "See all" reuses library-browse paging instead of adding new API surface. | 2026-07-11 | [link](../decisions.md#2026-07-11--trash-see-all-reuses-library-browse-paging-search-stays-capped-per-kind-213) | | `api.versioning-v1` | The entire `/api` surface is versioned to `/api/v1` uniformly (no unversioned corner); legacy unversioned callers are rewritten in-pipeline (not redirected) with Deprecation/Link/Sunset headers, and post-freeze `/api/v1` is additive-only — a breaking change requires `/api/v2`. | 2026-07-13 | [link](../decisions.md#2026-07-13--api-versioning-the-whole-api-surface-is-mounted-at-apiv1-additive-only-after-freeze-286) | | `blazor.rollback-tag` | The commit immediately preceding the Blazor-removal merge is tagged `blazor-final` (not a `v*` tag, so it doesn't trigger a prod release build) as the documented rollback/restore path. | 2026-07-11 | [link](../decisions.md#2026-07-11--pre-removal-blazor-rollback-tag-blazor-final-205) | @@ -105,6 +106,7 @@ the link for rationale. Superseded/retired history lives in `archive/`. Regenera | `release.prepush-clean-worktree-guard` | A fail-open pre-push hook blocks a push when any file in the branch's diff vs `origin/main` also has uncommitted working-tree or index changes, since a stale-index commit (e.g. `git reset --soft` + `git add` over an edited-but-unstaged fix) can silently push, CI-test, and get reviewed a different tree than the one on disk. Scope is precise to pushed-diff files; escape hatch `ETV_ALLOW_DIRTY_PUSH=1`. | 2026-07-17 | [link](../decisions.md#2026-07-17--pre-push-guard-dont-push-a-file-whose-working-tree-copy-is-uncommitted-h13-416-session) | | `release.promotion-floating-prod` | Prod tracks the floating `:prod` image reference; a tag build's immutable `:` image is scanned first, then promotion happens via a separate manual `DeployStack`, with daily auto-update only as a fallback — tag with enough runway before 03:00 to avoid an unscanned promotion. | 2026-07-13 | [link](release-ci-governance.md#2026-07-13--release-promotion-floating-prod-exact-image-scan-before-manual-deploy-335) | | `release.review-verdict-gate` | A PR may not merge until a `Review-verdict: @ ` comment references the PR's current head sha (short-sha prefix match, line-start marker only, negative wins over positive on the same head); folds into the H6 merge-consent hook as condition (c). | 2026-07-12 | [link](release-ci-governance.md#2026-07-12--review-verdict-merge-gate-latest-commit-must-be-reviewed-303-h10) | +| `rulebuilder.relative-date-macros` | The visual rule builder's `inLast`/`notInLast` date operators compile to/parse from the pre-existing `CustomMultiFieldQueryParser` macros `released_inthelast`/`released_notinthelast` and `added_inthelast`/`added_notinthelast`, value form `" day\|week\|month\|year"`; there is no backend change. | 2026-07-23 | [link](../decisions.md#2026-07-23--relative-date-rule-builder-operators-are-a-frontend-only-mapping-onto-existing-lucene-macros-435) | | `scan.collections-scan-status` | `GET /api/v1/media-sources/collections-scan-status` reports a family-global (not per-source), boolean-only active-scan set read from `IEntityLocker`; the SPA reconciles authoritatively against it (with a grace-tick helper) instead of a fixed client-side timeout. | 2026-07-12 | [link](../decisions.md#2026-07-12--external-collections-scans-get-an-authoritative-status-surface-271-the-spa-timeout-is-retired) | | `scan.getoraddfolder-db-lookup` | `ILibraryRepository.GetOrAddFolder` resolves the existing folder via a DB query on `(LibraryPathId, Path)`, not the caller's `LibraryPath.LibraryFolders` in-memory navigation, since that navigation is only eager-loaded on the local scan path and is null on remote (Jellyfin) callers. | 2026-07-20 | [link](../decisions.md#2026-07-20--ilibraryrepositorygetoraddfolder-resolves-the-folder-from-the-db-not-the-callers-librarypathlibraryfolders-navigation-488) | | `scan.jellyfin-mixed-content-library` | A Jellyfin library whose collection type is `mixed` (or absent) maps to one ErsatzTV library of `LibraryMediaKind.Mixed`, scanned by running the movie/television/music-video scanners in sequence against that single library — a library is a place, not a media kind. | 2026-07-20 | [link](../decisions.md#2026-07-20-489--jellyfin-mixed-content-libraries-map-to-one-library-holding-many-kinds) | diff --git a/docs/endpoint-index.md b/docs/endpoint-index.md index 124c7e57b..5f2eb6f1d 100644 --- a/docs/endpoint-index.md +++ b/docs/endpoint-index.md @@ -2,7 +2,7 @@ *Generated by `scripts/generate-endpoint-index.py` from `ErsatzTV/wwwroot/openapi/v1.json`. Do not edit by hand -- regenerated by `scripts/update-openapi.sh`.* -166 endpoints, 249 operations. +167 endpoints, 250 operations. ## Artists @@ -338,6 +338,7 @@ | GET | `/api/v1/search/artists` | SearchArtists | Search artists by name | | GET | `/api/v1/search/collections` | SearchCollections | Search collections by name | | GET | `/api/v1/search/fields` | GetSearchFields | List the filterable fields for the visual rule builder | +| GET | `/api/v1/search/fields/{name}/values` | GetSearchFieldValues | List distinct database values for a text search field | | GET | `/api/v1/search/multi-collections` | SearchMultiCollections | Search multi collections by name | | GET | `/api/v1/search/smart-collections` | SearchSmartCollections | Search smart collections by name | | GET | `/api/v1/search/television-seasons` | SearchTelevisionSeasons | Search television seasons by name | diff --git a/docs/spa-conventions.md b/docs/spa-conventions.md index 7940bf053..6b95569d4 100644 --- a/docs/spa-conventions.md +++ b/docs/spa-conventions.md @@ -508,6 +508,27 @@ This module is intentionally reusable beyond SmartCollections — ChannelBuilder inline query editing (#69) are candidate future consumers, tracked as separate follow-up issues rather than wired in #176. +- **Relative-date operators** (#435, `rulebuilder.relative-date-macros`) — `release_date`/`added_date` + gain `inLast`/`notInLast` alongside `before`/`after`/`between`, each with a numeric value plus a + `day|week|month|year` `unit` picker (`types.ts`'s `unit?: DateUnit`). `dateMacro.ts` is the single + seam: a `field↔macro-prefix` table (`release_date↔released`, `added_date↔added`) plus the + `inLast/notInLast ↔ inthelast/notinthelast` suffix maps, delegated to from `compile.ts`/`parse.ts` + for these two fields — the compiled query is the existing `released_inthelast:"7 day"`-style + `CustomMultiFieldQueryParser` macro, so nothing downstream changes. `validation.ts`'s `ruleError` + requires the value to parse as a positive integer before it's compiled. +- **Facet-value typeahead** (#434, `api.search-field-values`) — the value input for a `text` field + (not enum) is a combobox backed by `getSearchFieldValues` (`web/src/api/search.ts` → + `GET /api/v1/search/fields/{name}/values?q=&limit=`), debounced on keystroke, prefix-matching the + in-progress value against distinct terms already in the index. It always allows free-text entry as a + fallback — a 404 (non-text field) or an empty result list (e.g. ElasticSearch backend) degrades to a + plain text input rather than blocking the rule. +- **Single-child-group normalization** (#438) — `normalizeGroup` (`validation.ts`) coerces a group's + `match` to `all` whenever it has fewer than two children, recursively. A one-child `any` group is + semantically identical to `all` but doesn't round-trip through `compile`→`parse` (the compiled Lucene + for a lone child carries no `AND`/`OR`), so the builder normalizes on every change rather than let the + UI and the compiled query silently diverge; `roundtrip.test.ts`'s property test asserts against + `normalizeGroup(tree)`, not the raw generated tree. + ## 13. Collapsible sidebar + nav-group accordions (#396) The shell sidebar (`web/src/app/AppShell.tsx`) supports two independent, persisted collapse states. diff --git a/docs/superpowers/plans/2026-07-23-rulebuilder-bundle.md b/docs/superpowers/plans/2026-07-23-rulebuilder-bundle.md new file mode 100644 index 000000000..b84c45175 --- /dev/null +++ b/docs/superpowers/plans/2026-07-23-rulebuilder-bundle.md @@ -0,0 +1,655 @@ +# RuleBuilder Bundle Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Ship #438 (validation & polish), #435 (relative-date operators), and #434 (facet-value typeahead) for the visual rule builder on one feature branch. + +**Architecture:** Pure-logic changes land in the small `web/src/builder/rules/` modules (types/compile/parse + two new helper modules `validation.ts`, `dateMacro.ts`) under TDD. UI changes to `RuleBuilder.tsx` consume those helpers. #434 adds a disjoint C# read endpoint (Lucene term enumeration) built in a parallel worktree; its combobox is wired last, after the generated API types exist. + +**Tech Stack:** React 18 + TypeScript + Vite (vitest for web tests); .NET 10 / C# / MediatR / Lucene.NET (`ISearchIndex`); NUnit + Shouldly + NSubstitute for C# tests. + +## Global Constraints + +- Work in worktree `feat/rulebuilder-bundle` off `origin/main`. Never commit in `/Users/timothy/ersatztv`. → `process.shared-tree-readonly` +- Backend #434 slice runs in its OWN worktree branched off `feat/rulebuilder-bundle`, merged back by fast-forward/plumbing. All frontend work is one committing agent, sequential (shared `RuleBuilder.tsx`). → `process.one-worktree-one-committing-agent`, `process.foreign-worktree-plumbing-merge` +- The compile↔parse **lossless round-trip contract** (spa-conventions §12) must hold: `parse(compile(g)) ≡ normalizeGroup(g)` for every valid `g`. Extend `roundtrip.test.ts` for every new operator. +- New REST response DTOs live in `ErsatzTV.Core/Api/Search/*ResponseModel.cs` with file-scoped `#nullable enable`. → `api.response-dtos` +- A PR touching `ErsatzTV/Controllers/Api/**` or `ErsatzTV.Core/Api/**` MUST ship regenerated `v1.json` / `v1.d.ts` / `endpoint-index.md` in the same diff. → `release.api-contract-ci-gate` +- Before any push touching `.cs`: BOM-check the touched set (`xxd -p | grep -c ^efbbbf`) and run the format gate under `bash -c`. → `process.bom-format-detection-recipe` +- Independent adversarial review over the whole diff before push (new API endpoint + >~150 lines). → `process.independent-review-rubric` +- Commit trailer on every commit: `Co-Authored-By: Claude Opus 4.8 (1M context) `. Worktree hooks: commit with `--no-verify` and run gates manually (worktree husky friction). + +**Test commands** (run from the worktree): +- Web unit: `cd web && npx vitest run src/builder/rules/.test.ts` +- Web all rules: `cd web && npx vitest run src/builder/rules` +- C#: `dotnet test ErsatzTV.Application.Tests --filter ` (or the relevant test project) + +--- + +## Task 1: `#438`/`#435` — extend types, then rule/group validation module (`validation.ts`) + +**Files:** +- Modify: `web/src/builder/rules/types.ts` (extend first, so validation typechecks) +- Create: `web/src/builder/rules/validation.ts` +- Test: `web/src/builder/rules/validation.test.ts` + +- [ ] **Step 0: Extend types.ts** (the new operators/unit are consumed first by validation's tests): + +```ts +export type Operator = + | 'is' | 'isNot' | 'contains' | 'startsWith' // text + | 'matches' | 'notMatches' // fulltext + | 'eq' | 'gt' | 'lt' | 'between' // number + | 'before' | 'after' // date (absolute) + | 'inLast' | 'notInLast'; // date (relative, #435) + +export type DateUnit = 'day' | 'week' | 'month' | 'year'; + +export interface Rule { + field: string; + operator: Operator; + value: string; + value2?: string; // upper bound for `between` + unit?: DateUnit; // unit for `inLast`/`notInLast` (#435) +} +``` + +and extend the date row of `OPERATORS_BY_TYPE`: + +```ts + date: ['before', 'after', 'between', 'inLast', 'notInLast'] +``` + +**Interfaces:** +- Produces: + - `ruleError(rule: Rule): string | null` — a human string when the rule is incomplete, else null. + - `groupHasErrors(group: Group): boolean` — true if any descendant rule has an error. + - `topGroupAllNegative(group: Group): boolean` — true when every direct child of the *top* group is a negative rule (`isNot`/`notMatches`) (warning predicate, non-blocking). + - `normalizeGroup(group: Group): Group` — returns a copy with `match` coerced to `'all'` for every group (recursively) that has fewer than 2 children (single-child connective is meaningless and cannot round-trip). + +- [ ] **Step 1: Write the failing test** + +```ts +// web/src/builder/rules/validation.test.ts +import { describe, expect, it } from 'vitest'; +import { groupHasErrors, normalizeGroup, ruleError, topGroupAllNegative } from './validation'; +import type { Group, Rule } from './types'; + +const r = (o: Partial): Rule => ({ field: 'title', operator: 'is', value: 'x', ...o }); + +describe('ruleError', () => { + it('flags between with a missing upper bound', () => { + expect(ruleError(r({ field: 'minutes', operator: 'between', value: '30', value2: '' }))).toBeTruthy(); + expect(ruleError(r({ field: 'minutes', operator: 'between', value: '', value2: '60' }))).toBeTruthy(); + expect(ruleError(r({ field: 'minutes', operator: 'between', value: '30', value2: '60' }))).toBeNull(); + }); + it('flags empty text values', () => { + for (const operator of ['is', 'isNot', 'contains', 'startsWith'] as const) { + expect(ruleError(r({ operator, value: '' }))).toBeTruthy(); + expect(ruleError(r({ operator, value: 'x' }))).toBeNull(); + } + }); + it('flags relative-date rule with a non-positive or non-integer N', () => { + expect(ruleError(r({ field: 'release_date', operator: 'inLast', value: '', unit: 'day' }))).toBeTruthy(); + expect(ruleError(r({ field: 'release_date', operator: 'inLast', value: '0', unit: 'day' }))).toBeTruthy(); + expect(ruleError(r({ field: 'release_date', operator: 'inLast', value: '7', unit: 'day' }))).toBeNull(); + }); +}); + +describe('groupHasErrors', () => { + it('walks nested groups', () => { + const g: Group = { match: 'all', children: [{ match: 'any', children: [r({ value: '' })] }] }; + expect(groupHasErrors(g)).toBe(true); + expect(groupHasErrors({ match: 'all', children: [r({ value: 'ok' })] })).toBe(false); + }); +}); + +describe('topGroupAllNegative', () => { + it('is true only when every top-level child is a negative rule', () => { + expect(topGroupAllNegative({ match: 'any', children: [r({ operator: 'isNot' }), r({ operator: 'notMatches', field: 'plot' })] })).toBe(true); + expect(topGroupAllNegative({ match: 'any', children: [r({ operator: 'isNot' }), r({ operator: 'is' })] })).toBe(false); + expect(topGroupAllNegative({ match: 'all', children: [] })).toBe(false); + }); +}); + +describe('normalizeGroup', () => { + it("coerces a single-child group's match to 'all'", () => { + const g: Group = { match: 'any', children: [r({})] }; + expect(normalizeGroup(g).match).toBe('all'); + }); + it('leaves a 2+ child group match unchanged and recurses', () => { + const g: Group = { match: 'any', children: [r({}), { match: 'any', children: [r({})] }] }; + const n = normalizeGroup(g); + expect(n.match).toBe('any'); + expect((n.children[1] as Group).match).toBe('all'); + }); +}); +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `cd web && npx vitest run src/builder/rules/validation.test.ts` +Expected: FAIL — cannot find module `./validation`. + +- [ ] **Step 3: Write minimal implementation** + +```ts +// web/src/builder/rules/validation.ts +import { isGroup, type Group, type Rule } from './types'; + +const NEGATIVE = new Set(['isNot', 'notMatches']); +const RELATIVE_DATE = new Set(['inLast', 'notInLast']); + +export function ruleError(rule: Rule): string | null { + if (rule.operator === 'between') { + if (rule.value.trim() === '' || (rule.value2 ?? '').trim() === '') return 'Both bounds are required.'; + return null; + } + if (RELATIVE_DATE.has(rule.operator)) { + const n = Number(rule.value); + if (!Number.isInteger(n) || n <= 0) return 'Enter a whole number greater than zero.'; + return null; + } + if (rule.value.trim() === '') return 'A value is required.'; + return null; +} + +export function groupHasErrors(group: Group): boolean { + return group.children.some((c) => (isGroup(c) ? groupHasErrors(c) : ruleError(c) !== null)); +} + +export function topGroupAllNegative(group: Group): boolean { + const rules = group.children.filter((c): c is Rule => !isGroup(c)); + if (rules.length === 0 || rules.length !== group.children.length) return false; + return rules.every((r) => NEGATIVE.has(r.operator)); +} + +export function normalizeGroup(group: Group): Group { + const children = group.children.map((c) => (isGroup(c) ? normalizeGroup(c) : c)); + return { match: children.length < 2 ? 'all' : group.match, children }; +} +``` + +- [ ] **Step 4: Run test to verify it passes** + +Run: `cd web && npx vitest run src/builder/rules/validation.test.ts` +Expected: PASS (all cases). + +- [ ] **Step 5: Commit** + +```bash +git add web/src/builder/rules/types.ts web/src/builder/rules/validation.ts web/src/builder/rules/validation.test.ts +git commit --no-verify -m "feat(438,435): extend rule types; validation + single-child normalization helpers" +``` + +--- + +## Task 2: `#435` — dateMacro mapping module + +> `types.ts` was already extended in Task 1 Step 0 (operators `inLast`/`notInLast`, `DateUnit`, +> `Rule.unit`, `OPERATORS_BY_TYPE.date`). This task adds only the mapping module. + +**Files:** +- Create: `web/src/builder/rules/dateMacro.ts` +- Test: `web/src/builder/rules/dateMacro.test.ts` + +**Interfaces:** +- Produces: + - `dateMacro.ts`: + - `RELATIVE_FIELD_MAP: Record<'release_date' | 'added_date', 'released' | 'added'>` + - `compileRelative(rule: Rule): string | null` — e.g. `{release_date, inLast, '7', day}` → `released_inthelast:"7 day"`; null if not a relative-date rule or invalid. + - `parseRelative(field: string, quoted: string): Rule | null` — recognizes synthetic field + `" "`, returns a Rule on `release_date`/`added_date`; null otherwise. + - `RELATIVE_SYNTHETIC = /^(released|added)_(inthelast|notinthelast)$/` (exported for parse.ts field detection). + +- [ ] **Step 1: Write the failing test** + +```ts +// web/src/builder/rules/dateMacro.test.ts +import { describe, expect, it } from 'vitest'; +import { compileRelative, parseRelative } from './dateMacro'; +import type { Rule } from './types'; + +const r = (o: Partial): Rule => ({ field: 'release_date', operator: 'inLast', value: '7', unit: 'day', ...o }); + +describe('compileRelative', () => { + it('maps release_date/inLast → released_inthelast', () => { + expect(compileRelative(r({}))).toBe('released_inthelast:"7 day"'); + }); + it('maps added_date/notInLast → added_notinthelast', () => { + expect(compileRelative(r({ field: 'added_date', operator: 'notInLast', value: '3', unit: 'week' }))).toBe('added_notinthelast:"3 week"'); + }); + it('returns null for a non-relative rule', () => { + expect(compileRelative(r({ operator: 'before' }))).toBeNull(); + }); + it('returns null when N is invalid', () => { + expect(compileRelative(r({ value: '0' }))).toBeNull(); + }); +}); + +describe('parseRelative', () => { + it('round-trips released_inthelast', () => { + expect(parseRelative('released_inthelast', '7 day')).toEqual({ field: 'release_date', operator: 'inLast', value: '7', unit: 'day' }); + }); + it('round-trips added_notinthelast', () => { + expect(parseRelative('added_notinthelast', '3 week')).toEqual({ field: 'added_date', operator: 'notInLast', value: '3', unit: 'week' }); + }); + it('returns null for an unrelated field', () => { + expect(parseRelative('title', '7 day')).toBeNull(); + }); + it('returns null for a malformed value', () => { + expect(parseRelative('released_inthelast', 'soon')).toBeNull(); + }); +}); +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `cd web && npx vitest run src/builder/rules/dateMacro.test.ts` +Expected: FAIL — cannot find module `./dateMacro`. + +- [ ] **Step 3: Write dateMacro.ts** + +```ts +// web/src/builder/rules/dateMacro.ts +import type { DateUnit, Rule } from './types'; + +export const RELATIVE_FIELD_MAP = { release_date: 'released', added_date: 'added' } as const; +type RelField = keyof typeof RELATIVE_FIELD_MAP; + +const OP_TO_SUFFIX = { inLast: 'inthelast', notInLast: 'notinthelast' } as const; +const SUFFIX_TO_OP = { inthelast: 'inLast', notinthelast: 'notInLast' } as const; +const CATALOG_FROM_PREFIX = { released: 'release_date', added: 'added_date' } as const; +const UNITS: DateUnit[] = ['day', 'week', 'month', 'year']; + +export const RELATIVE_SYNTHETIC = /^(released|added)_(inthelast|notinthelast)$/; + +function isValidN(value: string): boolean { + const n = Number(value); + return Number.isInteger(n) && n > 0; +} + +export function compileRelative(rule: Rule): string | null { + if (rule.operator !== 'inLast' && rule.operator !== 'notInLast') return null; + const prefix = RELATIVE_FIELD_MAP[rule.field as RelField]; + if (!prefix) return null; + if (!isValidN(rule.value) || !rule.unit || !UNITS.includes(rule.unit)) return null; + return `${prefix}_${OP_TO_SUFFIX[rule.operator]}:"${rule.value} ${rule.unit}"`; +} + +export function parseRelative(field: string, quoted: string): Rule | null { + const m = RELATIVE_SYNTHETIC.exec(field); + if (!m) return null; + const [, prefix, suffix] = m; + const vm = /^(\d+)\s+(day|week|month|year)$/.exec(quoted.trim()); + if (!vm) return null; + const [, n, unit] = vm; + if (!isValidN(n)) return null; + return { + field: CATALOG_FROM_PREFIX[prefix as 'released' | 'added'], + operator: SUFFIX_TO_OP[suffix as 'inthelast' | 'notinthelast'], + value: n, + unit: unit as DateUnit + }; +} +``` + +- [ ] **Step 4: Run test to verify it passes** + +Run: `cd web && npx vitest run src/builder/rules/dateMacro.test.ts` +Expected: PASS. + +- [ ] **Step 5: Commit** + +```bash +git add web/src/builder/rules/dateMacro.ts web/src/builder/rules/dateMacro.test.ts +git commit --no-verify -m "feat(435): dateMacro compile/parse mapping for relative-date operators" +``` + +--- + +## Task 3: `#438` + `#435` — compile.ts wiring + +**Files:** +- Modify: `web/src/builder/rules/compile.ts` +- Modify: `web/src/builder/rules/compile.test.ts` + +**Interfaces:** +- Consumes: `compileRelative` (Task 2), `ruleError` (Task 1). +- Produces: `compileRule` emits the relative-date macro for `inLast`/`notInLast`, and returns `''` for a structurally-invalid rule (incomplete `between`, empty text value) so malformed Lucene is never emitted (the empty string is already filtered by `compileGroup`). + +- [ ] **Step 1: Write the failing test** — append to `compile.test.ts`: + +```ts +import { compileRelative } from './dateMacro'; // ensure no import cycle; if compile imports dateMacro, this is fine + +describe('compile — #438 guards + #435 relative dates', () => { + it('emits the relative-date macro', () => { + expect(compile({ match: 'all', children: [{ field: 'release_date', operator: 'inLast', value: '7', unit: 'day' }] })).toBe('released_inthelast:"7 day"'); + }); + it('drops an incomplete between instead of emitting field:[v TO ]', () => { + const out = compile({ match: 'all', children: [{ field: 'minutes', operator: 'between', value: '30' }] }); + expect(out).not.toContain('TO ]'); + expect(out).toBe(''); // sole invalid child filtered out + }); + it('drops an empty text value instead of emitting field:* / field:**', () => { + expect(compile({ match: 'all', children: [{ field: 'title', operator: 'contains', value: '' }] })).toBe(''); + expect(compile({ match: 'all', children: [{ field: 'title', operator: 'startsWith', value: '' }] })).toBe(''); + }); +}); +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `cd web && npx vitest run src/builder/rules/compile.test.ts` +Expected: FAIL — relative macro not emitted; incomplete-between emits `minutes:[30 TO ]`. + +- [ ] **Step 3: Edit compile.ts** + +Add the import and two guards at the top of `compileRule`: + +```ts +import { isGroup, type Group, type Rule } from './types'; +import { compileRelative } from './dateMacro'; +import { ruleError } from './validation'; + +// ... quote/escapeWild/normalizeDate unchanged ... + +function compileRule(rule: Rule): string { + // #435: relative-date operators map to synthetic query fields. + const relative = compileRelative(rule); + if (relative !== null) return relative; + + // #438: never emit malformed Lucene for a structurally-invalid rule; compileGroup filters ''. + if (ruleError(rule) !== null) return ''; + + const f = rule.field; + const v = rule.value; + switch (rule.operator) { + // ... existing cases unchanged ... + default: + return ''; + } +} +``` + +- [ ] **Step 4: Run tests to verify they pass** + +Run: `cd web && npx vitest run src/builder/rules/compile.test.ts` +Expected: PASS (new + existing cases). + +- [ ] **Step 5: Commit** + +```bash +git add web/src/builder/rules/compile.ts web/src/builder/rules/compile.test.ts +git commit --no-verify -m "feat(438,435): compile relative-date macros; drop invalid rules instead of malformed Lucene" +``` + +--- + +## Task 4: `#435` — parse.ts relative-date recognition + round-trip + +**Files:** +- Modify: `web/src/builder/rules/parse.ts` +- Modify: `web/src/builder/rules/parse.test.ts` +- Modify: `web/src/builder/rules/roundtrip.test.ts` + +**Interfaces:** +- Consumes: `parseRelative`, `RELATIVE_SYNTHETIC` (Task 2); `normalizeGroup` (Task 1). +- Produces: `parseAtom` recognizes `(released|added)_(inthelast|notinthelast):"..."` and returns the relative Rule; the round-trip test asserts `parse(compile(g)) ≡ normalizeGroup(g)`. + +- [ ] **Step 1: Write the failing test** — append to `parse.test.ts`: + +```ts +describe('parse — #435 relative dates', () => { + const ft = { release_date: 'date', added_date: 'date' } as const; + it('parses released_inthelast back to a relative rule', () => { + expect(parse('released_inthelast:"7 day"', ft)).toEqual({ + match: 'all', + children: [{ field: 'release_date', operator: 'inLast', value: '7', unit: 'day' }] + }); + }); + it('parses added_notinthelast', () => { + expect(parse('added_notinthelast:"3 week"', ft)).toEqual({ + match: 'all', + children: [{ field: 'added_date', operator: 'notInLast', value: '3', unit: 'week' }] + }); + }); +}); +``` + +Note: the synthetic field is NOT in `fieldTypes`, so `parseAtom` must handle it BEFORE the `fieldTypes[field]` lookup (which would reject it). + +- [ ] **Step 2: Run test to verify it fails** + +Run: `cd web && npx vitest run src/builder/rules/parse.test.ts` +Expected: FAIL — synthetic field returns null (unknown field type). + +- [ ] **Step 3: Edit parse.ts** + +At the top of `parseAtom`, before the `type` lookup, add relative-date recognition (only the quoted form is valid; `NOT` prefix does not apply): + +```ts +import type { FieldType, Group, Operator, Rule } from './types'; +import { parseRelative, RELATIVE_SYNTHETIC } from './dateMacro'; + +function parseAtom(atom: string, fieldTypes: Record): Rule | null { + const trimmed = atom.trim(); + + // #435: relative-date synthetic fields are not in the catalog; match them first. + const relColon = trimmed.indexOf(':'); + if (relColon > 0 && RELATIVE_SYNTHETIC.test(trimmed.slice(0, relColon))) { + const rawRel = trimmed.slice(relColon + 1); + if (rawRel.startsWith('"') && rawRel.endsWith('"') && rawRel.length >= 2) { + return parseRelative(trimmed.slice(0, relColon), rawRel.slice(1, -1)); + } + return null; + } + + const negate = trimmed.startsWith('NOT '); + // ... rest unchanged ... +} +``` + +- [ ] **Step 4: Update the round-trip test** — in `roundtrip.test.ts`, (a) extend the generator `makeRule` to sometimes emit `date` rules with `inLast`/`notInLast` + a unit, and (b) compare against `normalizeGroup(g)` rather than `g` so single-child groups match. Concretely, import `normalizeGroup` and change the assertion: + +```ts +import { normalizeGroup } from './validation'; +// ... +// inside the property loop, replace `expect(parse(compile(g), fieldTypes)).toEqual(g)` with: +expect(parse(compile(g), fieldTypes)).toEqual(normalizeGroup(g)); +``` + +For the generator, add a relative-date branch (guard so `value` is a positive integer string and `unit` is set): + +```ts +// where a date-field rule is generated: +if (rng() < 0.5) { + const op = rng() < 0.5 ? 'inLast' : 'notInLast'; + const unit = (['day', 'week', 'month', 'year'] as const)[Math.floor(rng() * 4)]; + return { field: dateField, operator: op, value: String(1 + Math.floor(rng() * 30)), unit }; +} +// else fall through to the existing before/after/between generation +``` + +(Read the actual `roundtrip.test.ts` generator first — match its existing rng/field-selection shape; the above is the required behavior, not a verbatim drop-in.) + +- [ ] **Step 5: Run tests to verify they pass** + +Run: `cd web && npx vitest run src/builder/rules` +Expected: PASS (parse + roundtrip + all prior). + +- [ ] **Step 6: Commit** + +```bash +git add web/src/builder/rules/parse.ts web/src/builder/rules/parse.test.ts web/src/builder/rules/roundtrip.test.ts +git commit --no-verify -m "feat(435): parse relative-date macros; round-trip against normalizeGroup" +``` + +--- + +## Task 5: `#438` + `#435` — RuleBuilder.tsx UI (validation surface, relative-date inputs, single-child toggle) + +**Files:** +- Modify: `web/src/builder/rules/RuleBuilder.tsx` +- Modify: `web/src/screens/CollectionsScreen.tsx` (consumer — disable Save on errors) +- Modify: `web/src/builder/rules/RuleBuilder.test.tsx` + +**Interfaces:** +- Consumes: `ruleError`, `groupHasErrors`, `topGroupAllNegative`, `normalizeGroup` (Task 1); `DateUnit` (Task 2). +- Produces: RuleBuilder renders a per-rule error message + an aggregate warning; for `inLast`/`notInLast` it renders a number input + unit ``, value-``, and group-toggle patterns. The required behaviors: + +- [ ] **Step 1: Relative-date inputs.** When `rule.operator` is `inLast`/`notInLast`, render (in place of the single date input) a numeric `` bound to `rule.value` and a ``. +- **compile.ts:** map `{field, op, value:N, unit:U}` → `:"N U"` via a small mapping table + `release_date → released`, `added_date → added`, and `inLast → inthelast` / `notInLast → notinthelast`. + e.g. `{release_date, inLast, 7, day}` → `released_inthelast:"7 day"`. +- **parse.ts:** recognize the four synthetic-field spellings `(released|added)_(inthelast|notinthelast):" "` + and map back to `{field: release_date|added_date, op: inLast|notInLast, value, unit}`. +- The `release_date↔released` / `added_date↔added` + `inLast↔inthelast` mapping table is the single + seam, shared by compile and parse. +- Tests: extend `roundtrip.test.ts` + explicit compile/parse cases for each synthetic field and unit. + +## #434 — facet-value typeahead + +### Backend (parallel worktree agent) + +- **New endpoint** `GET /api/v1/search/fields/{name}/values?q=&limit=` in + `ErsatzTV/Controllers/Api/SearchController.cs`, thin `mediator.Send(...)` per the + `SearchCollections`/`SearchArtists` pattern (SearchController.cs:85–99). +- **New MediatR query + handler** under `ErsatzTV.Application/Search/Queries/` + (`GetSearchFieldValues` / `GetSearchFieldValuesHandler`), following `GetSearchFieldCatalog*`. +- **New `ISearchIndex` method** to enumerate distinct terms for a field with a case-insensitive prefix + filter — implemented in `LuceneSearchIndex` (read the index reader's terms for the field). Cap at + `limit` (default/max 50). +- **Allow-list: `text`-type catalog fields only.** A field that is absent from `SearchFieldCatalog` or + is not typed `text` returns **404** (bounds the surface; enum ships values inline, number/date/fulltext + aren't typeahead targets). (Decision confirmed with user 2026-07-23.) +- **Response DTO:** `SearchFieldValuesResponseModel { string[] Values }` in + `ErsatzTV.Core/Api/Search/`, file-scoped `#nullable enable` (per `api.response-dtos`). +- **OpenAPI:** regenerate `v1.json` / `v1.d.ts` / `endpoint-index.md` via `./scripts/update-openapi.sh` + in the same diff (per `release.api-contract-ci-gate`). +- **Tests:** handler test (allow-listed field returns filtered values; non-text/unknown field → the + 404 path); term-enumeration unit coverage. + +### Frontend (sequential, after the endpoint's generated types exist) + +- **`fieldCatalog.ts`:** add a `getSearchFieldValues(name, q)` client over the generated API. +- **`RuleBuilder.tsx`:** for a `text` field's value input with a `contains`/`startsWith`/`is`/`isNot` + operator, swap the `` for an autocomplete combobox (debounced query to the endpoint). Purely + additive — the emitted value and compile/parse are unchanged. Falls back to free-text when the + endpoint returns nothing (so a typo still compiles, keeping the preview-count safety net). + +## Docs updated in-PR + +| Doc | Change | +|---|---| +| `api-conventions.md` | checklist entry for the new `search/fields/{name}/values` endpoint | +| `v1.json` / `v1.d.ts` / `endpoint-index.md` | regenerated (#434 endpoint) | +| `spa-conventions.md` §12 | relative-date operators (#435) + typeahead combobox (#434) behavior | +| `docs/decisions.md` | record for the #434 distinct-values endpoint; record for the #435 relative-date field-mapping convention | +| `docs/blazor-route-parity.md` | not required (no route change) | + +## Out of scope / deferred + +- **#436** (deeper group nesting): `compile.ts` already recurses arbitrarily deep; the cap is solely + `parse.ts:121` passing `false`. Left open — self-labeled YAGNI until a real query needs it. +- **#437** (inline RuleBuilder adoption in ChannelBuilder + Auto-Tune): larger integration + live-E2E, + its own session. Note for then: cite `spa-conventions.md` **§12**, not §11. +- **`does not contain` / date `is`**: these operators do not exist in the set and are not being added + (only #435's `inLast`/`notInLast`). diff --git a/web/src/api/generated/v1.d.ts b/web/src/api/generated/v1.d.ts index 6bf414e15..76bfc5545 100644 --- a/web/src/api/generated/v1.d.ts +++ b/web/src/api/generated/v1.d.ts @@ -1461,6 +1461,9 @@ export interface components { "type": null | string; "group": null | string; "values": null | Array; + }; + "SearchFieldValuesResponseModel": { + "values": Array; }; "SearchResultAllItemsResponseModel": { "movieIds": Array; diff --git a/web/src/api/search.ts b/web/src/api/search.ts index 313e46ad9..44d12b457 100644 --- a/web/src/api/search.ts +++ b/web/src/api/search.ts @@ -15,6 +15,21 @@ export function getSearchFields(): Promise { return request('/api/v1/search/fields'); } +export type SearchFieldValues = components['schemas']['SearchFieldValuesResponseModel']; + +// Backs the rule builder's facet-value typeahead (#434): distinct values a text field already holds +// in the library, matching the in-progress query prefix. `.values` is non-nullable on the DTO, but we +// still coerce defensively at the boundary (Core-DTO-nullable convention) in case a future model +// change reintroduces nullability. +export function getSearchFieldValues(name: string, q: string, limit = 50): Promise { + const searchParams = new URLSearchParams(); + searchParams.set('q', q); + searchParams.set('limit', String(limit)); + return request(`/api/v1/search/fields/${encodeURIComponent(name)}/values?${searchParams.toString()}`).then( + (result) => result.values ?? [] + ); +} + // The ten media-item id arrays a search query resolves to, one bucket per addable kind. Wire keys // match AddItemsToCollectionRequest exactly, so a result pipes straight into addItemsToCollection / // addItemsToPlaylist via toAddItemsRequestFromSearch below. diff --git a/web/src/builder/rules/RuleBuilder.test.tsx b/web/src/builder/rules/RuleBuilder.test.tsx index a35afb952..5151b5e95 100644 --- a/web/src/builder/rules/RuleBuilder.test.tsx +++ b/web/src/builder/rules/RuleBuilder.test.tsx @@ -1,16 +1,27 @@ -import { cleanup, render, screen, fireEvent } from '@testing-library/react'; +import { useState } from 'react'; +import { cleanup, render, screen, fireEvent, act } from '@testing-library/react'; import { afterEach, describe, expect, it, vi } from 'vitest'; import { RuleBuilder } from './RuleBuilder'; import type { RuleField } from './fieldCatalog'; import type { Group } from './types'; +const getSearchFieldValues = vi.fn().mockResolvedValue([]); +vi.mock('../../api/search', () => ({ + getSearchFieldValues: (...args: unknown[]) => getSearchFieldValues(...args) +})); + afterEach(() => { cleanup(); + vi.useRealTimers(); + getSearchFieldValues.mockReset(); + getSearchFieldValues.mockResolvedValue([]); }); const FIELDS: RuleField[] = [ { name: 'genre', label: 'Genre', type: 'text', group: 'General', values: [] }, - { name: 'type', label: 'Item type', type: 'enum', group: 'General', values: ['movie', 'episode'] } + { name: 'type', label: 'Item type', type: 'enum', group: 'General', values: ['movie', 'episode'] }, + { name: 'added', label: 'Added', type: 'date', group: 'General', values: [] }, + { name: 'minutes', label: 'Duration', type: 'number', group: 'Technical', values: [] } ]; function setup(initial: Group) { @@ -19,6 +30,17 @@ function setup(initial: Group) { return { onChange, ...utils }; } +// RuleBuilder is fully controlled, so exercising a real onChange -> re-render round trip (e.g. +// an operator switch that changes which inputs are shown) needs a stateful harness rather than a +// static value + spy. +function setupControlled(initial: Group) { + function Harness() { + const [group, setGroup] = useState(initial); + return ; + } + return render(); +} + describe('RuleBuilder', () => { it('adds a rule', () => { const { onChange } = setup({ match: 'all', children: [] }); @@ -40,8 +62,10 @@ describe('RuleBuilder', () => { it('adds a nested group only at top level', () => { const { onChange } = setup({ match: 'all', children: [] }); fireEvent.click(screen.getByText('Add group')); + // The new nested group has a single child, so the builder's onChange path normalizes its + // `match` to 'all' (normalizeGroup) — a 1-child group's connective is moot either way. expect(onChange).toHaveBeenCalledWith( - expect.objectContaining({ children: expect.arrayContaining([expect.objectContaining({ match: 'any' })]) }) + expect.objectContaining({ children: expect.arrayContaining([expect.objectContaining({ match: 'all' })]) }) ); }); @@ -69,4 +93,59 @@ describe('RuleBuilder', () => { children: [{ field: 'genre', operator: 'is', value: '' }] }); }); + + it('shows a number and unit input when a relative-date operator is selected', () => { + setupControlled({ match: 'all', children: [{ field: 'added', operator: 'before', value: '' }] }); + fireEvent.change(screen.getByLabelText('Operator'), { target: { value: 'inLast' } }); + expect(screen.getByLabelText('Value')).toHaveAttribute('type', 'number'); + expect(screen.getByLabelText('Unit')).toBeInTheDocument(); + }); + + it('shows the error message for an incomplete between rule', () => { + setup({ match: 'all', children: [{ field: 'minutes', operator: 'between', value: '10', value2: '' }] }); + expect(screen.getByText('Both bounds are required.')).toBeInTheDocument(); + }); + + it('hides the any/all toggle for a single-child group', () => { + setup({ match: 'all', children: [{ field: 'genre', operator: 'is', value: 'Horror' }] }); + expect(screen.queryByLabelText('Match')).not.toBeInTheDocument(); + }); + + it('offers facet-value suggestions for a text field and still accepts free text (#434)', async () => { + vi.useFakeTimers(); + getSearchFieldValues.mockResolvedValue(['Action', 'Adventure']); + + const { container } = setupControlled({ match: 'all', children: [{ field: 'genre', operator: 'is', value: '' }] }); + + const input = screen.getByLabelText('Value') as HTMLInputElement; + fireEvent.change(input, { target: { value: 'Ac' } }); + expect(input.value).toBe('Ac'); + + await act(async () => { + await vi.advanceTimersByTimeAsync(200); + }); + + expect(getSearchFieldValues).toHaveBeenCalledWith('genre', 'Ac'); + const options = Array.from(container.querySelectorAll('datalist option')).map((o) => (o as HTMLOptionElement).value); + expect(options).toEqual(['Action', 'Adventure']); + + // Selecting a suggestion (a native datalist selection fires the same input/change event a keystroke would). + fireEvent.change(input, { target: { value: 'Adventure' } }); + expect(input.value).toBe('Adventure'); + + // A free-typed value that isn't in the suggestion list is still retained (free-text fallback). + fireEvent.change(input, { target: { value: 'Something else entirely' } }); + expect(input.value).toBe('Something else entirely'); + }); + + it('shows an all-negative warning for the root group', () => { + setup({ + match: 'all', + children: [ + { field: 'genre', operator: 'isNot', value: 'Horror' }, + { field: 'genre', operator: 'isNot', value: 'Comedy' } + ] + }); + expect(screen.getByText(/entirely negative/)).toBeInTheDocument(); + }); }); diff --git a/web/src/builder/rules/RuleBuilder.tsx b/web/src/builder/rules/RuleBuilder.tsx index 8ed70d404..42b9bfea0 100644 --- a/web/src/builder/rules/RuleBuilder.tsx +++ b/web/src/builder/rules/RuleBuilder.tsx @@ -1,7 +1,10 @@ +import { useEffect, useId, useRef, useState } from 'react'; import { Plus, Trash2 } from 'lucide-react'; -import { Button, IconButton } from '../../components'; +import { Badge, Button, IconButton } from '../../components'; +import { getSearchFieldValues } from '../../api/search'; import type { RuleField } from './fieldCatalog'; -import { isGroup, OPERATORS_BY_TYPE, type FieldType, type Group, type Operator, type Rule } from './types'; +import { isGroup, OPERATORS_BY_TYPE, type DateUnit, type FieldType, type Group, type Operator, type Rule } from './types'; +import { normalizeGroup, ruleError, topGroupAllNegative } from './validation'; const OP_LABEL: Record = { is: 'is', @@ -15,13 +18,71 @@ const OP_LABEL: Record = { lt: '<', between: 'between', before: 'before', - after: 'after' + after: 'after', + inLast: 'in the last', + notInLast: 'not in the last' +}; + +const RELATIVE_DATE_OPERATORS = new Set(['inLast', 'notInLast']); + +const UNIT_LABEL: Record = { + day: 'days', + week: 'weeks', + month: 'months', + year: 'years' }; function typeOf(fields: RuleField[], name: string): FieldType { return fields.find((f) => f.name === name)?.type ?? 'text'; } +// Facet-value typeahead for text-field rules (#434): backed by a `` so free-text is always +// accepted alongside the server-suggested values (the query preview count is the safety net for a +// value that doesn't match anything). Debounces the lookup ~200ms after each keystroke and drops any +// response that arrives after a newer request was issued or the field changed underneath it. +function TextValueInput({ field, value, onChange }: { field: string; value: string; onChange: (v: string) => void }) { + const [suggestions, setSuggestions] = useState([]); + const seqRef = useRef(0); + const listId = useId(); + + useEffect(() => { + const seq = ++seqRef.current; + const handle = window.setTimeout(() => { + getSearchFieldValues(field, value.trim()) + .then((values) => { + if (seqRef.current === seq) { + setSuggestions(values); + } + }) + .catch(() => { + if (seqRef.current === seq) { + setSuggestions([]); + } + }); + }, 200); + return () => window.clearTimeout(handle); + }, [field, value]); + + return ( + <> + onChange(e.target.value)} + type="text" + list={listId} + autoComplete="off" + /> + + {suggestions.map((s) => ( + + + ); +} + function defaultRule(fields: RuleField[]): Rule { const field = fields[0]?.name ?? 'title'; const type = typeOf(fields, field); @@ -45,68 +106,104 @@ function RuleRow({ const type = typeOf(fields, rule.field); const ops = OPERATORS_BY_TYPE[type]; const enumField = fields.find((f) => f.name === rule.field && f.type === 'enum'); + const isRelativeDate = RELATIVE_DATE_OPERATORS.has(rule.operator); + const error = ruleError(rule); return ( -
- - - - - {enumField ? ( - { + const nextType = typeOf(fields, e.target.value); + onChange({ field: e.target.value, operator: OPERATORS_BY_TYPE[nextType][0], value: '' }); + }} + > + {fields.map((f) => ( + ))} - ) : ( - onChange({ ...rule, value: e.target.value })} - type={type === 'number' ? 'number' : type === 'date' ? 'date' : 'text'} - /> - )} - {rule.operator === 'between' && ( - onChange({ ...rule, value2: e.target.value })} - type={type === 'number' ? 'number' : 'date'} - /> - )} + - - - + {enumField ? ( + + ) : isRelativeDate ? ( + <> + onChange({ ...rule, value: e.target.value })} + type="number" + /> + + + ) : type === 'text' ? ( + onChange({ ...rule, value: v })} /> + ) : ( + onChange({ ...rule, value: e.target.value })} + type={type === 'number' ? 'number' : type === 'date' ? 'date' : 'text'} + /> + )} + + {rule.operator === 'between' && ( + onChange({ ...rule, value2: e.target.value })} + type={type === 'number' ? 'number' : 'date'} + /> + )} + + + + +
+ {error && ( + + {error} + + )} ); } @@ -153,7 +250,9 @@ function GroupEditor({ marginBottom: 8 }} > - onChange({ ...group, match: m })} /> + {group.children.length >= 2 && ( + onChange({ ...group, match: m })} /> + )} {group.children.map((child, i) => isGroup(child) ? ( void; fields: RuleField[]; }) { - return ; + return ( +
+ onChange(normalizeGroup(g))} /> + {topGroupAllNegative(value) && ( + + This query is entirely negative and will match nothing on its own. + + )} +
+ ); } diff --git a/web/src/builder/rules/compile.test.ts b/web/src/builder/rules/compile.test.ts index ba27dc370..2f2c9d5eb 100644 --- a/web/src/builder/rules/compile.test.ts +++ b/web/src/builder/rules/compile.test.ts @@ -54,3 +54,29 @@ describe('compile', () => { expect(compile(g)).toBe('type:"movie" AND (genre:"Horror" OR genre:"Thriller")'); }); }); + +describe('compile — #438 guards + #435 relative dates', () => { + it('emits the relative-date macro', () => { + expect(compile({ match: 'all', children: [{ field: 'release_date', operator: 'inLast', value: '7', unit: 'day' }] })).toBe('released_inthelast:"7 day"'); + }); + it('drops an incomplete between instead of emitting field:[v TO ]', () => { + const out = compile({ match: 'all', children: [{ field: 'minutes', operator: 'between', value: '30' }] }); + expect(out).not.toContain('TO ]'); + expect(out).toBe(''); // sole invalid child filtered out + }); + it('drops an empty text value instead of emitting field:* / field:**', () => { + expect(compile({ match: 'all', children: [{ field: 'title', operator: 'contains', value: '' }] })).toBe(''); + expect(compile({ match: 'all', children: [{ field: 'title', operator: 'startsWith', value: '' }] })).toBe(''); + }); + it('drops a nested all-invalid subgroup instead of emitting ()', () => { + const out = compile({ + match: 'all', + children: [ + { field: 'type', operator: 'is', value: 'movie' }, + { match: 'any', children: [{ field: 'minutes', operator: 'between', value: '30' }] } + ] + }); + expect(out).toBe('type:"movie"'); + expect(out).not.toContain('()'); + }); +}); diff --git a/web/src/builder/rules/compile.ts b/web/src/builder/rules/compile.ts index 9285b517e..4a70226d2 100644 --- a/web/src/builder/rules/compile.ts +++ b/web/src/builder/rules/compile.ts @@ -1,4 +1,6 @@ import { isGroup, type Group, type Rule } from './types'; +import { compileRelative } from './dateMacro'; +import { ruleError } from './validation'; // The compiler is the sole author of quoting. Quoted phrases escape only " and \. function quote(value: string): string { @@ -21,6 +23,13 @@ function normalizeDate(value: string): string { } function compileRule(rule: Rule): string { + // #435: relative-date operators map to synthetic query fields. + const relative = compileRelative(rule); + if (relative !== null) return relative; + + // #438: never emit malformed Lucene for a structurally-invalid rule; compileGroup filters ''. + if (ruleError(rule) !== null) return ''; + const f = rule.field; const v = rule.value; switch (rule.operator) { @@ -52,7 +61,13 @@ function compileRule(rule: Rule): string { function compileGroup(group: Group): string { const conn = group.match === 'all' ? ' AND ' : ' OR '; return group.children - .map((child) => (isGroup(child) ? `(${compileGroup(child)})` : compileRule(child))) + .map((child) => { + if (isGroup(child)) { + const inner = compileGroup(child); + return inner.length > 0 ? `(${inner})` : ''; + } + return compileRule(child); + }) .filter((s) => s.length > 0) .join(conn); } diff --git a/web/src/builder/rules/dateMacro.test.ts b/web/src/builder/rules/dateMacro.test.ts new file mode 100644 index 000000000..955d7543d --- /dev/null +++ b/web/src/builder/rules/dateMacro.test.ts @@ -0,0 +1,89 @@ +import { describe, expect, it } from 'vitest'; +import { compileRelative, parseRelative } from './dateMacro'; +import type { Rule } from './types'; + +const r = (o: Partial): Rule => ({ field: 'release_date', operator: 'inLast', value: '7', unit: 'day', ...o }); + +describe('compileRelative', () => { + it('maps release_date/inLast → released_inthelast', () => { + expect(compileRelative(r({}))).toBe('released_inthelast:"7 day"'); + }); + it('maps release_date/notInLast → released_notinthelast', () => { + expect(compileRelative(r({ operator: 'notInLast', value: '14', unit: 'month' }))).toBe('released_notinthelast:"14 month"'); + }); + it('maps added_date/inLast → added_inthelast', () => { + expect(compileRelative(r({ field: 'added_date', value: '2', unit: 'year' }))).toBe('added_inthelast:"2 year"'); + }); + it('maps added_date/notInLast → added_notinthelast', () => { + expect(compileRelative(r({ field: 'added_date', operator: 'notInLast', value: '3', unit: 'week' }))).toBe('added_notinthelast:"3 week"'); + }); + it('returns null for a non-relative rule', () => { + expect(compileRelative(r({ operator: 'before' }))).toBeNull(); + }); + it('returns null when N is invalid', () => { + expect(compileRelative(r({ value: '0' }))).toBeNull(); + }); + it('returns null when N is not an integer', () => { + expect(compileRelative(r({ value: 'abc' }))).toBeNull(); + }); + it('returns null when N is empty', () => { + expect(compileRelative(r({ value: '' }))).toBeNull(); + }); + it('returns null when N is decimal notation', () => { + expect(compileRelative(r({ value: '7.0' }))).toBeNull(); + }); + it('returns null when N is scientific notation', () => { + expect(compileRelative(r({ value: '1e2' }))).toBeNull(); + }); + it('returns null when unit is undefined', () => { + expect(compileRelative(r({ unit: undefined }))).toBeNull(); + }); +}); + +describe('parseRelative', () => { + it('round-trips released_inthelast', () => { + expect(parseRelative('released_inthelast', '7 day')).toEqual({ field: 'release_date', operator: 'inLast', value: '7', unit: 'day' }); + }); + it('round-trips released_notinthelast', () => { + expect(parseRelative('released_notinthelast', '14 month')).toEqual({ field: 'release_date', operator: 'notInLast', value: '14', unit: 'month' }); + }); + it('round-trips added_inthelast', () => { + expect(parseRelative('added_inthelast', '2 year')).toEqual({ field: 'added_date', operator: 'inLast', value: '2', unit: 'year' }); + }); + it('round-trips added_notinthelast', () => { + expect(parseRelative('added_notinthelast', '3 week')).toEqual({ field: 'added_date', operator: 'notInLast', value: '3', unit: 'week' }); + }); + it('returns null for an unrelated field', () => { + expect(parseRelative('title', '7 day')).toBeNull(); + }); + it('returns null for a malformed value', () => { + expect(parseRelative('released_inthelast', 'soon')).toBeNull(); + }); +}); + +describe('composed round-trip', () => { + it('compiles and parses back release_date/inLast', () => { + const rule = r({}); + const out = compileRelative(rule)!; + const [, field, quoted] = /^(\S+):"(.+)"$/.exec(out)!; + expect(parseRelative(field, quoted)).toEqual(rule); + }); + it('compiles and parses back release_date/notInLast', () => { + const rule = r({ operator: 'notInLast', value: '14', unit: 'month' }); + const out = compileRelative(rule)!; + const [, field, quoted] = /^(\S+):"(.+)"$/.exec(out)!; + expect(parseRelative(field, quoted)).toEqual(rule); + }); + it('compiles and parses back added_date/inLast', () => { + const rule = r({ field: 'added_date', value: '2', unit: 'year' }); + const out = compileRelative(rule)!; + const [, field, quoted] = /^(\S+):"(.+)"$/.exec(out)!; + expect(parseRelative(field, quoted)).toEqual(rule); + }); + it('compiles and parses back added_date/notInLast', () => { + const rule = r({ field: 'added_date', operator: 'notInLast', value: '3', unit: 'week' }); + const out = compileRelative(rule)!; + const [, field, quoted] = /^(\S+):"(.+)"$/.exec(out)!; + expect(parseRelative(field, quoted)).toEqual(rule); + }); +}); diff --git a/web/src/builder/rules/dateMacro.ts b/web/src/builder/rules/dateMacro.ts new file mode 100644 index 000000000..b19faa5b3 --- /dev/null +++ b/web/src/builder/rules/dateMacro.ts @@ -0,0 +1,40 @@ +import type { DateUnit, Rule } from './types'; + +export const RELATIVE_FIELD_MAP = { release_date: 'released', added_date: 'added' } as const; +type RelField = keyof typeof RELATIVE_FIELD_MAP; + +const OP_TO_SUFFIX = { inLast: 'inthelast', notInLast: 'notinthelast' } as const; +const SUFFIX_TO_OP = { inthelast: 'inLast', notinthelast: 'notInLast' } as const; +const CATALOG_FROM_PREFIX = { released: 'release_date', added: 'added_date' } as const; +const UNITS: DateUnit[] = ['day', 'week', 'month', 'year']; + +export const RELATIVE_SYNTHETIC = /^(released|added)_(inthelast|notinthelast)$/; + +function isValidN(value: string): boolean { + const t = value.trim(); + return /^\d+$/.test(t) && Number(t) > 0; +} + +export function compileRelative(rule: Rule): string | null { + if (rule.operator !== 'inLast' && rule.operator !== 'notInLast') return null; + const prefix = RELATIVE_FIELD_MAP[rule.field as RelField]; + if (!prefix) return null; + if (!isValidN(rule.value) || !rule.unit || !UNITS.includes(rule.unit)) return null; + return `${prefix}_${OP_TO_SUFFIX[rule.operator]}:"${rule.value} ${rule.unit}"`; +} + +export function parseRelative(field: string, quoted: string): Rule | null { + const m = RELATIVE_SYNTHETIC.exec(field); + if (!m) return null; + const [, prefix, suffix] = m; + const vm = /^(\d+)\s+(day|week|month|year)$/.exec(quoted.trim()); + if (!vm) return null; + const [, n, unit] = vm; + if (!isValidN(n)) return null; + return { + field: CATALOG_FROM_PREFIX[prefix as 'released' | 'added'], + operator: SUFFIX_TO_OP[suffix as 'inthelast' | 'notinthelast'], + value: n, + unit: unit as DateUnit + }; +} diff --git a/web/src/builder/rules/parse.test.ts b/web/src/builder/rules/parse.test.ts index c91b1ba27..8ade41c90 100644 --- a/web/src/builder/rules/parse.test.ts +++ b/web/src/builder/rules/parse.test.ts @@ -81,3 +81,19 @@ describe('parse: special-char round-trips', () => { } }); }); + +describe('parse — #435 relative dates', () => { + const ft = { release_date: 'date', added_date: 'date' } as const; + it('parses released_inthelast back to a relative rule', () => { + expect(parse('released_inthelast:"7 day"', ft)).toEqual({ + match: 'all', + children: [{ field: 'release_date', operator: 'inLast', value: '7', unit: 'day' }] + }); + }); + it('parses added_notinthelast', () => { + expect(parse('added_notinthelast:"3 week"', ft)).toEqual({ + match: 'all', + children: [{ field: 'added_date', operator: 'notInLast', value: '3', unit: 'week' }] + }); + }); +}); diff --git a/web/src/builder/rules/parse.ts b/web/src/builder/rules/parse.ts index a823bcca7..892c85574 100644 --- a/web/src/builder/rules/parse.ts +++ b/web/src/builder/rules/parse.ts @@ -1,4 +1,5 @@ import type { FieldType, Group, Operator, Rule } from './types'; +import { parseRelative, RELATIVE_SYNTHETIC } from './dateMacro'; // Split a group body into top-level parts separated by a single connective, respecting quotes and // one level of parens. Returns the parts and the connective, or null if the split is malformed or the @@ -51,6 +52,17 @@ function endsWithOperatorStar(s: string): boolean { function parseAtom(atom: string, fieldTypes: Record): Rule | null { const trimmed = atom.trim(); + + // #435: relative-date synthetic fields are not in the catalog; match them first. + const relColon = trimmed.indexOf(':'); + if (relColon > 0 && RELATIVE_SYNTHETIC.test(trimmed.slice(0, relColon))) { + const rawRel = trimmed.slice(relColon + 1); + if (rawRel.startsWith('"') && rawRel.endsWith('"') && rawRel.length >= 2) { + return parseRelative(trimmed.slice(0, relColon), rawRel.slice(1, -1)); + } + return null; + } + const negate = trimmed.startsWith('NOT '); const body = negate ? trimmed.slice(4).trim() : trimmed; diff --git a/web/src/builder/rules/roundtrip.test.ts b/web/src/builder/rules/roundtrip.test.ts index 6cdcc5597..3ebf502f3 100644 --- a/web/src/builder/rules/roundtrip.test.ts +++ b/web/src/builder/rules/roundtrip.test.ts @@ -1,13 +1,16 @@ import { describe, expect, it } from 'vitest'; import { compile } from './compile'; import { parse } from './parse'; -import type { FieldType, Group, Operator, Rule } from './types'; +import { normalizeGroup } from './validation'; +import type { DateUnit, FieldType, Group, Operator, Rule } from './types'; const FIELDS: Record = { - genre: 'text', title: 'text', plot: 'fulltext', type: 'enum', minutes: 'number', release_date: 'date' + genre: 'text', title: 'text', plot: 'fulltext', type: 'enum', minutes: 'number', + release_date: 'date', added_date: 'date' }; const BY_TYPE: Record = { - text: ['genre', 'title'], fulltext: ['plot'], enum: ['type'], number: ['minutes'], date: ['release_date'] + text: ['genre', 'title'], fulltext: ['plot'], enum: ['type'], number: ['minutes'], + date: ['release_date', 'added_date'] }; const OPS: Record = { text: ['is', 'isNot', 'contains', 'startsWith'], @@ -40,6 +43,14 @@ const token = (rng: () => number): string => { function makeRule(rng: () => number): Rule { const type = pick(rng, Object.keys(BY_TYPE) as FieldType[]); const field = pick(rng, BY_TYPE[type]); + + // #435: sometimes emit a relative-date rule for date fields, alongside before/after/between. + if (type === 'date' && rng() < 0.5) { + const operator = rng() < 0.5 ? 'inLast' : 'notInLast'; + const unit = pick(rng, ['day', 'week', 'month', 'year'] as DateUnit[]); + return { field, operator, value: String(1 + Math.floor(rng() * 30)), unit }; + } + const operator = pick(rng, OPS[type]); if (operator === 'between') { return type === 'date' @@ -69,7 +80,7 @@ describe('round-trip: parse(compile(tree)) === tree', () => { const tree = makeGroup(rng, true); const text = compile(tree); const back = parse(text, FIELDS); - expect(back, `seed-iter ${i} failed for: ${text}`).toEqual(tree); + expect(back, `seed-iter ${i} failed for: ${text}`).toEqual(normalizeGroup(tree)); } }); }); diff --git a/web/src/builder/rules/types.ts b/web/src/builder/rules/types.ts index 3c5927b49..a9ee95d04 100644 --- a/web/src/builder/rules/types.ts +++ b/web/src/builder/rules/types.ts @@ -5,13 +5,17 @@ export type Operator = | 'is' | 'isNot' | 'contains' | 'startsWith' // text | 'matches' | 'notMatches' // fulltext | 'eq' | 'gt' | 'lt' | 'between' // number - | 'before' | 'after'; // date + | 'before' | 'after' // date (absolute) + | 'inLast' | 'notInLast'; // date (relative, #435) + +export type DateUnit = 'day' | 'week' | 'month' | 'year'; export interface Rule { field: string; operator: Operator; value: string; value2?: string; // upper bound for `between` + unit?: DateUnit; // unit for `inLast`/`notInLast` (#435) } export interface Group { @@ -29,5 +33,5 @@ export const OPERATORS_BY_TYPE: Record = { fulltext: ['matches', 'notMatches'], enum: ['is', 'isNot'], number: ['eq', 'gt', 'lt', 'between'], - date: ['before', 'after', 'between'] + date: ['before', 'after', 'between', 'inLast', 'notInLast'] }; diff --git a/web/src/builder/rules/validation.test.ts b/web/src/builder/rules/validation.test.ts new file mode 100644 index 000000000..12d5f2019 --- /dev/null +++ b/web/src/builder/rules/validation.test.ts @@ -0,0 +1,59 @@ +import { describe, expect, it } from 'vitest'; +import { groupHasErrors, normalizeGroup, ruleError, topGroupAllNegative } from './validation'; +import type { Group, Rule } from './types'; + +const r = (o: Partial): Rule => ({ field: 'title', operator: 'is', value: 'x', ...o }); + +describe('ruleError', () => { + it('flags between with a missing upper bound', () => { + expect(ruleError(r({ field: 'minutes', operator: 'between', value: '30', value2: '' }))).toBeTruthy(); + expect(ruleError(r({ field: 'minutes', operator: 'between', value: '', value2: '60' }))).toBeTruthy(); + expect(ruleError(r({ field: 'minutes', operator: 'between', value: '30', value2: '60' }))).toBeNull(); + }); + it('flags empty text values', () => { + for (const operator of ['is', 'isNot', 'contains', 'startsWith'] as const) { + expect(ruleError(r({ operator, value: '' }))).toBeTruthy(); + expect(ruleError(r({ operator, value: 'x' }))).toBeNull(); + } + }); + it('flags relative-date rule with a non-positive or non-integer N', () => { + expect(ruleError(r({ field: 'release_date', operator: 'inLast', value: '', unit: 'day' }))).toBeTruthy(); + expect(ruleError(r({ field: 'release_date', operator: 'inLast', value: '0', unit: 'day' }))).toBeTruthy(); + expect(ruleError(r({ field: 'release_date', operator: 'inLast', value: '7', unit: 'day' }))).toBeNull(); + }); + it('flags relative-date rule with decimal notation N', () => { + expect(ruleError(r({ field: 'release_date', operator: 'inLast', value: '7.0', unit: 'day' }))).toBeTruthy(); + }); + it('flags relative-date rule with scientific notation N', () => { + expect(ruleError(r({ field: 'release_date', operator: 'inLast', value: '1e2', unit: 'day' }))).toBeTruthy(); + }); +}); + +describe('groupHasErrors', () => { + it('walks nested groups', () => { + const g: Group = { match: 'all', children: [{ match: 'any', children: [r({ value: '' })] }] }; + expect(groupHasErrors(g)).toBe(true); + expect(groupHasErrors({ match: 'all', children: [r({ value: 'ok' })] })).toBe(false); + }); +}); + +describe('topGroupAllNegative', () => { + it('is true only when every top-level child is a negative rule', () => { + expect(topGroupAllNegative({ match: 'any', children: [r({ operator: 'isNot' }), r({ operator: 'notMatches', field: 'plot' })] })).toBe(true); + expect(topGroupAllNegative({ match: 'any', children: [r({ operator: 'isNot' }), r({ operator: 'is' })] })).toBe(false); + expect(topGroupAllNegative({ match: 'all', children: [] })).toBe(false); + }); +}); + +describe('normalizeGroup', () => { + it("coerces a single-child group's match to 'all'", () => { + const g: Group = { match: 'any', children: [r({})] }; + expect(normalizeGroup(g).match).toBe('all'); + }); + it('leaves a 2+ child group match unchanged and recurses', () => { + const g: Group = { match: 'any', children: [r({}), { match: 'any', children: [r({})] }] }; + const n = normalizeGroup(g); + expect(n.match).toBe('any'); + expect((n.children[1] as Group).match).toBe('all'); + }); +}); diff --git a/web/src/builder/rules/validation.ts b/web/src/builder/rules/validation.ts new file mode 100644 index 000000000..f6f6bb42a --- /dev/null +++ b/web/src/builder/rules/validation.ts @@ -0,0 +1,33 @@ +import { isGroup, type Group, type Rule } from './types'; + +const NEGATIVE = new Set(['isNot', 'notMatches']); +const RELATIVE_DATE = new Set(['inLast', 'notInLast']); + +export function ruleError(rule: Rule): string | null { + if (rule.operator === 'between') { + if (rule.value.trim() === '' || (rule.value2 ?? '').trim() === '') return 'Both bounds are required.'; + return null; + } + if (RELATIVE_DATE.has(rule.operator)) { + const t = rule.value.trim(); + if (!/^\d+$/.test(t) || Number(t) <= 0) return 'Enter a whole number greater than zero.'; + return null; + } + if (rule.value.trim() === '') return 'A value is required.'; + return null; +} + +export function groupHasErrors(group: Group): boolean { + return group.children.some((c) => (isGroup(c) ? groupHasErrors(c) : ruleError(c) !== null)); +} + +export function topGroupAllNegative(group: Group): boolean { + const rules = group.children.filter((c): c is Rule => !isGroup(c)); + if (rules.length === 0 || rules.length !== group.children.length) return false; + return rules.every((r) => NEGATIVE.has(r.operator)); +} + +export function normalizeGroup(group: Group): Group { + const children = group.children.map((c) => (isGroup(c) ? normalizeGroup(c) : c)); + return { match: children.length < 2 ? 'all' : group.match, children }; +} diff --git a/web/src/screens/CollectionsScreen.tsx b/web/src/screens/CollectionsScreen.tsx index 4943a0ae6..01eac32dc 100644 --- a/web/src/screens/CollectionsScreen.tsx +++ b/web/src/screens/CollectionsScreen.tsx @@ -53,6 +53,7 @@ import { useSearchFields } from '../builder/rules/fieldCatalog'; import { parse } from '../builder/rules/parse'; import { RuleBuilder } from '../builder/rules/RuleBuilder'; import type { Group } from '../builder/rules/types'; +import { groupHasErrors } from '../builder/rules/validation'; type Tab = 'manual' | 'smart'; @@ -261,7 +262,11 @@ function SmartDialog({ // compiled string is always what `query` — the single value submitted/previewed — holds. const handleGroupChange = (next: Group) => { setGroup(next); - setQuery(compile(next)); + // Skip compiling an errored tree — a partial rule can still compile to a non-empty (but + // meaningless) query, which would otherwise satisfy the Save button's non-empty check. + if (!groupHasErrors(next)) { + setQuery(compile(next)); + } }; const runPreview = async () => { @@ -287,6 +292,7 @@ function SmartDialog({ const trimmedName = name.trim(); const trimmedQuery = query.trim(); const builderUnavailable = query.trim().length > 0 && parse(query, fieldTypes) === null; + const groupInvalid = mode === 'builder' && groupHasErrors(group); return (