Global "Search Anything" typeahead dropdown in navbar #91
Reference in New Issue
Block a user
Delete Branch "feature/global-search-dropdown"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
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:GET /api/personandGET /api/album>chevron indicating they navigate to the full list page.dropdown-menu/.dropdown-itemstyling (matching the user profile dropdown)SearchDropdown.razor.css— positioning, thumbnail sizing, clickable header stylesQuery Param Pre-fill on List Pages
Cosplayers.razor/Albums.razor— parse?search=query parameter on page load/cosplayers?search={term}//albums?search={term}Backend Fixes
Authentication & Authorization
PersonController.GetAll— return 401 whenGetUserDatareturns 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— useEF.Functions.ILike()for case-insensitive search (was usingContains()which is case-sensitive on PostgreSQL)PersonRepository.SearchQuery— materialize query before client-side visibility filter (nestedSharedWith.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]onPagedSearchParametersDtoparam (was returning 415)Test File Restructure
Lactose/WepApiTest.http— fully rewritten with three clearly separated users:admin_token): full accesscurator_token): elevated accessuser_token): own data onlyFiles Changed (14 files)
MilkStream.Client/Components/Shared/SearchDropdown.razorMilkStream.Client/Components/Shared/SearchDropdown.razor.cssMilkStream.Client/Components/Layout/NavMenu.razor<SearchDropdown />MilkStream.Client/Components/Pages/Cosplayers.razor?search=query paramMilkStream.Client/Components/Pages/Albums.razor?search=query paramLactose/Controllers/PersonController.csLactose/Controllers/AlbumController.csLactose/Controllers/UserController.csLactose/Controllers/AssetController.csLactose/Controllers/TagController.cs[FromQuery]Lactose/Repositories/PersonRepository.csLactose/Repositories/AlbumRepository.csLactose/WepApiTest.httpLactose/http-client.env.json- Add missing closing </div> for search-dropdown-container - Replace @{} variable blocks with IndexOf() calls (invalid inside @if) - Remove leftover pi++ and ai++ incrementers5743371ebdtofbb68397bafbb68397batof626228c2f@@ -125,3 +125,3 @@}if (searchOptionsDto.Page < 1 || searchOptionsDto.PageSize < 1) {if (searchOptionsDto.Page < 0 || searchOptionsDto.PageSize < 1) {I'll just add a note here, this is temporary and #95 should take care of this at a later stage.
@@ -97,18 +97,10 @@ namespace Lactose.Controllers {/// Gets all users./// </summary>/// <returns>A list of all users.</returns>[Authorize(Roles = "Admin")]how social do we want this to become? We'll change this in future I guess
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
@@ -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; }I really don't like a single property, not mapped that is reliable only in some contexts
Addressed review feedback on
TotalVisibleAlbums(commitc77096b)Removed the
[NotMapped] TotalVisibleAlbumsproperty fromPerson.csand refactoredPersonRepository.SearchQueryto projectPersonPreviewDtodirectly in the EF Core subquery.PersonRepository.SearchQuerynow returnsIEnumerable<PersonPreviewDto>instead ofIEnumerable<Person>Selectexpression — no temp property neededPersonMapper.ToPersonPreviewDtoreverts toperson.Albums?.Count ?? 0(only used byGet(id)endpoint now;GetAllbypasses the mapper entirely)PersonController.GetAllcallsToList()directly on the repository resultRefactored
AlbumRepository.SearchQueryto match the newPersonRepositorypattern (commit89edf29)Same approach as the Person repository — projects
AlbumPreviewDtodirectly with visibility-aware logic:SearchQuerynow returnsIEnumerable<AlbumPreviewDto>instead ofIEnumerable<Album>.Where()forEAccessLevel.User)AssetCountcomputed inline using the same visibility subquery — regular users see counts of only visible assets"assets"uses the same visibility-aware count for regular users.Include()of full asset graphs — only the columns needed for the DTO are selected.Select(x => x.ToAlbumPreviewDto())fromAlbumController.SearchAlbumMapper.ToAlbumPreviewDtoextensionAdded performance index for visibility queries (commit
48b6d4f)Created a partial index
IX_Assets_VisibleForSearchonAssets(IsPubliclyShared, OwnerId) WHERE "DeletedAt" IS NULLto speed up the correlated subqueries used in bothAlbumRepository.SearchQueryandPersonRepository.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.mdto document the fluent API convention (HasIndex().HasFilter()) for database indexes, since the[Index]attribute'sFilterparameter isn't available with the current EF Core Tools v8 vs Design v10 mismatch.@@ -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 {This should be kept
Rebased & addressed all review feedback:
Pagination zero-based standard (closes #95):
page * size(3 occurrences)Page + 1bridge conversionPage >= 0 && PageSize >= 1)Item 4 from previous review (double-correct page offset) resolved — the
+ 1was compensating for a 1-based repo; now everything is zero-based end-to-end.Ready for re-review.
Fixed the 4 failing tests (23, 24, 25, 45, 46):
SharedWith.Any()navigation through the many-to-manyAlbumAssetjoin table couldn't be translated by EF Core inside a SQL WHERE clausePersonRepository.SearchQueryandAlbumRepository.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 DTOsAssetCount/TotalAlbumsvisibility-aware counts still run inline (they work in the second-pass.Select()projection, just not in the first-pass.Where())Committed
20a4565.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();Page 0 should be the first page, what are we doing here?
@@ -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(); }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.
@@ -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"); }Here aswell like before... Page 0 shoudl be valid, not the opposite.
Addressed the validation review points:
PageSize < 1back to originalPageSize < 0PageSize < 1, kept onlyPage < 0PageSize < 0for consistencyOnly
Page < 0is rejected everywhere. PageSize=0 is allowed.@@ -48,0 +74,4 @@).ToList();// Project to DTOsvar dtos = pagedAlbums.Select(a => new AlbumPreviewDto {Don't we have an overload method in the mapper that allows us to to an album.ToAlbumPreviewDto()?
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]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
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)
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
@Ai_Agent remove allow anonymus for now, we will enable this when it's time with prooper rate limiting
@@ -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(); }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
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
@@ -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]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
See above
Keep looking above, and just follow that one
@@ -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"); }Can we have a single const with the max PageSize, instead of using magic numbers
Sure, Where do we stash it? Are we making a static class?
it could be a public static property from the same paged class, unless you plan to have a config for that too
On it