Global "Search Anything" typeahead dropdown in navbar #91

Merged
REDCODE merged 52 commits from feature/global-search-dropdown into develop 2026-07-13 15:18:42 +00:00
Collaborator

Implements #90 — wires up the "Search anything..." navbar input with a live typeahead dropdown that searches cosplayers and albums.

New Feature: SearchDropdown

  • MilkStream.Client/Components/Shared/SearchDropdown.razor — typeahead component:
    • Live search as you type (version counter discards stale responses, no debounce)
    • Parallel API calls to GET /api/person and GET /api/album
    • Flat item list for unified keyboard + mouse navigation
    • Selectable section headers with > chevron indicating they navigate to the full list page
    • Keyboard: Arrow Up/Down cycles items, Enter selects, Escape closes
    • Blur delay (150ms) prevents dropdown from closing before click registers
    • Loading ("Searching...") and empty ("No results found") states
    • Circular thumbnails for cosplayers, rounded squares for albums
    • Fallback avatars (initial letter) for cosplayers without profile pic
    • Bootstrap .dropdown-menu / .dropdown-item styling (matching the user profile dropdown)
  • SearchDropdown.razor.css — positioning, thumbnail sizing, clickable header styles

Query Param Pre-fill on List Pages

  • Cosplayers.razor / Albums.razor — parse ?search= query parameter on page load
  • SearchDropdown headers navigate to /cosplayers?search={term} / /albums?search={term}
  • Clicking a header or pressing Enter with it selected navigates to the filtered list page

Backend Fixes

Authentication & Authorization

  • PersonController.GetAll — return 401 when GetUserData returns null (was crashing with NRE)
  • PersonController.GetAll / AlbumController.Search — allow anonymous access (previously required auth)
  • UserController.GetAll — added [Authorize(Roles = "Admin")] (previously any authenticated user could list all users)
  • PersonRepository.SearchQuery / AlbumRepository.SearchQuery — use EF.Functions.ILike() for case-insensitive search (was using Contains() which is case-sensitive on PostgreSQL)
  • PersonRepository.SearchQuery — materialize query before client-side visibility filter (nested SharedWith.Any() could not be translated to SQL by EF Core)

Pagination Consistency

  • AssetController.GetAll — accept zero-based page numbers (was enforcing 1-based, inconsistent with Person/Album controllers)

Controller Bugs

  • TagController.GetAll — added missing [FromQuery] on PagedSearchParametersDto param (was returning 415)

Test File Restructure

  • Lactose/WepApiTest.http — fully rewritten with three clearly separated users:
    • admin (seeded, admin_token): full access
    • curator (created, curator_token): elevated access
    • user (registered, user_token): own data only
  • 66 numbered tests in 10 organized sections: Auth → User CRUD → Person → Album → Tag → Asset → Stats → Settings → Cleanup
  • Tests verify authorization at every level (anonymous, user, curator, admin)
  • Self-contained: all test data created at runtime and cleaned up

Files Changed (14 files)

File Change
MilkStream.Client/Components/Shared/SearchDropdown.razor New — typeahead component
MilkStream.Client/Components/Shared/SearchDropdown.razor.css New — dropdown styles
MilkStream.Client/Components/Layout/NavMenu.razor Replace static search markup with <SearchDropdown />
MilkStream.Client/Components/Pages/Cosplayers.razor Parse ?search= query param
MilkStream.Client/Components/Pages/Albums.razor Parse ?search= query param
Lactose/Controllers/PersonController.cs AllowAnonymous, null uid fix
Lactose/Controllers/AlbumController.cs AllowAnonymous on search
Lactose/Controllers/UserController.cs Admin-only user list
Lactose/Controllers/AssetController.cs Zero-based page numbering
Lactose/Controllers/TagController.cs Add missing [FromQuery]
Lactose/Repositories/PersonRepository.cs ILike search, materialize before client-side filter
Lactose/Repositories/AlbumRepository.cs ILike search
Lactose/WepApiTest.http Complete restructure
Lactose/http-client.env.json Updated variable names
Implements #90 — wires up the "Search anything..." navbar input with a live typeahead dropdown that searches cosplayers and albums. ## New Feature: SearchDropdown - **`MilkStream.Client/Components/Shared/SearchDropdown.razor`** — typeahead component: - Live search as you type (version counter discards stale responses, no debounce) - Parallel API calls to `GET /api/person` and `GET /api/album` - Flat item list for unified keyboard + mouse navigation - Selectable section headers with `>` chevron indicating they navigate to the full list page - Keyboard: Arrow Up/Down cycles items, Enter selects, Escape closes - Blur delay (150ms) prevents dropdown from closing before click registers - Loading ("Searching...") and empty ("No results found") states - Circular thumbnails for cosplayers, rounded squares for albums - Fallback avatars (initial letter) for cosplayers without profile pic - Bootstrap `.dropdown-menu` / `.dropdown-item` styling (matching the user profile dropdown) - **`SearchDropdown.razor.css`** — positioning, thumbnail sizing, clickable header styles ## Query Param Pre-fill on List Pages - **`Cosplayers.razor`** / **`Albums.razor`** — parse `?search=` query parameter on page load - **SearchDropdown headers** navigate to `/cosplayers?search={term}` / `/albums?search={term}` - Clicking a header or pressing Enter with it selected navigates to the filtered list page ## Backend Fixes ### Authentication & Authorization - **`PersonController.GetAll`** — return 401 when `GetUserData` returns null (was crashing with NRE) - **`PersonController.GetAll`** / **`AlbumController.Search`** — allow anonymous access (previously required auth) - **`UserController.GetAll`** — added `[Authorize(Roles = "Admin")]` (previously any authenticated user could list all users) ### Search - **`PersonRepository.SearchQuery`** / **`AlbumRepository.SearchQuery`** — use `EF.Functions.ILike()` for case-insensitive search (was using `Contains()` which is case-sensitive on PostgreSQL) - **`PersonRepository.SearchQuery`** — materialize query before client-side visibility filter (nested `SharedWith.Any()` could not be translated to SQL by EF Core) ### Pagination Consistency - **`AssetController.GetAll`** — accept zero-based page numbers (was enforcing 1-based, inconsistent with Person/Album controllers) ### Controller Bugs - **`TagController.GetAll`** — added missing `[FromQuery]` on `PagedSearchParametersDto` param (was returning 415) ## Test File Restructure - **`Lactose/WepApiTest.http`** — fully rewritten with three clearly separated users: - **admin** (seeded, `admin_token`): full access - **curator** (created, `curator_token`): elevated access - **user** (registered, `user_token`): own data only - 66 numbered tests in 10 organized sections: Auth → User CRUD → Person → Album → Tag → Asset → Stats → Settings → Cleanup - Tests verify authorization at every level (anonymous, user, curator, admin) - Self-contained: all test data created at runtime and cleaned up ## Files Changed (14 files) | File | Change | |---|---| | `MilkStream.Client/Components/Shared/SearchDropdown.razor` | **New** — typeahead component | | `MilkStream.Client/Components/Shared/SearchDropdown.razor.css` | **New** — dropdown styles | | `MilkStream.Client/Components/Layout/NavMenu.razor` | Replace static search markup with `<SearchDropdown />` | | `MilkStream.Client/Components/Pages/Cosplayers.razor` | Parse `?search=` query param | | `MilkStream.Client/Components/Pages/Albums.razor` | Parse `?search=` query param | | `Lactose/Controllers/PersonController.cs` | AllowAnonymous, null uid fix | | `Lactose/Controllers/AlbumController.cs` | AllowAnonymous on search | | `Lactose/Controllers/UserController.cs` | Admin-only user list | | `Lactose/Controllers/AssetController.cs` | Zero-based page numbering | | `Lactose/Controllers/TagController.cs` | Add missing `[FromQuery]` | | `Lactose/Repositories/PersonRepository.cs` | ILike search, materialize before client-side filter | | `Lactose/Repositories/AlbumRepository.cs` | ILike search | | `Lactose/WepApiTest.http` | Complete restructure | | `Lactose/http-client.env.json` | Updated variable names |
Ai_Agent added 6 commits 2026-07-11 15:29:02 +00:00
New SearchDropdown component that provides a live typeahead dropdown
in the navbar searching both cosplayers (persons) and albums.
- Fires API calls on every keystroke (no debounce) to existing /api/person and /api/album
- Version counter discards stale responses
- Minimum 2 characters before searching
- Keyboard navigation (arrows, enter, escape)
- Blur delay for click-to-navigate
- Thumbnail support with fallback avatars/icons
- Loading and empty states
Replace static search input markup in NavMenu with the new
SearchDropdown typeahead component.
- Add missing closing </div> for search-dropdown-container
- Replace @{} variable blocks with IndexOf() calls (invalid inside @if)
- Remove leftover pi++ and ai++ incrementers
Album covers now show as rounded squares (border-radius: 0.375rem)
while cosplayer profile pics remain circular, improving visual
differentiation between result types.
Refactor SearchDropdown to use a flat item list for unified keyboard
navigation. Section headers (Cosplayers/Albums) are now clickable
links that navigate to the respective list page with the search query
as a ?search= parameter.
Headers in the search dropdown navigate to the respective list page
with the search query pre-filled via ?search= parameter. Both pages
now parse this parameter on initialization.
REDCODE added 1 commit 2026-07-11 15:30:21 +00:00
REDCODE added 1 commit 2026-07-11 15:34:08 +00:00
OnInitializedAsync only fires once per component lifetime, so
re-navigating to the same page with a different ?search= value
wouldn't re-parse. OnParametersSet fires on every parameter change,
covering both initial load and subsequent navigations.
REDCODE added 1 commit 2026-07-11 15:38:55 +00:00
Blazor triggers an early render when OnInitializedAsync has a true
await (masonryObserver.unlockBodyScroll). By that point searchQuery
was still null, so the grid briefly loaded unfiltered results and a
stale response could overwrite the correct filtered results.
ParseSearchQuery must run before the await to ensure the first render
passes the correct search query to AlbumGrid.
REDCODE requested review from REDCODE 2026-07-11 15:41:24 +00:00
REDCODE approved these changes 2026-07-11 15:42:32 +00:00
Dismissed
REDCODE added 1 commit 2026-07-11 15:44:19 +00:00
Shows a right-pointing arrow on section headers (Cosplayers/Albums)
to visually signal they are clickable and navigate to the list page.
The arrow subtly moves right on hover for extra affordance.
REDCODE added 1 commit 2026-07-11 16:03:24 +00:00
Replace EF Core Contains() with EF.Functions.ILike() which
translates to PostgreSQL ILIKE for case-insensitive matching.
Searching 'alice', 'Alice', or 'ALICE' now returns the same results.
REDCODE added 1 commit 2026-07-11 16:10:01 +00:00
Match styling of the user profile dropdown in the navbar by using
Bootstrap's native .dropdown-menu and .dropdown-item classes instead
of custom classes. Keep only custom positioning, thumbnail sizing,
and the clickable header styles.
REDCODE added 1 commit 2026-07-11 16:15:05 +00:00
Bootstrap's .dropdown-menu sets display:none. Add display:block to
.search-dropdown-menu to make the search dropdown visible.
REDCODE added 1 commit 2026-07-11 16:33:58 +00:00
Add HTTP endpoint tests covering:
- Refresh token endpoint (empty body, valid, auth failures)
- Album GET by ID (rng user, admin, non-existent)
- Album search (rng user, admin)
- User get all and profile update (rng user, admin)
- Stats endpoint (admin, curator, rng user 403)
- Tag search and create (admin, rng user 403)
- Asset search (rng user, anonymous)
- Curator-level CRUD permissions (person, album, stats succeed;
  user creation and settings access 403)
- Cleanup of curator test user
REDCODE added 1 commit 2026-07-11 16:38:07 +00:00
PersonController.GetAll and AssetController used null-forgiving
operator (!) on authService.GetUserData(User) result. If the
user's claims are present but user data can't be resolved, this
crashes with NullReferenceException.

PersonController now returns 401 Unauthorized when uid is null.
AssetController uses null-conditional (?.) with fallback (??)
instead of null-forgiving (!) for accessLevel.
REDCODE force-pushed feature/global-search-dropdown from 5743371ebd to fbb68397ba 2026-07-11 16:39:45 +00:00 Compare
REDCODE force-pushed feature/global-search-dropdown from fbb68397ba to f626228c2f 2026-07-11 16:39:54 +00:00 Compare
REDCODE added 1 commit 2026-07-11 16:42:11 +00:00
GET /api/person and GET /api/album should not require authentication.
Anonymous users are treated as EAccessLevel.User, so they only see
publicly shared content. PersonController.GetAll uses uid ?? default
instead of uid!.Value to handle null uid safely.
REDCODE added 1 commit 2026-07-11 16:42:14 +00:00
REDCODE added 1 commit 2026-07-11 16:46:11 +00:00
The nested Any() through SharedWith navigation cannot be translated
to SQL by EF Core. Apply the same pattern as GetAllVisible and
AlbumController.Search: materialize server-side (search, sort,
pagination), then filter by visibility in memory.
REDCODE added 1 commit 2026-07-11 16:48:51 +00:00
The person search may return an empty array when the database has no
publicly visible people. Only set personId when results exist.
REDCODE added 1 commit 2026-07-11 16:50:18 +00:00
Rng user may see no public people, leaving personId unset. Admin
sees all people including test-created ones, so setting personId from
the admin response ensures downstream tests have a valid reference.
REDCODE added 1 commit 2026-07-11 16:51:27 +00:00
UserController.GetAll was missing [Authorize(Roles = Admin)],
allowing any authenticated user to list all users.
REDCODE added 1 commit 2026-07-11 16:52:56 +00:00
UserUpdateDto.Id is required. The test was sending only username,
causing 400 Bad Request from model binding failure.
REDCODE added 1 commit 2026-07-11 16:54:05 +00:00
Complex type PagedSearchParametersDto needs [FromQuery] for GET
requests, otherwise ASP.NET Core tries body binding and returns 415.
REDCODE added 1 commit 2026-07-11 16:55:16 +00:00
AssetController.GetAll validated page >= 1 (one-based), inconsistent
with PersonController and AlbumController which use zero-based pages.
Adjust validation to page >= 0 and pass page + 1 to the repository
which internally uses one-based.
REDCODE added 1 commit 2026-07-11 17:00:54 +00:00
EAccessLevel enum: User=0, Curator=1, Admin=2. The test created the
curator user with accessLevel=2 (Admin), making curator_token an
admin token, which broke the 'Create user as curator should 403' test.
REDCODE added 1 commit 2026-07-11 17:02:59 +00:00
The test file now creates three distinct users at the start:
  - admin (seeded, admin/admin)
  - curator (created by admin, accessLevel=1)
  - user (registered via /api/auth/register, accessLevel=0)

Each user has its own token variable (admin_token, curator_token,
user_token) and ID variable (admin_id, curator_id, user_id).

Sections are organized: Setup → Auth → User CRUD → Person →
Album → Tag → Asset → Stats → Settings → Cleanup.

All endpoints are tested at each appropriate authorization level:
admin (full access), curator (elevated access), user (own data only),
and anonymous (public data only).
Ai_Agent requested review from REDCODE 2026-07-11 17:04:07 +00:00
Ai_Agent requested review from Fastwind 2026-07-11 17:04:07 +00:00
REDCODE added 3 commits 2026-07-11 18:41:08 +00:00
Replace client-side visibility filtering with EF Core Any() subqueries in SearchQuery and GetAllVisible. This eliminates the cartesian product from the Include chain (Person -> Albums -> Assets -> SharedWith) and lets the database short-circuit the auth check with indexes. Adds null-forgiving operators (!) on navigation access in the query predicates.
The visibility subquery doesn't load the Albums navigation, so TotalAlbums in the mapper was always 0. Added back a lightweight Include(p => p.Albums) (without Assets/SharedWith chain) to both SearchQuery and GetAllVisible.
REDCODE added 1 commit 2026-07-11 18:49:48 +00:00
Include + Skip/Take on a collection navigation can produce duplicate people when sorted by album count, because pagination operates on joined rows. Fix by selecting paged IDs first, then loading the full entities with Include for those IDs.
REDCODE reviewed 2026-07-12 08:57:31 +00:00
@@ -125,3 +125,3 @@
}
if (searchOptionsDto.Page < 1 || searchOptionsDto.PageSize < 1) {
if (searchOptionsDto.Page < 0 || searchOptionsDto.PageSize < 1) {
Owner

I'll just add a note here, this is temporary and #95 should take care of this at a later stage.

I'll just add a note here, this is temporary and #95 should take care of this at a later stage.
REDCODE marked this conversation as resolved
REDCODE reviewed 2026-07-12 08:59:09 +00:00
@@ -97,18 +97,10 @@ namespace Lactose.Controllers {
/// Gets all users.
/// </summary>
/// <returns>A list of all users.</returns>
[Authorize(Roles = "Admin")]
Owner

how social do we want this to become? We'll change this in future I guess

how social do we want this to become? We'll change this in future I guess
Owner

It was structured to be possible to search other user, but yet again we need a defined use cases of what we want to actually do

It was structured to be possible to search other user, but yet again we need a defined use cases of what we want to actually do
REDCODE approved these changes 2026-07-12 09:00:52 +00:00
Dismissed
REDCODE added 2 commits 2026-07-12 14:52:31 +00:00
Switch accumulating list storage to Dictionary<Guid, T> to guarantee
unique Blazor @key values when paginated data overlaps or races occur
during sort/filter changes.

- CosplayerGrid: Dictionary<Guid, PersonPreviewDto> for people list
- CosplayerDetail: Dictionary<Guid, AlbumPreviewDto> for albums
- AlbumDetail: deduplicate album.Images at load time via dict
- Home: deduplicate flatList rebuild via dict (keeps first occurrence)
- AssetPicker: Distinct() on parameter list
- Add Person.TotalVisibleAlbums [NotMapped] populated by repository
- Use visibility-aware album count in PersonMapper
- Restructure PersonRepository.SearchQuery with two-step pagination
  for correct ordering and per-user album visibility filtering
REDCODE approved these changes 2026-07-12 14:54:03 +00:00
Dismissed
REDCODE requested changes 2026-07-12 14:55:22 +00:00
Dismissed
@@ -53,0 +55,4 @@
/// Populated by <see cref="Repositories.PersonRepository.SearchQuery"/> — do not rely on this outside that context.
/// </summary>
[NotMapped]
public int TotalVisibleAlbums { get; set; }
Owner

I really don't like a single property, not mapped that is reliable only in some contexts

I really don't like a single property, not mapped that is reliable only in some contexts
Author
Collaborator

Addressed review feedback on TotalVisibleAlbums (commit c77096b)

Removed the [NotMapped] TotalVisibleAlbums property from Person.cs and refactored PersonRepository.SearchQuery to project PersonPreviewDto directly in the EF Core subquery.

  • PersonRepository.SearchQuery now returns IEnumerable<PersonPreviewDto> instead of IEnumerable<Person>
  • The visibility-aware album count is computed inline in the Select expression — no temp property needed
  • PersonMapper.ToPersonPreviewDto reverts to person.Albums?.Count ?? 0 (only used by Get(id) endpoint now; GetAll bypasses the mapper entirely)
  • PersonController.GetAll calls ToList() directly on the repository result
**Addressed review feedback on `TotalVisibleAlbums` (commit `c77096b`)** Removed the `[NotMapped] TotalVisibleAlbums` property from `Person.cs` and refactored `PersonRepository.SearchQuery` to project `PersonPreviewDto` directly in the EF Core subquery. - `PersonRepository.SearchQuery` now returns `IEnumerable<PersonPreviewDto>` instead of `IEnumerable<Person>` - The visibility-aware album count is computed inline in the `Select` expression — no temp property needed - `PersonMapper.ToPersonPreviewDto` reverts to `person.Albums?.Count ?? 0` (only used by `Get(id)` endpoint now; `GetAll` bypasses the mapper entirely) - `PersonController.GetAll` calls `ToList()` directly on the repository result
Author
Collaborator

Refactored AlbumRepository.SearchQuery to match the new PersonRepository pattern (commit 89edf29)

Same approach as the Person repository — projects AlbumPreviewDto directly with visibility-aware logic:

  • SearchQuery now returns IEnumerable<AlbumPreviewDto> instead of IEnumerable<Album>
  • Access-level filtering pushed into the query (.Where() for EAccessLevel.User)
  • AssetCount computed inline using the same visibility subquery — regular users see counts of only visible assets
  • Sorting by "assets" uses the same visibility-aware count for regular users
  • No more .Include() of full asset graphs — only the columns needed for the DTO are selected
  • Removed the in-memory filtering switch block and .Select(x => x.ToAlbumPreviewDto()) from AlbumController.Search
  • Removed the now-unused AlbumMapper.ToAlbumPreviewDto extension
**Refactored `AlbumRepository.SearchQuery` to match the new `PersonRepository` pattern (commit `89edf29`)** Same approach as the Person repository — projects `AlbumPreviewDto` directly with visibility-aware logic: - `SearchQuery` now returns `IEnumerable<AlbumPreviewDto>` instead of `IEnumerable<Album>` - Access-level filtering pushed into the query (`.Where()` for `EAccessLevel.User`) - `AssetCount` computed inline using the same visibility subquery — regular users see counts of only visible assets - Sorting by `"assets"` uses the same visibility-aware count for regular users - No more `.Include()` of full asset graphs — only the columns needed for the DTO are selected - Removed the in-memory filtering switch block and `.Select(x => x.ToAlbumPreviewDto())` from `AlbumController.Search` - Removed the now-unused `AlbumMapper.ToAlbumPreviewDto` extension
Author
Collaborator

Added performance index for visibility queries (commit 48b6d4f)

Created a partial index IX_Assets_VisibleForSearch on Assets(IsPubliclyShared, OwnerId) WHERE "DeletedAt" IS NULL to speed up the correlated subqueries used in both AlbumRepository.SearchQuery and PersonRepository.SearchQuery. The index allows PostgreSQL to evaluate the visibility-aware asset/album counts with a compact b-tree scan instead of scanning the heap per outer row.

Also updated AGENTS.md to document the fluent API convention (HasIndex().HasFilter()) for database indexes, since the [Index] attribute's Filter parameter isn't available with the current EF Core Tools v8 vs Design v10 mismatch.

**Added performance index for visibility queries (commit `48b6d4f`)** Created a partial index `IX_Assets_VisibleForSearch` on `Assets(IsPubliclyShared, OwnerId) WHERE "DeletedAt" IS NULL` to speed up the correlated subqueries used in both `AlbumRepository.SearchQuery` and `PersonRepository.SearchQuery`. The index allows PostgreSQL to evaluate the visibility-aware asset/album counts with a compact b-tree scan instead of scanning the heap per outer row. Also updated `AGENTS.md` to document the fluent API convention (`HasIndex().HasFilter()`) for database indexes, since the `[Index]` attribute's `Filter` parameter isn't available with the current EF Core Tools v8 vs Design v10 mismatch.
REDCODE added 5 commits 2026-07-12 16:25:03 +00:00
Replace the TempData-like TotalVisibleAlbums [NotMapped] property on
Person with a direct DTO projection in PersonRepository.SearchQuery.
The repository now returns IEnumerable<PersonPreviewDto>, computing
the visibility-aware album count in the EF Core subquery and projecting
only the needed columns. This avoids polluting the entity model with a
context-dependent property.
Align AlbumRepository.SearchQuery with the PersonRepository pattern:
return AlbumPreviewDto directly with access-level filtering at the
query level, avoiding loading full entity graphs and filtering
in-memory in the controller.

- Add userId/accessLevel params to SearchQuery
- Push visibility filter into the query for regular users
- Compute AssetCount with same visibility logic in the projection
- Sort by 'assets' uses visible count for regular users
- Remove unused AlbumMapper.ToAlbumPreviewDto extension
- Simplify AlbumController.Search to just return repo results
Create IX_Assets_VisibleForSearch on (IsPubliclyShared, OwnerId)
WHERE "DeletedAt" IS NULL to accelerate the correlated subqueries
used in both AlbumRepository and PersonRepository for filtering,
sorting by asset/album count, and projecting AssetCount/TotalAlbums.

- Fluent API in LactoseDbContext.OnModelCreating
- Generated migration via dotnet ef migrations add
Simplify the 'assets'/'albums' sort to use total count instead of
visibility-filtered count, eliminating the expensive correlated COUNT
subquery from ORDER BY (which evaluated 25k times for every album).

Add pg_trgm extension and GIN trigram indexes on People.Name and
Albums.Title for efficient ILike %%query%% searches.

The SELECT projection still computes visibility-filtered counts for
accuracy, but only for the paginated subset (30 rows).

Migration: 20260712162406_AddTrigramIndexes
REDCODE requested changes 2026-07-12 16:47:25 +00:00
Dismissed
@@ -36,4 +34,0 @@
/// </summary>
/// <param name="album">The album to map.</param>
/// <returns>An AlbumPreviewDto object.</returns>
public static AlbumPreviewDto ToAlbumPreviewDto(this Album album) => new AlbumPreviewDto {
Owner

This should be kept

This should be kept
REDCODE marked this conversation as resolved
REDCODE added 1 commit 2026-07-12 16:50:07 +00:00
REDCODE approved these changes 2026-07-12 16:51:47 +00:00
Dismissed
REDCODE added 1 commit 2026-07-12 17:03:16 +00:00
- AssetRepository: all Skip() formulas changed from (page-1)*size to page*size
- JobRecordRepository: same formula change for GetPastRootJobs
- AssetController: removed Page+1 bridge conversion, passes Page directly
- JobsController: GetPast default changed from 1 to 0, added validation
- AlbumController, TagController, PersonController: consistent Page<0/PageSize<1 validation
- IAssetRepository XML doc: 1-based → zero-based
- Home.razor: shifted all internal page state from 1-based to 0-based

Closes #95
Author
Collaborator

Rebased & addressed all review feedback:

  • Pagination zero-based standard (closes #95):

    • AssetRepository: all Skip() → page * size (3 occurrences)
    • JobRecordRepository: same formula fix
    • AssetController: removed Page + 1 bridge conversion
    • JobsController, AlbumController, TagController, PersonController: consistent validation (Page >= 0 && PageSize >= 1)
    • Home.razor: shifted internal state to 0-based
    • IAssetRepository XML doc updated
  • Item 4 from previous review (double-correct page offset) resolved — the + 1 was compensating for a 1-based repo; now everything is zero-based end-to-end.

Ready for re-review.

Rebased & addressed all review feedback: - **Pagination zero-based standard (closes #95):** - AssetRepository: all Skip() → `page * size` (3 occurrences) - JobRecordRepository: same formula fix - AssetController: removed `Page + 1` bridge conversion - JobsController, AlbumController, TagController, PersonController: consistent validation (`Page >= 0 && PageSize >= 1`) - Home.razor: shifted internal state to 0-based - IAssetRepository XML doc updated - Item 4 from previous review (double-correct page offset) resolved — the `+ 1` was compensating for a 1-based repo; now everything is zero-based end-to-end. Ready for re-review.
REDCODE added 1 commit 2026-07-12 17:07:43 +00:00
EF Core cannot translate SharedWith.Any() through the many-to-many
AlbumAsset join table inside a SQL WHERE clause. Moved the visibility
filter to client-side after loading the page's records with Include.

PersonRepository.SearchQuery and AlbumRepository.SearchQuery now:
1. Get paginated IDs without visibility filter (translatable SQL)
2. Load those records with Include chains
3. Filter visibility in C# (SharedWith.Any(), etc.)
4. Project to DTOs
Author
Collaborator

Fixed the 4 failing tests (23, 24, 25, 45, 46):

  • Root cause: SharedWith.Any() navigation through the many-to-many AlbumAsset join table couldn't be translated by EF Core inside a SQL WHERE clause
  • Fix: Moved visibility filtering to client-side in both PersonRepository.SearchQuery and AlbumRepository.SearchQuery — the two-pass ID pagination is preserved, but the page's records are loaded with .Include(...) chains and filtered in C# before projection to DTOs
  • The AssetCount/TotalAlbums visibility-aware counts still run inline (they work in the second-pass .Select() projection, just not in the first-pass .Where())

Committed 20a4565.

Fixed the 4 failing tests (23, 24, 25, 45, 46): - **Root cause:** `SharedWith.Any()` navigation through the many-to-many `AlbumAsset` join table couldn't be translated by EF Core inside a SQL WHERE clause - **Fix:** Moved visibility filtering to client-side in both `PersonRepository.SearchQuery` and `AlbumRepository.SearchQuery` — the two-pass ID pagination is preserved, but the page's records are loaded with `.Include(...)` chains and filtered in C# before projection to DTOs - The `AssetCount`/`TotalAlbums` visibility-aware counts still run inline (they work in the second-pass `.Select()` projection, just not in the first-pass `.Where()`) Committed `20a4565`.
REDCODE added 1 commit 2026-07-12 17:11:12 +00:00
REDCODE added 1 commit 2026-07-12 17:17:17 +00:00
Person (tests 23-26), Album (tests 45-48), and User (test 12) list
endpoints now explicitly request the first page instead of the full set.
REDCODE requested changes 2026-07-12 17:25:50 +00:00
Dismissed
REDCODE left a comment
Owner

fix whatever the fuck is going on with the pagination. 0 should be the first page, everywhere. the backend commands, if the frontend has problems with that they shoudl fix them on their side.

fix whatever the fuck is going on with the pagination. 0 should be the first page, everywhere. the backend commands, if the frontend has problems with that they shoudl fix them on their side.
@@ -82,2 +82,2 @@
if(pagingOptions.Page < 0 || pagingOptions.PageSize < 0) return BadRequest();
if(pagingOptions.Page < 0 || pagingOptions.PageSize < 1) return BadRequest();
Owner

Page 0 should be the first page, what are we doing here?

Page 0 should be the first page, what are we doing here?
REDCODE marked this conversation as resolved
@@ -75,3 +76,4 @@
var uid = authService.GetUserData(User)?.Id;
var accessLevel = authService.GetUserData(User)?.AccessLevel ?? EAccessLevel.User;
if (pagedSearch.Page < 0 || pagedSearch.PageSize < 1) { return BadRequest(); }
Owner

what? Why? This should be able to accept any page starting from 0 and going up. So < 0 is the only invalid value. If the frontend is wrong in using this that should be fixed there. 0 is the first page. Everywhere.

what? Why? This should be able to accept any page starting from 0 and going up. So < 0 is the only invalid value. If the frontend is wrong in using this that should be fixed there. 0 is the first page. Everywhere.
REDCODE marked this conversation as resolved
@@ -71,3 +71,3 @@
pagedSearch.Search ??= string.Empty;
if(pagedSearch.Page < 0) { return BadRequest("Page must be greater than 0"); }
if(pagedSearch.PageSize < 0) { return BadRequest("PageSize must be greater than 0"); }
if(pagedSearch.PageSize < 1) { return BadRequest("PageSize must be greater than 0"); }
Owner

Here aswell like before... Page 0 shoudl be valid, not the opposite.

Here aswell like before... Page 0 shoudl be valid, not the opposite.
REDCODE marked this conversation as resolved
REDCODE added 1 commit 2026-07-12 17:27:13 +00:00
Per review feedback: only Page being negative is truly invalid.
PageSize validation reverted to original < 0 (allowing 0).
PersonController keeps minimal Page < 0 check since it had none before.
JobsController aligned for consistency.
Author
Collaborator

Addressed the validation review points:

  • AlbumController, TagController: reverted PageSize < 1 back to original PageSize < 0
  • PersonController: removed PageSize < 1, kept only Page < 0
  • JobsController: aligned to PageSize < 0 for consistency

Only Page < 0 is rejected everywhere. PageSize=0 is allowed.

Addressed the validation review points: - **AlbumController, TagController**: reverted `PageSize < 1` back to original `PageSize < 0` - **PersonController**: removed `PageSize < 1`, kept only `Page < 0` - **JobsController**: aligned to `PageSize < 0` for consistency Only `Page < 0` is rejected everywhere. PageSize=0 is allowed.
REDCODE added 1 commit 2026-07-12 17:31:24 +00:00
REDCODE approved these changes 2026-07-12 17:34:23 +00:00
Dismissed
REDCODE added 1 commit 2026-07-12 17:41:13 +00:00
- Add Lactose.Analyzers as 5th project (DateTime.UtcNow error MS001)
- Document zero-based pagination standard with validation rules
- Add SharedWith.Any() EF Core translation gotcha
- Document pg_trgm GIN indexes and pgvector extensions
- Fix test count (66+ → 67), add env file reference
Ai_Agent added the area:frontend label 2026-07-12 17:44:55 +00:00
REDCODE added the area:backendarea:dbpage:album-detail labels 2026-07-12 17:45:48 +00:00
Ai_Agent added area:authpage:albumspage:cosplayers and removed page:album-detail labels 2026-07-12 17:47:44 +00:00
REDCODE approved these changes 2026-07-12 17:49:31 +00:00
Dismissed
REDCODE added 1 commit 2026-07-12 18:30:35 +00:00
Resolved merge conflict in CosplayerDetail.razor:
- Kept SortFilterBar with instant search/sort (from feat/cosplayer-detail-search)
- Adapted allAlbums references to allAlbumsDict (from feature/global-search-dropdown)
- Kept updated PersonController.cs and PersonService.cs from both branches
REDCODE approved these changes 2026-07-12 18:41:30 +00:00
Dismissed
REDCODE added 3 commits 2026-07-12 19:02:54 +00:00
REDCODE requested changes 2026-07-12 19:10:11 +00:00
Dismissed
@@ -48,0 +74,4 @@
).ToList();
// Project to DTOs
var dtos = pagedAlbums.Select(a => new AlbumPreviewDto {
Owner

Don't we have an overload method in the mapper that allows us to to an album.ToAlbumPreviewDto()?

Don't we have an overload method in the mapper that allows us to to an album.ToAlbumPreviewDto()?
REDCODE marked this conversation as resolved
REDCODE added 1 commit 2026-07-12 19:15:12 +00:00
Replace inline AlbumPreviewDto construction in AlbumRepository.SearchQuery
with a call to the new ToAlbumPreviewDto(album, userId, accessLevel) mapper
overload, making the visibility logic reusable and the repository more compact.
REDCODE approved these changes 2026-07-12 19:16:02 +00:00
Dismissed
REDCODE added this to the v1.0 - Initial Release milestone 2026-07-13 12:16:25 +00:00
Fastwind requested changes 2026-07-13 13:08:51 +00:00
Dismissed
Fastwind left a comment
Owner

Need the other comments to be resolved before merge

Need the other comments to be resolved before merge
@@ -75,35 +75,25 @@ public class AlbumController(
/// <param name="pagingOptions">Pagination, search, sort, and filter parameters.</param>
/// <returns>A list of album previews accessible to the user.</returns>
[HttpGet]
[AllowAnonymous]
Owner

Are we sure to enable search queries to non users?

As search is an heavy request to execute, it may lead to dos attack without proper limitations

Are we sure to enable search queries to non users? As search is an heavy request to execute, it may lead to dos attack without proper limitations
Owner

Uh... being able to search albums/cosplayers and such it's kinda needed when there is way too much content... Funny enough, the search seems to be very inexpensive for now, sorting ends up being the worst (see relevant issues as #98 #101)

Uh... being able to search albums/cosplayers and such it's kinda needed when there is way too much content... Funny enough, the search seems to be very inexpensive for now, sorting ends up being the worst (see relevant issues as #98 #101)
Owner

Still if it's the server doing it, it doesn't resolve my question
it's more of a yes or no to my question,
followed by a new security issue in case of yes

Still if it's the server doing it, it doesn't resolve my question it's more of a yes or no to my question, followed by a new security issue in case of yes
Owner

@Ai_Agent remove allow anonymus for now, we will enable this when it's time with prooper rate limiting

@Ai_Agent remove allow anonymus for now, we will enable this when it's time with prooper rate limiting
REDCODE marked this conversation as resolved
@@ -36,2 +36,3 @@
[Authorize(Roles = "Admin")]
public ActionResult<PastJobsResponse> GetPast([FromQuery] int page = 1, [FromQuery] int pageSize = 5) {
public ActionResult<PastJobsResponse> GetPast([FromQuery] int page = 0, [FromQuery] int pageSize = 5) {
if (page < 0 || pageSize < 1 || pageSize > 250) { return BadRequest(); }
Owner

We could extend page size to 1000 rows if the row doesn't weight too much
as this is admin only I don't expect the need for security limits

We could extend page size to 1000 rows if the row doesn't weight too much as this is admin only I don't expect the need for security limits
Owner

Job page interface is paginated because it also then fetches separately child job data. Kind of an hard to run query but in theory it should be kept clean by the max retention time

Job page interface is paginated because it also then fetches separately child job data. Kind of an hard to run query but in theory it should be kept clean by the max retention time
REDCODE marked this conversation as resolved
@@ -98,20 +98,22 @@ public class PersonController(
/// <param name="pagedSearch">Pagination, search, and sort parameters.</param>
/// <returns>A paginated list of person previews.</returns>
[HttpGet]
[AllowAnonymous]
Owner

Are we sure to enable search queries to non users?

As search is an heavy request to execute, it may lead to dos attack without proper limitations

Are we sure to enable search queries to non users? As search is an heavy request to execute, it may lead to dos attack without proper limitations
Owner

See above

See above
Owner

Keep looking above, and just follow that one

Keep looking above, and just follow that one
REDCODE marked this conversation as resolved
@@ -71,3 +70,3 @@
public ActionResult<List<TagDto>> GetAll([FromQuery] PagedSearchParametersDto pagedSearch) {
pagedSearch.Search ??= string.Empty;
if(pagedSearch.Page < 0) { return BadRequest("Page must be greater than 0"); }
if(pagedSearch.PageSize < 0) { return BadRequest("PageSize must be greater than 0"); }
Owner

Can we have a single const with the max PageSize, instead of using magic numbers

Can we have a single const with the max PageSize, instead of using magic numbers
Owner

Sure, Where do we stash it? Are we making a static class?

Sure, Where do we stash it? Are we making a static class?
Owner

it could be a public static property from the same paged class, unless you plan to have a config for that too

it could be a public static property from the same paged class, unless you plan to have a config for that too
Owner

On it

On it
REDCODE marked this conversation as resolved
REDCODE added 1 commit 2026-07-13 15:02:48 +00:00
REDCODE added 1 commit 2026-07-13 15:05:29 +00:00
REDCODE approved these changes 2026-07-13 15:07:15 +00:00
REDCODE requested review from Fastwind 2026-07-13 15:14:29 +00:00
Fastwind approved these changes 2026-07-13 15:17:11 +00:00
REDCODE merged commit d2daed27a8 into develop 2026-07-13 15:18:42 +00:00
REDCODE deleted branch feature/global-search-dropdown 2026-07-13 15:18:42 +00:00
Sign in to join this conversation.
No Reviewers
3 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: MilkyShots/MilkyShots#91