[Fix] Video filtering #8765
[Fix] Video filtering #8765
Conversation
Adds a new API endpoint to retrieve the count of videos. Updates the video service to use the new API endpoint for checking video availability. This improves performance by only retrieving the count instead of the entire list of videos. Adds update video API endpoint to update video metadata.
Moves RxJS operators import to be with other RxJS imports. This improves code readability and consistency.
- Updated `VideoService` in `video.service.ts` to refine selection logic, prioritizing `selectedOrganization` over `organizationId`. - Enhanced `GetVideoCountQueryHandler` in `get-video-count.handler.ts` by introducing permission checks for `CHANGE_SELECTED_EMPLOYEE`, ensuring queries respect user permissions.
- Introduced `moment-timezone` for converting start and end dates to UTC. - Updated query to dynamically filter videos based on provided `startDate`, `endDate`, and `timeZone`. - Implemented dynamic WHERE clause with optional employee filtering using `TypeORM`'s `In` and `Between` utilities.
Modify the query to conditionally include the recordedAt filter only when both startDate and endDate are provided. This change enhances the query flexibility by avoiding unnecessary date filtering.
- Updated import statements for DTOs and models in videos.controller.ts - Changed the Delete method to accept an optional IDeleteVideo parameter
- Updated the VideoEffects update logic for better state management - Refactored UpdateVideoHandler to include description in video updates - Modified UpdateVideoCommand and UpdateVideoDTO for simplified DTO structure - Revised VideosController to adapt to new UpdateVideoCommand constructor
- Ensures that the video store is cleared before fetching new video data when the filter changes.
- Disable log type, source, and activity level filters in the video page filter component. - This change simplifies the available filters for a streamlined user experience.
WalkthroughThe changes improve various aspects of the video modules by updating state management, UI behavior, service logic, and command–query flows. In the videos-ui package, the video state update now directly stores the updated video; video page components are modified to set filter properties and clear the video list on initialization; and routing has been adjusted. In the videos package, commands and DTOs have been refined to update only essential fields, and new query classes and handlers are added to support video counting along with controller endpoint updates. Changes
Sequence Diagram(s)sequenceDiagram
participant C as Client
participant VC as VideosController
participant Q as GetVideoCountQuery
participant QH as GetVideoCountQueryHandler
participant VS as VideoService
participant DB as Database
C->>VC: GET /videos/count?options
VC->>Q: Create GetVideoCountQuery(options)
Q->>QH: Dispatch query
QH->>VS: Call getCount(options)
VS->>DB: Query video count
DB-->>VS: Return count result
VS-->>QH: Return count
QH-->>VC: Return count
VC-->>C: Send count response
sequenceDiagram
participant C as Client
participant VC as VideosController
participant CMD as UpdateVideoCommand
participant UH as UpdateVideoHandler
participant VS as VideoService
C->>VC: PUT /videos/{id} with update data
VC->>CMD: Instantiate update command (id, input)
CMD->>UH: Dispatch update command
UH->>VS: Call update({...})
VS-->>UH: Return updated video
UH-->>VC: Return updated video
VC-->>C: Send updated video response
Suggested reviewers
Poem
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
packages/plugins/videos-ui/src/lib/shared/services/video.service.ts (2)
34-38: Method getCount lacks dedicated error handling.
Although the caller can catch errors, consider adding basic error handling here if other consumers call getCount directly. This helps ensure robust error propagation outside of the isAvailable$ flow.
41-52: Consider dynamic re-evaluation for isAvailable$.
The logic is straightforward, but be mindful that it only evaluates once when the getter is subscribed to. If the organization or tenant changes in the Store at runtime, re-subscribing or wrapping these lookups in real-time reactive streams (e.g., store.selectedOrganization$) could be beneficial to avoid stale data.packages/plugins/videos/src/lib/queries/handlers/get-videos.handler.ts (2)
3-4: Evaluate choice of moment-timezone.
moment-timezone is widely used and reliable but can be large. Consider dayjs or date-fns if bundle size and performance with time zones become concerns.
33-57: Consider indexing and updating comment references.
- If the dataset grows large, ensure recordedAt is indexed in the DB for efficient range queries.
- The inline comment mentions “valueDate” but the code uses “recordedAt.” Update the comment to match the actual property or vice versa.
packages/plugins/videos/src/lib/queries/handlers/get-video-count.handler.ts (1)
19-34: Consider enhancing type safety for query options.While the implementation is correct, the type safety could be improved.
Consider this improvement:
- const { options } = query || {}; - const { organizationId, tenantId } = options; + const { options = {} } = query; + const { organizationId, tenantId } = options; + + if (!organizationId || !tenantId) { + throw new Error('organizationId and tenantId are required'); + }packages/plugins/videos/src/lib/dto/count-video.dto.ts (1)
51-52: Consider adding date transformation.While the date validation is correct, adding transformation would ensure consistent date handling.
Add Transform decorator to automatically convert string dates to Date objects:
+import { Transform } from 'class-transformer'; @IsOptional() @IsISO8601({ strict: true }, { message: 'startDate must be a valid ISO 8601 date string' }) +@Transform(({ value }) => value ? new Date(value) : value) -startDate: Date | string; +startDate: Date; @IsOptional() @IsISO8601({ strict: true }, { message: 'endDate must be a valid ISO 8601 date string' }) +@Transform(({ value }) => value ? new Date(value) : value) -endDate: Date | string; +endDate: Date;Also applies to: 68-69
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
packages/plugins/videos-ui/src/lib/+state/video.effect.ts(1 hunks)packages/plugins/videos-ui/src/lib/pages/video-page/video-page.component.html(1 hunks)packages/plugins/videos-ui/src/lib/pages/video-page/video-page.component.ts(1 hunks)packages/plugins/videos-ui/src/lib/shared/services/video.service.ts(2 hunks)packages/plugins/videos-ui/src/lib/video-ui-routing.module.ts(1 hunks)packages/plugins/videos/src/lib/commands/handlers/update-video.handler.ts(1 hunks)packages/plugins/videos/src/lib/commands/update-video.command.ts(1 hunks)packages/plugins/videos/src/lib/dto/count-video.dto.ts(1 hunks)packages/plugins/videos/src/lib/dto/update-video.dto.ts(1 hunks)packages/plugins/videos/src/lib/queries/get-video-count.query.ts(1 hunks)packages/plugins/videos/src/lib/queries/handlers/get-video-count.handler.ts(1 hunks)packages/plugins/videos/src/lib/queries/handlers/get-videos.handler.ts(2 hunks)packages/plugins/videos/src/lib/queries/handlers/index.ts(1 hunks)packages/plugins/videos/src/lib/videos.controller.ts(5 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: build
- GitHub Check: test
- GitHub Check: test
🔇 Additional comments (19)
packages/plugins/videos-ui/src/lib/shared/services/video.service.ts (2)
5-6: Smooth integration of new imports.
No concerns with the additional imports of Store and rxjs operators. This cleanly aligns with Angular best practices for state management and observables.
14-14: Validate Store injection.
While injecting the Store is appropriate, ensure that the module providing Store is properly configured and that the Store object is guaranteed to be defined at runtime.packages/plugins/videos/src/lib/queries/handlers/get-videos.handler.ts (2)
6-6: New IVideo import appears correct.
No issues with referencing the IVideo interface in this context.
25-31: Destructuring filter parameters is clear and concise.
The default values for employeeIds and timeZone are a nice touch. This makes the handler resilient to undefined inputs.packages/plugins/videos/src/lib/dto/update-video.dto.ts (1)
3-3: Confirm removal of ID and file fields.
This DTO no longer contains the previously declared properties (id, file). Verify that no other parts of the code rely on them.✅ Verification successful
🏁 Script executed:
#!/bin/bash # Search the codebase for references to removed fields in UpdateVideoDTO rg -A 5 'UpdateVideoDTO'Length of output: 2086
Verification of removed "id" and "file" fields in UpdateVideoDTO
The search confirms that the UpdateVideoDTO now extends BaseVideoDTO without including the removed properties. The only reference to an "id" comes from UpdateVideoCommand—which accepts the id as a separate constructor argument—and there is no evidence that any part of the system is still relying on UpdateVideoDTO to contain id or file fields.
packages/plugins/videos/src/lib/queries/get-video-count.query.ts (1)
1-8: LGTM! Well-structured query implementation.The implementation follows CQRS pattern best practices with clear type definition and proper encapsulation of query parameters.
packages/plugins/videos/src/lib/queries/handlers/index.ts (1)
1-1: LGTM! Consistent handler registration.The GetVideoCountQueryHandler is properly registered alongside existing handlers.
Also applies to: 5-5
packages/plugins/videos/src/lib/commands/update-video.command.ts (1)
3-3: LGTM! Improved type safety with explicit ID parameter.The command structure is enhanced by requiring an explicit ID parameter, making the command more type-safe and self-documenting.
Also applies to: 7-7
packages/plugins/videos/src/lib/commands/handlers/update-video.handler.ts (2)
10-18: LGTM! Clear and helpful documentation.The JSDoc comments effectively document the purpose and behavior of the execute method.
21-30: Verify the intentional removal of size, file, and duration fields.The update operation has been simplified to only update title and description. Please confirm if this is intentional and that size, file, and duration fields are no longer needed for updates.
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Search for usages of removed fields in the codebase echo "Searching for references to removed fields..." rg -A 5 "size|file|duration" packages/plugins/videos/Length of output: 21243
UPDATE HANDLER: Confirmed Simplified Update is Intentional
The search results indicate that the fields (size, file, and duration) are still present and used in other parts of the videos feature (such as during creation and entity validation), but they are omitted from the update command. This suggests that the update operation is intentionally limited to modifying only the title and description, while keeping the original file and its metadata intact.
packages/plugins/videos/src/lib/queries/handlers/get-video-count.handler.ts (1)
11-18: LGTM! Well-documented method.The documentation clearly describes the purpose, parameters, and return type of the execute method.
packages/plugins/videos-ui/src/lib/video-ui-routing.module.ts (1)
39-39: LGTM! Routing configuration changes look good.The changes to disable the date selector and enable route reuse for the video detail page are appropriate.
Also applies to: 51-51
packages/plugins/videos/src/lib/dto/count-video.dto.ts (1)
4-70: LGTM! Well-structured DTO with comprehensive validation.The DTO is well-documented with proper validation rules and API property decorators.
packages/plugins/videos-ui/src/lib/pages/video-page/video-page.component.ts (1)
56-56: LGTM! Clean state management.Resetting the videos array before fetching new data ensures a clean state and prevents stale data issues.
packages/plugins/videos-ui/src/lib/+state/video.effect.ts (1)
108-109: LGTM! Good improvement to state management.The addition of the
videoproperty to the state update ensures consistency between the list and individual video states.packages/plugins/videos/src/lib/videos.controller.ts (3)
174-195: LGTM! Well-structured count endpoint.The implementation includes:
- Comprehensive API documentation
- Proper validation using ValidationPipe
- Clear error responses
207-232: LGTM! Well-implemented update endpoint.The implementation includes:
- Comprehensive API documentation
- Input validation
- UUID validation for the ID parameter
292-292: LGTM! Improved delete endpoint signature.Making the options parameter optional with proper typing improves the API's flexibility.
packages/plugins/videos-ui/src/lib/pages/video-page/video-page.component.html (1)
10-12: LGTM! Improved filter configuration.Disabling irrelevant filters (log type, source, and activity level) streamlines the UI for video filtering.
|
View your CI Pipeline Execution ↗ for commit 92474d0.
☁️ Nx Cloud last updated this comment at |
PR
Please note: we will close your PR without comment if you do not check the boxes above and provide ALL requested information.
Summary by CodeRabbit
New Features
• Introduced a video count feature that displays the total number of videos based on custom filters (such as date range and organization).
• Launched a streamlined video update experience, allowing users to easily modify key details like title and description.
Enhancements
• Adjusted video page filters by disabling options (log type, source, and activity level) for a cleaner, more focused search experience.
• Refined video listing filters to better sort and display content based on selected criteria.