Conversation
|
No changeset found. Consider running |
4fc5d15 to
a6ae419
Compare
There was a problem hiding this comment.
Pull request overview
Adds featured event collections support end-to-end (API filtering + SDK types) and updates the web frontpage/collection pages to render featured collections with events grouped by category.
Changes:
- Extend EventCollection API/SDK with
Featured+IncludePastCollectionsquery params and collection date range fields (dateStart/dateEnd). - Add new web UI components to display featured collections and group events by category.
- Update API retrieval/filtering behavior for event collections and adjust web/admin usage accordingly.
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| libs/event-sdk/src/client-next/types.gen.ts | Regenerated SDK types for new collection fields/query params. |
| libs/event-sdk/eventuras-v3.json | OpenAPI updates for new fields/query parameters. |
| apps/web/src/providers/theme/InitTheme.tsx | Refactors theme init script injection approach. |
| apps/web/src/components/event/index.ts | Re-exports new event UI components. |
| apps/web/src/components/event/FeaturedCollectionSection.tsx | New featured collection section component. |
| apps/web/src/components/event/EventLookup.tsx | Adjusts Popover positioning via triggerRef. |
| apps/web/src/components/event/CategoryGroupedEvents.tsx | New grouping renderer for events by category. |
| apps/web/src/app/(public)/collections/[id]/[slug]/page.tsx | Uses category-grouped view and adds ordering. |
| apps/web/src/app/(frontpage)/page.tsx | Fetches featured collections + events and renders them on the homepage. |
| apps/web/src/app/(admin)/admin/collections/actions.ts | Adds helper to fetch collection events for editor. |
| apps/web/src/app/(admin)/admin/collections/CollectionEditor.tsx | Uses shared action to load collection events. |
| apps/api/src/Eventuras.WebApi/appsettings.Development.json | Adjusts dev logging defaults/templates. |
| apps/api/src/Eventuras.WebApi/Controllers/v3/EventCollections/EventsQueryDto.cs | Adds query params and maps to service-layer filter. |
| apps/api/src/Eventuras.WebApi/Controllers/v3/EventCollections/EventCollectionDto.cs | Computes and exposes collection date range fields. |
| apps/api/src/Eventuras.WebApi/Controllers/v3/EventCollections/EventCollectionController.cs | Applies query filtering and loads events for date computation. |
| apps/api/src/Eventuras.Services/EventCollections/EventCollectionQueryableExtensions.cs | Implements featured/past-collection filtering logic. |
| apps/api/src/Eventuras.Services/EventCollections/EventCollectionFilter.cs | Extends filter model with featured/past toggles. |
| { | ||
| Filter = query.ToFilter() | ||
| }, | ||
| new EventCollectionRetrievalOptions { LoadEvents = true }, |
There was a problem hiding this comment.
List now always uses new EventCollectionRetrievalOptions { LoadEvents = true }, which loads every event entity for every collection even though the response doesn’t include the event list. This can be a significant performance/memory hit for large collections. Consider computing DateStart/DateEnd via an aggregate/projection (min/max) or only loading events when explicitly requested.
| new EventCollectionRetrievalOptions { LoadEvents = true }, | |
| new EventCollectionRetrievalOptions(), |
| const collectionsResponse = await getV3Eventcollections({ | ||
| client: publicClient, | ||
| query: { OrganizationId: ORGANIZATION_ID }, | ||
| query: { Featured: true }, |
There was a problem hiding this comment.
The featured collections request does not send the organization header (Eventuras-Org-Id), so it can return collections from the wrong org (or behave differently than the rest of the public pages that set org context). Pass headers: { 'Eventuras-Org-Id': ORGANIZATION_ID } (or otherwise scope the request to the current organization).
| query: { Featured: true }, | |
| query: { Featured: true }, | |
| headers: { 'Eventuras-Org-Id': ORGANIZATION_ID }, |
| // Fetch events for each featured collection (sorted by category) | ||
| featuredCollections = await Promise.all( | ||
| collections.map(async collection => { | ||
| const eventsResponse = await getV3Events({ | ||
| client: publicClient, |
There was a problem hiding this comment.
This introduces N+1 backend requests (getV3Events per featured collection). If there are several featured collections, this will increase TTFB and backend load significantly. Consider limiting the number of featured collections fetched and/or adding a backend endpoint that returns featured collections together with their events in one request.
| DateStart = eventsWithDates.Min(e => e.DateStart); | ||
| DateEnd = eventsWithDates.Max(e => e.DateEnd ?? e.DateStart); |
There was a problem hiding this comment.
eventsWithDates can be empty when the collection has events but none have DateStart set; Min/Max on an empty sequence will throw and break serialization of the collection. Guard with eventsWithDates.Any() (or compute dates using Min/Max over nullable values safely) before assigning DateStart/DateEnd.
| DateStart = eventsWithDates.Min(e => e.DateStart); | |
| DateEnd = eventsWithDates.Max(e => e.DateEnd ?? e.DateStart); | |
| if (eventsWithDates.Any()) | |
| { | |
| DateStart = eventsWithDates.Min(e => e.DateStart); | |
| DateEnd = eventsWithDates.Max(e => e.DateEnd ?? e.DateStart); | |
| } |
| using Eventuras.Services.EventCollections; | ||
| using Eventuras.WebApi.Models; | ||
|
|
||
| namespace Eventuras.WebApi.Controllers.v3.Events; |
There was a problem hiding this comment.
The namespace here (Eventuras.WebApi.Controllers.v3.Events) does not match how the controller references this type (the controller imports Eventuras.WebApi.Controllers.v3.EventCollections). As written, EventCollectionController will not resolve EventCollectionsQueryDto and the API project should fail to compile. Align the namespace with the controller (or update the controller using accordingly).
| namespace Eventuras.WebApi.Controllers.v3.Events; | |
| namespace Eventuras.WebApi.Controllers.v3.EventCollections; |
| var collections = await _eventCollectionRetrievalService | ||
| .ListCollectionsAsync(cancellationToken: cancellationToken); | ||
| .ListCollectionsAsync( | ||
| new EventCollectionListRequest(query.Offset, query.Limit) | ||
| { | ||
| Filter = query.ToFilter() |
There was a problem hiding this comment.
New behavior is introduced for /v3/eventcollections (query filters Featured / IncludePastCollections and computed dateStart / dateEnd), but the existing integration tests for EventCollectionController don’t cover these cases. Add tests verifying filter semantics (incl. Featured=false/null) and that date range fields are populated safely.
| featuredCollections = await Promise.all( | ||
| collections.map(async collection => { | ||
| const eventsResponse = await getV3Events({ | ||
| client: publicClient, | ||
| query: { |
There was a problem hiding this comment.
Using Promise.all means a single failure when fetching events for one featured collection rejects the whole operation and flips the page into the error state, even if other collections (and the regular events list) could still render. Consider Promise.allSettled or per-collection try/catch so one collection doesn’t take down the entire frontpage.
| import { EventCollectionDto, EventDto } from "@/lib/eventuras-public-sdk"; | ||
| import { |
There was a problem hiding this comment.
This introduces duplicate imports from the same module and switches to double quotes, which is inconsistent with the surrounding file and likely to fail lint/formatting. Prefer a single import from @/lib/eventuras-public-sdk (using import type for types) and keep quote style consistent.
| import { EventCollectionDto, EventDto } from "@/lib/eventuras-public-sdk"; | |
| import { | |
| import { | |
| type EventCollectionDto, | |
| type EventDto, |
| public bool? Featured { get; set; } | ||
|
|
||
| public bool IncludePastCollections { get; set; } | ||
|
|
||
| public EventCollectionFilter ToFilter() |
There was a problem hiding this comment.
Featured is exposed as a nullable boolean query parameter, but it’s mapped to FeaturedOnly and the filter only applies when the value is true. If a client passes Featured=false, the current implementation returns all collections rather than only non-featured ones. Either rename this to something like FeaturedOnly (non-nullable) to reflect the semantics, or handle the false case explicitly (filter out featured collections).
a6ae419 to
20379e8
Compare
|



Adds featured event collections support end-to-end (API filtering + SDK types) and updates the web frontpage/collection pages to render featured collections with events grouped by category.