Admin/text and video block and task#94#165
Conversation
WalkthroughIntroduces create flows for Video and Term: new DTOs, MediatR commands/handlers, and validators. Updates VideoController with a POST endpoint. Refactors Video DTO structure (adds Title; relocates class), removes legacy DTO file. Adjusts DAL entities (Video.Url non-nullable; Term expanded). Adds mappings, tests, and project references. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant API as VideoController
participant Med as Mediator
participant H as CreateVideoHandler
participant Repo as RepositoryWrapper
participant DB as DB
participant Map as AutoMapper
Client->>API: POST /api/video (CreateVideoDTO)
API->>Med: Send(CreateVideoCommand)
Med->>H: Handle(command)
H->>Map: Map DTO -> Entity(Video)
alt Validation fails
H-->>Med: Result.Fail(error)
Med-->>API: Failure
API-->>Client: 400/422 with errors
else Valid
H->>Repo: VideoRepository.CreateAsync(entity)
H->>Repo: SaveChangesAsync()
Repo->>DB: Persist
DB-->>Repo: Saved
H->>Map: Map Entity -> VideoDTO
H-->>Med: Result.Ok(VideoDTO)
Med-->>API: Success
API-->>Client: 200 with VideoDTO
end
sequenceDiagram
autonumber
actor Caller
participant Med as Mediator
participant H as CreateTermHandler
participant Repo as RepositoryWrapper
participant DB as DB
participant Map as AutoMapper
Caller->>Med: Send(CreateTermCommand(TermCreateDTO))
Med->>H: Handle(command)
H->>Map: Map DTO -> Term
alt Mapping/save fails
H-->>Med: Result.Fail(error)
Med-->>Caller: Failure
else Success
H->>Repo: TermRepository.CreateAsync(term)
H->>Repo: SaveChangesAsync()
Repo->>DB: Persist
DB-->>Repo: Saved
H->>Map: Map Term -> TermDTO
H-->>Med: Result.Ok(TermDTO)
Med-->>Caller: Success
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
Pre-merge checks (2 warnings, 1 inconclusive)❌ Failed Checks (2 warnings, 1 inconclusive)
Poem
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Flag potential breaking changes that are not documented:
1. Identify changes to public APIs/exports, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints (including removed/renamed items and changes to types, required params, return values, defaults, or behavior).
2. Ignore purely internal/private changes (e.g., code not exported from package entry points or marked internal).
3. Verify documentation exists: a "Breaking Change" section in the PR description and updates to CHANGELOG.md.✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Streetcode/Streetcode.XUnitTest/Streetcode.XUnitTest.csproj (1)
50-53: Remove redundant ProjectReference group in Streetcode.XUnitTest.csproj
The BLL and DAL references appear at lines 11–12 and again at 51–52; remove one occurrence so each project is referenced only once.
🧹 Nitpick comments (20)
Streetcode/Streetcode.DAL/Entities/Media/Video.cs (1)
18-20: Constrain Url at the model level (length/format).Add a reasonable max length and (optionally) URL format attribute to catch invalid data early.
[Required] - public string Url { get; set; } + [MaxLength(2048)] + [Url] + public string Url { get; set; }Streetcode/Streetcode.BLL/DTO/Media/Video/VideoDTO.cs (1)
8-8: Align DTO nullability with DAL: make Url non-nullable.DAL now enforces non-null
Url; returningstring?in DTO leaks inconsistency and forces extra null checks in consumers.- public string? Url { get; set; } + public string Url { get; set; }Streetcode/Streetcode.DAL/Entities/Streetcode/TextContent/RelatedTerm.cs (1)
13-16: Required + nullable string is inconsistent.
[Required]onstring? Wordcontradicts nullability. Make it non-nullable to match intent and DB constraint.- [Required] - [MaxLength(50)] - public string? Word { get; set; } + [Required] + [MaxLength(50)] + public string Word { get; set; }Streetcode/Streetcode.WebApi/Streetcode.WebApi.csproj (1)
62-64: Duplicate Streetcode.BLL ProjectReference — keep a single reference.Two identical references exist; remove one.
<ItemGroup> - <ProjectReference Include="..\Streetcode.BLL\Streetcode.BLL.csproj" /> + <ProjectReference Include="..\Streetcode.BLL\Streetcode.BLL.csproj" /> </ItemGroup> - <ItemGroup> - <ProjectReference Include="..\Streetcode.BLL\Streetcode.BLL.csproj" /> - </ItemGroup> + <!-- removed duplicate ProjectReference -->Also applies to: 16-18
Streetcode/Streetcode.BLL/Validators/TextContent/CreateTermValidator.cs (1)
4-4: Namespace consistency with folder structure.Other validators typically live under
...Validators...; consider pluralizing for consistency and discoverability.-namespace Streetcode.BLL.Validator.Streetcode.Term.Create; +namespace Streetcode.BLL.Validators.Streetcode.Term.Create;Streetcode/Streetcode.BLL/MediatR/Streetcode/Text/Create/CreateTermCommand.cs (1)
7-7: Align command payload property naming with CreateVideo for consistencyConsider renaming
TermtoCreateTermRequestto mirrorCreateVideoCommandand keep pipelines uniform.-public record CreateTermCommand(TermCreateDTO Term) : IRequest<Result<TermDTO>>; +public record CreateTermCommand(TermCreateDTO CreateTermRequest) : IRequest<Result<TermDTO>>;Streetcode/Streetcode.BLL/Validators/Media/Video/CreateVideoCommandValidator.cs (2)
4-4: Namespace should match folder and existing conventionUse
Validators(plural) to stay consistent and simplify assembly scanning.-namespace Streetcode.BLL.Validator.Media.Video; +namespace Streetcode.BLL.Validators.Media.Video;
10-17: Tighten rules, stop on first failure, and provide clear messagesPrevent whitespace-only values, stop subsequent checks after a failure, and add explicit messages.
-RuleFor(c => c.CreateVideoRequest.Title) - .NotEmpty() - .MaximumLength(100); +RuleFor(c => c.CreateVideoRequest.Title) + .Cascade(CascadeMode.Stop) + .NotEmpty().WithMessage("Title is required.") + .Must(t => !string.IsNullOrWhiteSpace(t)).WithMessage("Title cannot be whitespace.") + .MaximumLength(100).WithMessage("Title must not exceed 100 characters."); -RuleFor(c => c.CreateVideoRequest.Url) - .NotEmpty() - .Must(url => Uri.IsWellFormedUriString(url, UriKind.Absolute)); +RuleFor(c => c.CreateVideoRequest.Url) + .Cascade(CascadeMode.Stop) + .NotEmpty().WithMessage("URL is required.") + .Must(url => Uri.IsWellFormedUriString(url, UriKind.Absolute)) + .WithMessage("URL must be absolute and well-formed.");Streetcode/Streetcode.BLL/DTO/TextContent/TermCreateDTO.cs (1)
5-10: Guard against null/whitespace in Title/Description via validatorDTO is fine; ensure a corresponding validator enforces non-empty values and reasonable length. If not present, add it.
I can draft
CreateTermValidatormirroring the Video rules if you’d like.Streetcode/Streetcode.BLL/MediatR/Streetcode/Text/Create/CreateTermHandler.cs (2)
36-38: Pass CancellationToken to repo callsPropagate
cancellationTokento support request aborts.- await _repository.TermRepository.CreateAsync(entity); - var result = await _repository.SaveChangesAsync(); + await _repository.TermRepository.CreateAsync(entity, cancellationToken); + var result = await _repository.SaveChangesAsync(cancellationToken);
41-43: Optionally verify DTO mapping resultVery unlikely with a valid profile, but guarding improves resilience.
- var dto = _mapper.Map<TermDTO>(entity); - return Result.Ok(dto); + var dto = _mapper.Map<TermDTO>(entity); + return dto is not null ? Result.Ok(dto) : Result.Fail("Failed to map Term entity to DTO.");Streetcode/Streetcode.BLL/MediatR/Media/Video/Create/CreateVideoHandler.cs (3)
4-4: Avoid dependency on Microsoft.IdentityModel.Tokens for string checksUse BCL
string.IsNullOrWhiteSpaceinstead ofIsNullOrEmpty()extension; drop this using.-using Microsoft.IdentityModel.Tokens;
29-50: Consider passing CancellationToken to repo callsImproves cancellation responsiveness.
- var entity = await _repositoryWrapper.VideoRepository.CreateAsync(newVideo); - var resultIsSuccess = await _repositoryWrapper.SaveChangesAsync() > 0; + var entity = await _repositoryWrapper.VideoRepository.CreateAsync(newVideo, cancellationToken); + var resultIsSuccess = await _repositoryWrapper.SaveChangesAsync(cancellationToken) > 0;
29-50: Do we need a uniqueness/duplication guard (e.g., Url per StreetcodeId)?Tests set up
GetFirstOrDefaultAsyncbut the handler doesn’t check duplicates. Confirm desired behavior and, if needed, add a pre-create existence check.I can extend the handler to query for an existing video by
StreetcodeId+Url(or Title) and return a 409-style failure.Streetcode/Streetcode.XUnitTest/BLL/MediatR/Media/Video/CreateVideoHandlerTests.cs (1)
125-155: Update URL error assertion and drop unused repo setupsAlign message with handler and remove unused
GetFirstOrDefaultAsyncsetups in these failure-path tests.- _repositoryWrapperMock - .Setup(r => r.VideoRepository.GetFirstOrDefaultAsync(It.IsAny<Expression<Func<Entity, bool>>>(), default)) - .ReturnsAsync((Entity)null); ... - result.Errors.First().Message.Should().Contain("��������� �� ���� � ����'�������."); + result.Errors.First().Message.Should().Contain("URL is required and must be absolute and well-formed.");Streetcode/Streetcode.DAL/Entities/Streetcode/TextContent/Term.cs (5)
1-1: Remove unused using.
using System;isn’t used.-using System;
8-10: Add a uniqueness/index for fast lookups and to prevent duplicates per Streetcode.If business logic expects a Term title to be unique within a Streetcode, add a composite unique index.
Add the attribute and ensure
using Microsoft.EntityFrameworkCore;is present:+using Microsoft.EntityFrameworkCore; @@ - [Table("terms", Schema = "streetcode")] + [Index(nameof(StreetcodeId), nameof(Title), IsUnique = true)] + [Table("terms", Schema = "streetcode")] public class Term
15-17: [Required] is redundant on non-nullable reference type.EF Core infers non-nullability from
string(with NRTs enabled). Consider removing[Required]; keep max length.- [Required] [MaxLength(50)] public string Title { get; set; } = string.Empty;If you also want to prevent empty/whitespace values, enforce via FluentValidation or add
[MinLength(1)].
19-21: Confirm whether Description must be mandatory.If optional, make it nullable and drop
[Required]. If mandatory, consider guarding against empty strings.Option A (optional field):
- [Required] - [MaxLength(500)] - public string Description { get; set; } = string.Empty; + [MaxLength(500)] + public string? Description { get; set; }Option B (required and non-empty):
- [Required] + [Required] + [MinLength(1)] [MaxLength(500)] public string Description { get; set; } = string.Empty;
26-26: Prefer HashSet and/or virtual for EF collections.For better set semantics and proxy support, consider:
- public ICollection<RelatedTerm> RelatedTerms { get; set; } = new List<RelatedTerm>(); + public virtual ICollection<RelatedTerm> RelatedTerms { get; set; } = new HashSet<RelatedTerm>();Note: Keep
virtualonly if lazy-loading proxies are used.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
Streetcode/Streetcode.BLL/DTO/Media/Video/CreateVideoDTO.cs(1 hunks)Streetcode/Streetcode.BLL/DTO/Media/Video/VideoDTO.cs(1 hunks)Streetcode/Streetcode.BLL/DTO/Media/VideoDTO.cs(0 hunks)Streetcode/Streetcode.BLL/DTO/TextContent/TermCreateDTO.cs(1 hunks)Streetcode/Streetcode.BLL/Mapping/Streetcode/TextContent/TermProfile.cs(1 hunks)Streetcode/Streetcode.BLL/MediatR/Media/Video/Create/CreateVideoCommand.cs(1 hunks)Streetcode/Streetcode.BLL/MediatR/Media/Video/Create/CreateVideoHandler.cs(1 hunks)Streetcode/Streetcode.BLL/MediatR/Streetcode/Text/Create/CreateTermCommand.cs(1 hunks)Streetcode/Streetcode.BLL/MediatR/Streetcode/Text/Create/CreateTermHandler.cs(1 hunks)Streetcode/Streetcode.BLL/Validators/Media/Video/CreateVideoCommandValidator.cs(1 hunks)Streetcode/Streetcode.BLL/Validators/TextContent/CreateTermValidator.cs(1 hunks)Streetcode/Streetcode.DAL/Entities/Media/Video.cs(1 hunks)Streetcode/Streetcode.DAL/Entities/Streetcode/TextContent/RelatedTerm.cs(2 hunks)Streetcode/Streetcode.DAL/Entities/Streetcode/TextContent/Term.cs(1 hunks)Streetcode/Streetcode.WebApi/Controllers/Media/VideoController.cs(2 hunks)Streetcode/Streetcode.WebApi/Streetcode.WebApi.csproj(1 hunks)Streetcode/Streetcode.XUnitTest/BLL/MediatR/Media/Video/CreateVideoHandlerTests.cs(1 hunks)Streetcode/Streetcode.XUnitTest/Streetcode.XUnitTest.csproj(1 hunks)
💤 Files with no reviewable changes (1)
- Streetcode/Streetcode.BLL/DTO/Media/VideoDTO.cs
🧰 Additional context used
🧬 Code graph analysis (9)
Streetcode/Streetcode.BLL/Mapping/Streetcode/TextContent/TermProfile.cs (1)
Streetcode/Streetcode.BLL/DTO/TextContent/TermCreateDTO.cs (1)
TermCreateDTO(3-10)
Streetcode/Streetcode.BLL/DTO/Media/Video/CreateVideoDTO.cs (1)
Streetcode/Streetcode.BLL/DTO/Media/Video/VideoDTO.cs (1)
VideoDTO(3-10)
Streetcode/Streetcode.BLL/MediatR/Media/Video/Create/CreateVideoCommand.cs (3)
Streetcode/Streetcode.BLL/DTO/Media/Video/CreateVideoDTO.cs (1)
CreateVideoDTO(3-5)Streetcode/Streetcode.BLL/MediatR/Media/Video/Create/CreateVideoHandler.cs (2)
Result(52-73)Result(75-79)Streetcode/Streetcode.BLL/DTO/Media/Video/VideoDTO.cs (1)
VideoDTO(3-10)
Streetcode/Streetcode.BLL/MediatR/Streetcode/Text/Create/CreateTermCommand.cs (1)
Streetcode/Streetcode.BLL/DTO/TextContent/TermCreateDTO.cs (1)
TermCreateDTO(3-10)
Streetcode/Streetcode.BLL/MediatR/Streetcode/Text/Create/CreateTermHandler.cs (2)
Streetcode/Streetcode.BLL/Services/Logging/LoggerService.cs (1)
LogError(35-40)Streetcode/Streetcode.BLL/MediatR/Streetcode/RelatedTerm/Create/CreateRelatedTermHandler.cs (1)
CreateRelatedTermHandler(12-71)
Streetcode/Streetcode.BLL/MediatR/Media/Video/Create/CreateVideoHandler.cs (2)
Streetcode/Streetcode.BLL/DTO/Media/Video/VideoDTO.cs (1)
VideoDTO(3-10)Streetcode/Streetcode.BLL/Services/Logging/LoggerService.cs (1)
LogError(35-40)
Streetcode/Streetcode.XUnitTest/BLL/MediatR/Media/Video/CreateVideoHandlerTests.cs (2)
Streetcode/Streetcode.BLL/MediatR/Media/Video/Create/CreateVideoHandler.cs (3)
CreateVideoHandler(13-80)CreateVideoHandler(19-27)Task(29-50)Streetcode/Streetcode.BLL/DTO/Media/Video/CreateVideoDTO.cs (1)
CreateVideoDTO(3-5)
Streetcode/Streetcode.WebApi/Controllers/Media/VideoController.cs (2)
Streetcode/Streetcode.BLL/MediatR/Media/Video/Create/CreateVideoHandler.cs (1)
Task(29-50)Streetcode/Streetcode.BLL/DTO/Media/Video/CreateVideoDTO.cs (1)
CreateVideoDTO(3-5)
Streetcode/Streetcode.DAL/Entities/Streetcode/TextContent/Term.cs (1)
Streetcode/Streetcode.DAL/Entities/Streetcode/TextContent/RelatedTerm.cs (1)
Table(6-19)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Build and analyze
- GitHub Check: Build and analyze
- GitHub Check: Build and analyze
🔇 Additional comments (8)
Streetcode/Streetcode.DAL/Entities/Media/Video.cs (1)
18-20: Verify EF migration for non-nullable Url
Could not locate a migration alteringmedia.videos.Urlto non-nullable; ensure you’ve added an EFAlterColumnmigration and handled any existing NULL values via backfill or data cleanup.Streetcode/Streetcode.BLL/Mapping/Streetcode/TextContent/TermProfile.cs (1)
12-12: Mapping for TermCreateDTO → Term looks good.Streetcode/Streetcode.BLL/MediatR/Media/Video/Create/CreateVideoCommand.cs (1)
7-7: LGTM: command contract is clear and consistentNo issues spotted.
Streetcode/Streetcode.BLL/MediatR/Streetcode/Text/Create/CreateTermHandler.cs (1)
25-34: Good null-mapping guard and error loggingMapping null-check and logging look correct.
Streetcode/Streetcode.XUnitTest/BLL/MediatR/Media/Video/CreateVideoHandlerTests.cs (2)
35-68: Happy-path coverage looks goodMocks and assertions correctly validate create flow success.
71-89: Null DTO case: OKFailure path is exercised and assertions are clear.
Streetcode/Streetcode.DAL/Entities/Streetcode/TextContent/Term.cs (2)
8-13: LGTM on entity key mapping.
[Table],[Key], and identity setup look correct.
23-24: Confirm and fix the misapplied ForeignKey attribute
The[ForeignKey(nameof(Streetcode))]onStreetcodeIdwill be ignored ifTermdoesn’t declare apublic Streetcode? Streetcodenavigation andStreetcodedoesn’t exposeICollection<Term> Terms. Manual verification required—confirm those navs exist (or not), then either remove the attribute to rely on conventions or add the missing navigation (annotated or by naming convention).
| public class CreateVideoDTO : VideoDTO | ||
| { | ||
| } |
There was a problem hiding this comment.
Create DTO must not expose Id — avoid overposting/identity insert issues.
Inheriting from VideoDTO brings Id into the create contract. A client can set it, leading to inserts with explicit identity or unexpected behavior in EF.
Refactor CreateVideoDTO to its own shape without Id:
-namespace Streetcode.BLL.DTO.Media.Video
-{
- public class CreateVideoDTO : VideoDTO
- {
- }
-}
+namespace Streetcode.BLL.DTO.Media.Video
+{
+ public class CreateVideoDTO
+ {
+ public required string Title { get; set; } // validator enforces max 100
+ public string? Description { get; set; }
+ public required string Url { get; set; }
+ public int StreetcodeId { get; set; }
+ }
+}If keeping inheritance temporarily, at minimum ensure AutoMapper ignores Id in the create mapping: ForMember(d => d.Id, opt => opt.Ignore());
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public class CreateVideoDTO : VideoDTO | |
| { | |
| } | |
| namespace Streetcode.BLL.DTO.Media.Video | |
| { | |
| public class CreateVideoDTO | |
| { | |
| public required string Title { get; set; } // validator enforces max 100 | |
| public string? Description { get; set; } | |
| public required string Url { get; set; } | |
| public int StreetcodeId { get; set; } | |
| } | |
| } |
🤖 Prompt for AI Agents
In Streetcode/Streetcode.BLL/DTO/Media/Video/CreateVideoDTO.cs around lines 3-5,
the CreateVideoDTO currently inherits VideoDTO and therefore exposes Id which
can cause overposting/identity insert issues; change CreateVideoDTO to a
standalone DTO type that declares the create-time properties (title, url,
duration, etc.) but omits Id, and update any usages to the new shape; if you
must keep inheritance temporarily, update the AutoMapper profile for the create
mapping to explicitly ignore Id (ForMember(d => d.Id, opt => opt.Ignore())) so
incoming Id values are not mapped.
| private Result<VideoDTO>? VideoValidation(CreateVideoCommand request, Entity? newVideo) | ||
| { | ||
| if (newVideo == null) | ||
| { | ||
| const string errorMsg = "Cannot convert null to Video."; | ||
| return LogAndFail(request, errorMsg); | ||
| } | ||
|
|
||
| if (newVideo.Title.Length > 100) | ||
| { | ||
| const string errorMsg = "��������� ���� �� ���� ���� ����� 100 �������."; | ||
| return LogAndFail(request, errorMsg); | ||
| } | ||
|
|
||
| if (newVideo.Url.IsNullOrEmpty()) | ||
| { | ||
| const string errorMsg = "��������� �� ���� � ����'�������."; | ||
| return LogAndFail(request, errorMsg); | ||
| } | ||
|
|
||
| return null; | ||
| } |
There was a problem hiding this comment.
Fix potential NRE, replace mojibake messages, and use BCL null/whitespace checks
newVideo.Title.Length will throw if Title is null; error texts appear corrupted; and IsNullOrEmpty() extension is unnecessary. Use string.IsNullOrWhiteSpace, check length safely, and provide clear messages.
- private Result<VideoDTO>? VideoValidation(CreateVideoCommand request, Entity? newVideo)
+ private Result<VideoDTO>? VideoValidation(CreateVideoCommand request, Entity? newVideo)
{
if (newVideo == null)
{
- const string errorMsg = "Cannot convert null to Video.";
+ const string errorMsg = "Cannot convert null to Video.";
return LogAndFail(request, errorMsg);
}
- if (newVideo.Title.Length > 100)
+ if (string.IsNullOrWhiteSpace(newVideo.Title) || newVideo.Title!.Length > 100)
{
- const string errorMsg = "��������� ���� �� ���� ���� ����� 100 �������.";
+ const string errorMsg = "Title is required and must not exceed 100 characters.";
return LogAndFail(request, errorMsg);
}
- if (newVideo.Url.IsNullOrEmpty())
+ if (string.IsNullOrWhiteSpace(newVideo.Url) || !Uri.IsWellFormedUriString(newVideo.Url, UriKind.Absolute))
{
- const string errorMsg = "��������� �� ���� � ����'�������.";
+ const string errorMsg = "URL is required and must be absolute and well-formed.";
return LogAndFail(request, errorMsg);
}
return null;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private Result<VideoDTO>? VideoValidation(CreateVideoCommand request, Entity? newVideo) | |
| { | |
| if (newVideo == null) | |
| { | |
| const string errorMsg = "Cannot convert null to Video."; | |
| return LogAndFail(request, errorMsg); | |
| } | |
| if (newVideo.Title.Length > 100) | |
| { | |
| const string errorMsg = "Çàãîëîâîê â³äåî íå ìîæå áóòè á³ëüøå 100 ñèìâîë³â."; | |
| return LogAndFail(request, errorMsg); | |
| } | |
| if (newVideo.Url.IsNullOrEmpty()) | |
| { | |
| const string errorMsg = "Ïîñèëàííÿ íà â³äåî º îáîâ'ÿçêîâèì."; | |
| return LogAndFail(request, errorMsg); | |
| } | |
| return null; | |
| } | |
| private Result<VideoDTO>? VideoValidation(CreateVideoCommand request, Entity? newVideo) | |
| { | |
| if (newVideo == null) | |
| { | |
| const string errorMsg = "Cannot convert null to Video."; | |
| return LogAndFail(request, errorMsg); | |
| } | |
| if (string.IsNullOrWhiteSpace(newVideo.Title) || newVideo.Title!.Length > 100) | |
| { | |
| const string errorMsg = "Title is required and must not exceed 100 characters."; | |
| return LogAndFail(request, errorMsg); | |
| } | |
| if (string.IsNullOrWhiteSpace(newVideo.Url) || !Uri.IsWellFormedUriString(newVideo.Url, UriKind.Absolute)) | |
| { | |
| const string errorMsg = "URL is required and must be absolute and well-formed."; | |
| return LogAndFail(request, errorMsg); | |
| } | |
| return null; | |
| } |
🤖 Prompt for AI Agents
In Streetcode/Streetcode.BLL/MediatR/Media/Video/Create/CreateVideoHandler.cs
around lines 52-73, the validation reads properties without null-safety and
contains corrupted messages; update the method to first null-check newVideo,
then use string.IsNullOrWhiteSpace(newVideo.Title) or safe access (e.g. if
(string.IsNullOrWhiteSpace(newVideo.Title)) ...) before checking length, use
newVideo.Title.Length only after ensuring Title is not null, replace
IsNullOrEmpty() extension with string.IsNullOrWhiteSpace(newVideo.Url), and
replace the mojibake error texts with clear English messages (e.g. "Title is
required", "Title must be at most 100 characters", "URL is required"); keep
returning LogAndFail(request, errorMsg) on failures.
| RuleFor(cmd => cmd.Term.Title) | ||
| .NotEmpty() | ||
| .MaximumLength(200); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Confirm DB constraint parity for Title length (200).
Ensure Term.Title in DAL has [MaxLength(200)] (or equivalent) to avoid runtime DB truncation errors.
🏁 Script executed:
#!/bin/bash
# Verify DAL constraint for Term.Title
rg -nP '(class\\s+Term\\b|\\[MaxLength\\(\\s*200\\s*\\)\\])' -C3 --type=csLength of output: 25219
🏁 Script executed:
#!/bin/bash
# Locate the Term entity class in DAL and verify Title length annotation
echo "Searching for Term entity definition…"
rg -nP 'public\s+class\s+Term\b' -C3 --type=cs
echo
echo "Checking for [MaxLength(200)] on Title property in Term entity…"
TERM_FILE=$(rg -lP 'public\s+class\s+Term\b' --type=cs)
rg -nP '\[MaxLength\(\s*200\s*\)\]' -C3 "$TERM_FILE"Length of output: 788
Add DB constraint for Term.Title
The Title property in Streetcode/Streetcode.DAL/Entities/Streetcode/TextContent/Term.cs lacks a [MaxLength(200)] attribute (or equivalent EF Core fluent‐API .HasMaxLength(200)), mismatching the BLL validator’s 200-char limit and risking runtime truncation errors.
🤖 Prompt for AI Agents
In Streetcode/Streetcode.BLL/Validators/TextContent/CreateTermValidator.cs
around lines 10-12, the BLL enforces a 200-char max for Term.Title but the Term
entity lacks a corresponding DB constraint; update
Streetcode/Streetcode.DAL/Entities/Streetcode/TextContent/Term.cs to add
[MaxLength(200)] (or configure .HasMaxLength(200) in the DbContext fluent API)
on the Title property and then add and run an EF Core migration to persist the
schema change so the database matches the validator.
| [HttpPost] | ||
| public async Task<IActionResult> Create([FromBody] CreateVideoDTO video) | ||
| { | ||
| return HandleResult(await Mediator.Send(new CreateVideoCommand(video))); | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
POST Create endpoint: OK; verify existing GET route ambiguity
Create action wiring is fine. However, current GET routes use identical templates "{int}" for both GetByStreetcodeId and GetById, which leads to ambiguous matches at runtime. Consider distinct templates (example below).
Example fix outside this hunk (for lines 18-26):
[HttpGet("by-streetcode/{streetcodeId:int}")]
public async Task<IActionResult> GetByStreetcodeId([FromRoute] int streetcodeId) =>
HandleResult(await Mediator.Send(new GetVideoByStreetcodeIdQuery(streetcodeId)));
[HttpGet("{id:int}")]
public async Task<IActionResult> GetById([FromRoute] int id) =>
HandleResult(await Mediator.Send(new GetVideoByIdQuery(id)));🏁 Script executed:
#!/bin/bash
# Locate the VideoController file
file=$(fd --hidden --full-path VideoController.cs)
# Show all HttpGet attributes with context
rg -nC2 "\[HttpGet" "$file"
# Show GetByStreetcodeId action with context
rg -nC2 "GetByStreetcodeId" "$file"
# Show GetById action with context
rg -nC2 "GetById" "$file"Length of output: 1277
Differentiate GET route templates to avoid ambiguity
Change GetByStreetcodeId to use a unique template (e.g. [HttpGet("by-streetcode/{streetcodeId:int}")]) instead of "{streetcodeId:int}", so it won’t collide with GetById’s "{id:int}".
🤖 Prompt for AI Agents
In Streetcode/Streetcode.WebApi/Controllers/Media/VideoController.cs around
lines 30 to 34, the GET action that returns videos by streetcodeId uses the same
numeric route template as GetById and can collide; change its route attribute to
a unique template such as [HttpGet("by-streetcode/{streetcodeId:int}")] (and
update any route names or client calls/Swagger references accordingly) so the
two endpoints no longer conflict.
| [Fact] | ||
| public async Task Handle_TitleMaxLength_ReturnsError() | ||
| { | ||
| var requestDto = new CreateVideoDTO | ||
| { | ||
| Title = new string('*', 101), | ||
| Description = "Description", | ||
| Url = "https://google.com", | ||
| StreetcodeId = 1 | ||
| }; | ||
|
|
||
| var mappedEntity = new Entity | ||
| { | ||
| Title = new string('*', 101), | ||
| Description = "Description", | ||
| Url = "https://google.com", | ||
| StreetcodeId = 1 | ||
| }; | ||
|
|
||
| _mapperMock.Setup(m => m.Map<Entity>(requestDto)).Returns(mappedEntity); | ||
|
|
||
| _repositoryWrapperMock | ||
| .Setup(r => r.VideoRepository.GetFirstOrDefaultAsync(It.IsAny<Expression<Func<Entity, bool>>>(), default)) | ||
| .ReturnsAsync((Entity)null); | ||
|
|
||
| _repositoryWrapperMock.Setup(r => r.VideoRepository.CreateAsync(mappedEntity)).ReturnsAsync(mappedEntity); | ||
| _repositoryWrapperMock.Setup(r => r.SaveChangesAsync()).ReturnsAsync(1); | ||
|
|
||
| var result = await _handler.Handle(new CreateVideoCommand(requestDto), CancellationToken.None); | ||
|
|
||
| result.IsFailed.Should().BeTrue(); | ||
| result.Errors.First().Message.Should().Contain("��������� ���� �� ���� ���� ����� 100 �������."); | ||
| } |
There was a problem hiding this comment.
Update expected message to match corrected validation text
Tests currently assert mojibake. Align with clear message from handler.
- result.Errors.First().Message.Should().Contain("��������� ���� �� ���� ���� ����� 100 �������.");
+ result.Errors.First().Message.Should().Contain("Title is required and must not exceed 100 characters.");Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
Streetcode/Streetcode.XUnitTest/BLL/MediatR/Media/Video/CreateVideoHandlerTests.cs
around lines 91 to 123, the test asserts a mojibake string for the title-length
validation; update the assertion to match the corrected validation message
emitted by the handler (replace the current expected message "Çàãîëîâîê â³äåî íå
ìîæå áóòè á³ëüøå 100 ñèìâîë³â." with the proper Ukrainian text, e.g. "Заголовок
відео не може бути більше 100 символів."). Ensure the test still checks IsFailed
is true and that Errors.First().Message.Contains(...) uses the corrected string.
weazy12
left a comment
weazy12
left a comment
There was a problem hiding this comment.
Move your validation into Validation folder. Write unit tests for all your new features and resolve all conversations from coderabbit.
2 participants
dev
JIRA
Code reviewers
Second Level Review
Summary of issue
ToDo
Summary of change
ToDo
Testing approach
ToDo
CHECK LIST
Summary by CodeRabbit
New Features
Improvements
Tests
Chores