Finishing Comment Section in API Design#21
Conversation
allow running all branches and PRs
Update dotnet.yml
Updating the main to the Develop branch
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughIntroduces comment-like functionality: new CommentController with GET/POST/DELETE endpoints for comment likes; corresponding BLL services and interfaces for comments and comment likes; DI registrations; DTO for likes; unit tests; user identity return type change; minor API controller response tweaks; EF Core defaults added for CreatedAt/UpdatedAt/DeletedAt via migrations; Thread alias fix in DAL. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant API as CommentController
participant U as IUserService
participant CLS as ICommentLikeService
participant CS as ICommentService
participant UoW as UnitOfWork/Repos
rect rgb(245,248,252)
Note over Client,API: GET /api/comment/{id}/likes
Client->>API: GetAllLikes(id)
API->>CLS: GetLikesForCommentAsync(id)
CLS->>UoW: Validate comment, fetch likes
UoW-->>CLS: Likes (DTOs)
CLS-->>API: ServiceResult<IEnumerable<CommentLikeDto>>
API-->>Client: 200/400/404/500
end
rect rgb(245,252,245)
Note over Client,API: POST /api/comment/{id}/likes
Client->>API: AddLike(id)
API->>U: GetCurrentUserAsync()
U-->>API: ServiceResult<ApplicationUser>
API->>CLS: AddLikeAsync(id, userId)
CLS->>UoW: Load comment/user, upsert like, save
UoW-->>CLS: Persisted like
CLS-->>API: ServiceResult<CommentLikeDto>
API-->>Client: 201/400/409/500
end
rect rgb(252,245,245)
Note over Client,API: DELETE /api/comment/{id}/likes
Client->>API: DeleteLike(id)
API->>U: GetCurrentUserAsync()
U-->>API: ServiceResult<ApplicationUser>
API->>CS: GetCommentAsync(id)
CS->>UoW: GetByIdAsync(id)
UoW-->>CS: Comment or null
CS-->>API: ServiceResult<Comment>
API->>CLS: DeleteLikeAsync(id, userId)
CLS->>UoW: Find like, remove, save
UoW-->>CLS: Success/failure
CLS-->>API: ServiceResult
API-->>Client: 204/403/404/500
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
✨ 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
AskFm/AskFm.API/Program.cs (2)
31-33:UseLazyLoadingProxies()requires the Proxies namespace/package import.Without
using Microsoft.EntityFrameworkCore.Proxies;the extension method is out of scope and compilation will fail.Apply:
+using Microsoft.EntityFrameworkCore.Proxies; ... builder.Services.AddDbContext<AppDbContext>(options => options .UseLazyLoadingProxies() .UseSqlServer(ConnectionString));
49-52: Critical: Configure and invoke authentication before authorizationYour API currently calls
app.UseAuthorization()without having first registered or enabled authentication. This meansHttpContext.Userwill never be populated. To fix:• In your service configuration (typically in
Program.csbefore building the app), register an authentication scheme. For example:builder.Services .AddAuthentication(JwtBearerDefaults.AuthenticationScheme) .AddJwtBearer(options => { // configure token validation parameters here });• In the middleware pipeline (around lines 49–52 of
Program.cs), insertUseAuthentication()beforeUseAuthorization():app.UseHttpsRedirection(); + app.UseAuthentication(); app.UseAuthorization();These changes are mandatory to ensure your controllers see an authenticated
Userand that any[Authorize]attributes work as intended.
🧹 Nitpick comments (11)
AskFm/AskFm.BLL/DTO/CommentLikeDto.cs (1)
3-9: Make the DTO immutable, fix non-nullable warnings, and prefer DateTimeOffset for timestamps.
- With enable,
string UserName { get; set; }is non-nullable but uninitialized, triggering CS8618 and risking nulls in serialization.- DTOs benefit from immutability;
init-only or arecordreduce accidental mutations.- For cross-time-zone correctness,
DateTimeOffsetis a better default thanDateTimefor persisted/serialized timestamps.Suggested refactor (record with required/init and DateTimeOffset):
-namespace AskFm.BLL.DTO; - -public class CommentLikeDto -{ - public int CommentId { get; set; } - public int UserId { get; set; } - public string UserName { get; set; } - public DateTime CreatedAt { get; set; } -} +namespace AskFm.BLL.DTO; + +public sealed record CommentLikeDto +{ + public required int CommentId { get; init; } + public required int UserId { get; init; } + public required string UserName { get; init; } + public DateTimeOffset CreatedAt { get; init; } +}If you prefer to keep a class:
-public class CommentLikeDto -{ - public int CommentId { get; set; } - public int UserId { get; set; } - public string UserName { get; set; } - public DateTime CreatedAt { get; set; } -} +public class CommentLikeDto +{ + public int CommentId { get; init; } + public int UserId { get; init; } + public required string UserName { get; init; } + public DateTimeOffset CreatedAt { get; init; } +}AskFm/AskFm.BLL/Services/ICommentService.cs (2)
1-1: Decouple BLL service contracts from DAL model types if possible.
ICommentServiceexposesAskFm.DAL.Models.Comment. Consider returning a BLL-facing DTO/view model to avoid leaking DAL entities across layers and to keep the BLL independent of persistence concerns. This also simplifies serialization concerns at the API boundary.
5-7: Enable async comment retrieval with nullability and optional cancellationTo align with EF Core’s asynchronous data access and clearly model “not found,” refactor the service method from synchronous
Comment GetComment(int)toTask<Comment?> GetCommentAsync(int, CancellationToken). This change will require updates in the interface, its implementation, and all call sites.Affected locations:
- ICommentService.cs (lines 5–7): change return type, rename method, add
CancellationToken.- CommentService.cs (around line 14): implement
GetCommentAsyncand invoke the async repository method (e.g.,GetByIdAsync).- CommentController.cs (around lines 94–100): update calls to
await _commentService.GetCommentAsync(id, cancellationToken)and handle the null result.Suggested diff for the interface:
namespace AskFm.BLL.Services; -public interface ICommentService -{ - Comment GetComment(int commentId); -} +using System.Threading; +using System.Threading.Tasks; + +public interface ICommentService +{ + Task<Comment?> GetCommentAsync(int commentId, CancellationToken cancellationToken = default); +}AskFm/AskFm.DAL/Interfaces/IUnitOfWork.cs (2)
2-2: Avoid aliasingThreadto minimize collisions withSystem.Threading.Thread.Using
Threadas an alias invites accidental confusion/import collisions in other files. Prefer a more explicit alias or fully-qualify the domain type.Apply:
-using Thread = AskFm.DAL.Models.Thread; +using DomainThread = AskFm.DAL.Models.Thread;And update the property:
- IRepository<Thread> Threads { get; } + IRepository<DomainThread> Threads { get; }Also applies to: 9-9
17-18: Async-first standardization with cancellation support – minimal sync API surface detectedWe ran the ripgrep search and found only two references to the synchronous Save() API:
- Implementation in AskFm/AskFm.DAL/UnitOfWork.cs (line 128):
public int Save() => _context.SaveChanges();
- One test in AskFm/Tests/CommentLikeServiceTest.cs (line 87):
_mockUnitOfWork.Verify(uow => uow.Save(), Times.Never);Since the sync API surface is small, migrating toward an async-first model with cancellation support should be straightforward.
Key action items:
- Add a cancellation-aware overload to the interface:
interface IUnitOfWork {Task<int> SaveAsync();
Task<int> SaveAsync(CancellationToken cancellationToken = default);
- Update the implementation in UnitOfWork.cs:
public Task<int> SaveAsync()=> _context.SaveChangesAsync();
=> _context.SaveChangesAsync(cancellationToken);
- Modify any callers (e.g., in CommentLikeServiceTest.cs) to verify the new async method instead of the sync one.
- Optionally mark int Save() as
[Obsolete("Use SaveAsync with CancellationToken")]or remove it in the next major release to discourage mixed usage.This refactor remains optional but will improve scalability and consistency if adopted.
AskFm/AskFm.BLL/Services/CommentService.cs (2)
8-12: Make the dependency readonly and guard against null.Minor polish for immutability and safety.
Apply:
- private IUnitOfWork _unitOfWork; + private readonly IUnitOfWork _unitOfWork; public CommentService(IUnitOfWork unitOfWork) { - _unitOfWork = unitOfWork; + ArgumentNullException.ThrowIfNull(unitOfWork); + _unitOfWork = unitOfWork; }
14-18: Consider async or explicit not-found behavior.
GetCommentreturnsnullwhen absent; either:
- Make it async if the repo supports it (
GetByIdAsync) and addCancellationToken, or- Throw a domain-specific exception when not found, or
- Rename to
FindComment/TryGetCommentto reflect nullable return.If async exists:
- public Comment GetComment(int commentId) + public Task<Comment?> GetCommentAsync(int commentId, CancellationToken ct = default) { - var comment = _unitOfWork.Comments.GetById(commentId); - return comment; + return _unitOfWork.Comments.GetByIdAsync(commentId, ct); }AskFm/AskFm.API/Controllers/CommentController.cs (1)
42-46: Consider not echoing raw exception messages to clientsReturning
ex.Messagecan leak internal details. Map to consistent, client-friendly messages, and keep details in logs.AskFm/AskFm.BLL/Services/CommentLikeService.cs (1)
121-123: Structured logging token namePrefer consistent casing for tokens: use
{UserId}not{userid}to keep property names uniform across logs.AskFm/Tests/CommentLikeServiceTest.cs (2)
37-72: Good happy-path coverage for AddLike, but consider asserting LikeCount incrementYou verify AddAsync and SaveAsync; also assert
comment.LikeCount == 1to ensure bookkeeping is updated.Example:
Assert.Equal(1, comment.LikeCount);
172-197: Assertion strategy: be explicit on fields or use custom comparer
Assert.Equivalentcan be lenient or brittle depending on versions. To avoid flakiness (extra fields like CreatedAt/UserName), assert per-field or provide a comparer ignoring those fields.Example:
Assert.All(result, dto => Assert.Equal(commentId, dto.CommentId)); Assert.Equal(new[] {4,5,6,8,9}, result.Select(r => r.UserId));
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (13)
AskFm/AskFm.API/Controllers/CommentController.cs(1 hunks)AskFm/AskFm.API/Program.cs(2 hunks)AskFm/AskFm.BLL/AskFm.BLL.csproj(1 hunks)AskFm/AskFm.BLL/DTO/CommentLikeDto.cs(1 hunks)AskFm/AskFm.BLL/DTO/CreateCommentLikeDto.cs(1 hunks)AskFm/AskFm.BLL/Services/CommentLikeService.cs(1 hunks)AskFm/AskFm.BLL/Services/CommentService.cs(1 hunks)AskFm/AskFm.BLL/Services/ICommentLikeService.cs(1 hunks)AskFm/AskFm.BLL/Services/ICommentService.cs(1 hunks)AskFm/AskFm.DAL/Interfaces/IUnitOfWork.cs(1 hunks)AskFm/AskFm.DAL/UnitOfWork.cs(1 hunks)AskFm/Tests/CommentLikeServiceTest.cs(1 hunks)AskFm/Tests/Tests.csproj(2 hunks)
🔇 Additional comments (3)
AskFm/Tests/Tests.csproj (1)
13-13: LGTM: Moq reference is appropriate for service-level unit tests.AskFm/AskFm.DAL/UnitOfWork.cs (1)
4-4: LGTM: Alias resolves ambiguity with System.Threading.Thread and aligns repository type.This change prevents accidental usage of the threading type in repository definitions in this file.
AskFm/Tests/CommentLikeServiceTest.cs (1)
34-35: Constructor wiring OKMocks and service under test are constructed correctly.
| builder.Services.AddScoped<ICommentLikeService, CommentLikeService>(); | ||
| builder.Services.AddScoped<ICommentService, CommentService>(); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Good DI registrations; ensure AutoMapper (if used) is registered too.
If CommentLikeService/CommentService use IMapper, you need AddAutoMapper(...) in DI.
Search for IMapper usage:
If found, add:
+builder.Services.AddAutoMapper(AppDomain.CurrentDomain.GetAssemblies());The scoped registrations for the services look correct alongside IUnitOfWork.
🏁 Script executed:
#!/bin/bash
rg -nP --type cs -C2 '\bIMapper\b|AutoMapper'Length of output: 1565
Ensure AutoMapper is registered and injected for all IMapper usages
We’ve confirmed IMapper is used in both the BLL service and the API controller but neither the DI registration nor the constructors reflect that:
- AskFm/AskFm.BLL/Services/CommentLikeService.cs declares
→ Missing constructor parameter and assignment for
private readonly IMapper _mapper; public CommentLikeService(IUnitOfWork unitOfWork, ILogger<CommentLikeService> logger) { … }
IMapper _mapper. - AskFm/AskFm.API/Controllers/CommentController.cs includes
→ Likewise needs
using AutoMapper; public class CommentController : ControllerBase { private readonly IMapper _mapper; … }
IMapperin its constructor.
Please apply the following updates:
• In Program.cs (AskFm/AskFm.API/Program.cs), before building the host, register AutoMapper:
builder.Services.AddScoped<ICommentLikeService, CommentLikeService>();
builder.Services.AddScoped<ICommentService, CommentService>();
+builder.Services.AddAutoMapper(AppDomain.CurrentDomain.GetAssemblies());• Update CommentLikeService constructor:
- public CommentLikeService(IUnitOfWork unitOfWork, ILogger<CommentLikeService> logger)
+ public CommentLikeService(IUnitOfWork unitOfWork,
+ ILogger<CommentLikeService> logger,
+ IMapper mapper)
{
_unitOfWork = unitOfWork;
_logger = logger;
+ _mapper = mapper;
}• Update CommentController constructor similarly to accept and assign IMapper.
These changes will ensure AutoMapper is available in DI and that each consuming class receives its IMapper instance.
🤖 Prompt for AI Agents
In AskFm/AskFm.API/Program.cs around lines 40-41, AutoMapper is not registered
in DI while IMapper is required by services and controllers; register AutoMapper
(e.g., builder.Services.AddAutoMapper(typeof(Startup) or an assembly marker)
before building the host. In AskFm/AskFm.BLL/Services/CommentLikeService.cs add
an IMapper parameter to the constructor and assign it to the existing private
readonly IMapper _mapper field (and import AutoMapper namespace), preserving
existing parameters and logger. In
AskFm/AskFm.API/Controllers/CommentController.cs update the controller
constructor to accept IMapper and assign it to the controller's private readonly
IMapper _mapper field (add using AutoMapper if missing). Ensure constructor
parameter order and DI registrations remain consistent so IMapper is injected
where used.
| <ItemGroup> | ||
| <Folder Include="DTO\" /> | ||
| <Folder Include="Services\" /> | ||
| <PackageReference Include="AutoMapper" Version="15.0.1" /> |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Confirm AutoMapper DI integration point; add DI package in the API if needed.
Adding AutoMapper here is fine if BLL uses IMapper internally. If the mapping is configured/registered in the API via services.AddAutoMapper(...), ensure the API project references AutoMapper.Extensions.Microsoft.DependencyInjection; that DI package is typically added to the startup project rather than BLL.
Run to verify usage and registration points:
🏁 Script executed:
#!/bin/bash
# Find AutoMapper registrations and usages
rg -n --type=cs '\bAddAutoMapper\b|IMapper\b|MapperConfiguration\b|Profile\b' -C3Length of output: 702
🏁 Script executed:
#!/bin/bash
# Search for DI package reference in all csproj files
rg -n "AutoMapper.Extensions.Microsoft.DependencyInjection" -g "*.csproj"
# Search for DI registration code (services.AddAutoMapper) in C# files
rg -n --type=cs "AddAutoMapper" -C3Length of output: 109
Add AutoMapper DI registration in the API project
The BLL layer is injecting IMapper (e.g., in CommentLikeService), but I didn’t find any DI package reference or services.AddAutoMapper(...) calls across the solution. Without registering AutoMapper in your startup project, those injections will fail at runtime.
Please address the following:
- In your API project’s
.csproj(e.g.AskFm/AskFm.API/AskFm.API.csproj), add:<PackageReference Include="AutoMapper.Extensions.Microsoft.DependencyInjection" Version="[latest-compatible-version]" />
- In your startup configuration (
Program.csorStartup.cs), register AutoMapper, for example:services.AddAutoMapper(typeof(Startup).Assembly); // or specify assemblies containing your Profile classes: // services.AddAutoMapper(typeof(CommentMappingProfile).Assembly);
This will ensure that IMapper is properly configured and available to the BLL.
🤖 Prompt for AI Agents
In AskFm/AskFm.BLL/AskFm.BLL.csproj around line 15, the project references
AutoMapper but the API project lacks the DI integration; add the
AutoMapper.Extensions.Microsoft.DependencyInjection package reference to your
API project's .csproj (AskFm/AskFm.API/AskFm.API.csproj) and then register
AutoMapper during startup (in Program.cs or Startup.cs) by calling the
AddAutoMapper extension and pointing it to the assembly or assemblies that
contain your AutoMapper Profile classes (for example the assembly containing
your mapping profiles or Startup class) so that IMapper can be resolved by the
BLL at runtime.
| public interface ICommentLikeService | ||
| { | ||
| Task<IEnumerable<CommentLikeDto>> GetLikesForCommentAsync(int commentId); | ||
| Task<CommentLikeDto> AddLikeAsync(int commentId, int userId); | ||
| Task DeleteLikeAsync(int commentId, int userId); | ||
| } No newline at end of file |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Add CancellationToken and define idempotent semantics.
Async service methods should accept CancellationToken. Also clarify whether adding a like is idempotent (no-op if already liked) or should error.
Consider:
public interface ICommentLikeService
{
- Task<IEnumerable<CommentLikeDto>> GetLikesForCommentAsync(int commentId);
- Task<CommentLikeDto> AddLikeAsync(int commentId, int userId);
- Task DeleteLikeAsync(int commentId, int userId);
+ Task<IEnumerable<CommentLikeDto>> GetLikesForCommentAsync(int commentId, CancellationToken ct = default);
+ Task<CommentLikeDto> AddLikeAsync(int commentId, int userId, CancellationToken ct = default);
+ Task<bool> DeleteLikeAsync(int commentId, int userId, CancellationToken ct = default); // return false if not found
}Scan implementation to ensure alignment if you adopt CTs/return type:
🏁 Script executed:
#!/bin/bash
rg -nP --type cs 'class\s+CommentLikeService\b|ICommentLikeService'Length of output: 583
🏁 Script executed:
#!/bin/bash
# Print the first 200 lines of the CommentLikeService implementation to inspect method signatures
sed -n '1,200p' AskFm/AskFm.BLL/Services/CommentLikeService.csLength of output: 4893
Improve CancellationToken support, idempotency semantics, and correct DeleteLikeAsync implementation
- All three interface methods lack a
CancellationTokenparameter – the implementation’sGetByIdAsync,FindAsync, etc., all accept a token. AddingCancellationToken ct = defaultensures upstream callers can cancel long-running operations. AddLikeAsynccurrently throws if a duplicate like exists. Decide and document whether this should remain an error or become an idempotent no-op.DeleteLikeAsyncsignature and behavior both need adjustment:- It blocks on
GetByIdAsyncvia.Resultinstead of awaiting, risking deadlocks. - It marks the comment (
comment.Result.IsDeleted = true) rather than the like as deleted – this will delete the comment itself. - It throws when no like exists; consider returning
falseinstead of throwing, and returntrueon success.
- It blocks on
Locations to update:
- AskFm/AskFm.BLL/Services/ICommentLikeService.cs – add
CancellationTokenparams and changeDeleteLikeAsyncreturn type. - AskFm/AskFm.BLL/Services/CommentLikeService.cs – update method signatures, await all async calls, delete the correct entity, and return a success flag on delete.
Suggested interface diff:
public interface ICommentLikeService
{
- Task<IEnumerable<CommentLikeDto>> GetLikesForCommentAsync(int commentId);
- Task<CommentLikeDto> AddLikeAsync(int commentId, int userId);
- Task DeleteLikeAsync(int commentId, int userId);
+ Task<IEnumerable<CommentLikeDto>> GetLikesForCommentAsync(int commentId, CancellationToken ct = default);
+ /// <summary>
+ /// Adds a like; throws if already liked unless idempotent semantics chosen.
+ /// </summary>
+ Task<CommentLikeDto> AddLikeAsync(int commentId, int userId, CancellationToken ct = default);
+ /// <summary>
+ /// Deletes a like; returns false if no like existed.
+ /// </summary>
+ Task<bool> DeleteLikeAsync(int commentId, int userId, CancellationToken ct = default);
}And in CommentLikeService.cs, for DeleteLikeAsync:
- public async Task DeleteLikeAsync(int commentId, int userId)
+ public async Task<bool> DeleteLikeAsync(int commentId, int userId, CancellationToken ct = default)
{
- var commentLike = await _unitOfWork.CommentLikes.FindAsync(cl => cl.CommentId == commentId && cl.UserId == userId && !cl.IsDeleted);
+ var commentLike = await _unitOfWork.CommentLikes.FindAsync(
+ predicate: cl => cl.CommentId == commentId && cl.UserId == userId && !cl.IsDeleted,
+ cancellationToken: ct);
// …
- var comment = _unitOfWork.Comments.GetByIdAsync(commentId);
- Console.WriteLine(comment.Result.Content);
- // if the user didn't like this comment before
- if(commentLike == null)
- throw new ArgumentException($"User didn't like this comment");
-
- await _unitOfWork.CommentLikes.RemoveAsync(commentLike);
-
- comment.Result.IsDeleted = true;
- comment.Result.CommentLikes.Remove(commentLike);
- comment.Result.LikeCount--;
-
- await _unitOfWork.SaveAsync();
-
- }
+ if (commentLike == null)
+ return false;
+
+ var comment = await _unitOfWork.Comments.GetByIdAsync(commentId, ct)
+ ?? throw new ArgumentException($"Comment {commentId} not found");
+
+ // Mark the like as deleted rather than the comment
+ commentLike.IsDeleted = true;
+ comment.LikeCount--;
+ _unitOfWork.CommentLikes.Update(commentLike);
+ _unitOfWork.Comments.Update(comment);
+
+ await _unitOfWork.SaveAsync(ct);
+ return true;
+ }📝 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 interface ICommentLikeService | |
| { | |
| Task<IEnumerable<CommentLikeDto>> GetLikesForCommentAsync(int commentId); | |
| Task<CommentLikeDto> AddLikeAsync(int commentId, int userId); | |
| Task DeleteLikeAsync(int commentId, int userId); | |
| } | |
| // AskFm/AskFm.BLL/Services/ICommentLikeService.cs | |
| public interface ICommentLikeService | |
| { | |
| Task<IEnumerable<CommentLikeDto>> GetLikesForCommentAsync(int commentId, CancellationToken ct = default); | |
| /// <summary> | |
| /// Adds a like; throws if already liked unless idempotent semantics chosen. | |
| /// </summary> | |
| Task<CommentLikeDto> AddLikeAsync(int commentId, int userId, CancellationToken ct = default); | |
| /// <summary> | |
| /// Deletes a like; returns false if no like existed. | |
| /// </summary> | |
| Task<bool> DeleteLikeAsync(int commentId, int userId, CancellationToken ct = default); | |
| } |
| public interface ICommentLikeService | |
| { | |
| Task<IEnumerable<CommentLikeDto>> GetLikesForCommentAsync(int commentId); | |
| Task<CommentLikeDto> AddLikeAsync(int commentId, int userId); | |
| Task DeleteLikeAsync(int commentId, int userId); | |
| } | |
| // AskFm/AskFm.BLL/Services/CommentLikeService.cs | |
| public async Task<bool> DeleteLikeAsync(int commentId, int userId, CancellationToken ct = default) | |
| { | |
| var commentLike = await _unitOfWork.CommentLikes.FindAsync( | |
| predicate: cl => cl.CommentId == commentId && cl.UserId == userId && !cl.IsDeleted, | |
| cancellationToken: ct); | |
| if (commentLike == null) | |
| return false; | |
| var comment = await _unitOfWork.Comments.GetByIdAsync(commentId, ct) | |
| ?? throw new ArgumentException($"Comment {commentId} not found"); | |
| // Mark the like as deleted rather than the comment | |
| commentLike.IsDeleted = true; | |
| comment.LikeCount--; | |
| _unitOfWork.CommentLikes.Update(commentLike); | |
| _unitOfWork.Comments.Update(comment); | |
| await _unitOfWork.SaveAsync(ct); | |
| return true; | |
| } |
🤖 Prompt for AI Agents
In AskFm/AskFm.BLL/Services/ICommentLikeService.cs around lines 5-10, update the
API to accept CancellationToken on all methods and make DeleteLikeAsync return a
success flag: change signatures to include CancellationToken ct = default for
GetLikesForCommentAsync and AddLikeAsync, and change DeleteLikeAsync to
Task<bool> DeleteLikeAsync(int commentId, int userId, CancellationToken ct =
default). In the implementation (CommentLikeService.cs) update method signatures
accordingly, await all async calls using the passed ct (no .Result), make
AddLikeAsync idempotent by returning the existing CommentLikeDto if a like
already exists instead of throwing (or document alternative if you choose to
keep throwing), and fix DeleteLikeAsync to locate the like entity, return false
if not found, set like.IsDeleted = true, persist changes, and return true on
success while passing ct through to repository calls.
| <ItemGroup> | ||
| <ProjectReference Include="..\AskFm.API\AskFm.API.csproj" /> | ||
| </ItemGroup> |
There was a problem hiding this comment.
💡 Verification agent
❓ Verification inconclusive
Avoid coupling unit tests to the API layer; reference BLL directly.
For unit-testing CommentLikeService, a reference to AskFm.API is unnecessary and tightens coupling. Prefer a project reference to AskFm.BLL (and DAL/Shared if truly required). Keep API references only for API integration tests.
Apply:
- <ProjectReference Include="..\AskFm.API\AskFm.API.csproj" />
+ <ProjectReference Include="..\AskFm.BLL\AskFm.BLL.csproj" />🏁 Script executed:
#!/bin/bash
# Check if tests are actually doing API/integration work; if none, drop API ref.
rg -n 'WebApplicationFactory|TestServer|HttpClient|ControllerBase|Controller\s' AskFm/Tests -C2Length of output: 97
Avoid coupling Tests to the API layer
Verified: no API/integration constructs (WebApplicationFactory, TestServer, HttpClient, Controller, etc.) are used in AskFm/Tests, so the API reference is unnecessary and introduces tight coupling. Tests should target the BLL (and DAL/Shared if they depend on those types) directly.
Please update the project reference in AskFm/Tests/Tests.csproj:
• File: AskFm/Tests/Tests.csproj (lines 22–24)
• Replace the API reference with the BLL reference:
--- a/AskFm/Tests/Tests.csproj
@@ 22,24
- <ProjectReference Include="..\AskFm.API\AskFm.API.csproj" />
+ <ProjectReference Include="..\AskFm.BLL\AskFm.BLL.csproj" />Ensure you also add references to AskFm.DAL or AskFm.Shared if any test cases need domain or shared types.
🤖 Prompt for AI Agents
In AskFm/Tests/Tests.csproj around lines 22 to 24, the tests currently reference
the API project which introduces unnecessary coupling; remove the
ProjectReference to ..\AskFm.API\AskFm.API.csproj and add a ProjectReference to
the BLL project (e.g. ..\AskFm.BLL\AskFm.BLL.csproj) instead; also inspect tests
for any domain/shared types and add ProjectReference(s) to AskFm.DAL and/or
AskFm.Shared as required so tests depend only on BLL (and DAL/Shared when
needed).
- Added CommentController (Adding 3 endpoints mentioned on API Design
issue)
- Added ICommentService, CommentService (With one function to get the
comment by id)
- Added ICommentLikeService and CommentLikeService to handle the core
logic function for the Comment likes.
- Added CommentLikeServiceTest to write Unit tests for the
CommentlikeService Class.
# Conflicts:
# AskFm/AskFm.API/Program.cs
# AskFm/AskFm.BLL/AskFm.BLL.csproj
…oundOrDeleted Test case
…tLikeService to program.cs
…he mapping on 'profile' endpoint
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
AskFm/AskFm.DAL/AppDbContext.cs (1)
70-76: Don’t setDeletedAton Added; this flags new entities as deletedOn insert you set
DeletedAt = UtcNow. That’s incorrect for soft-delete and contradicts the mapping.case EntityState.Added: entry.Entity.CreatedAt = DateTime.UtcNow; - entry.Entity.DeletedAt = DateTime.UtcNow;; entry.Entity.UpdatedAt = DateTime.UtcNow; entry.Entity.IsDeleted = false; break;AskFm/AskFm.BLL/Services/UserIdentityService/UserService.cs (1)
55-69: NRE risk when reading claims; also return failure when user not found.
FindFirst(ClaimTypes.Email).Valuewill throw if the claim/context is missing. Additionally, Success(null) propagates null to controllers.Apply this diff:
- public async Task<ServiceResult<ApplicationUser>> GetCurrentUserAsync() + public async Task<ServiceResult<ApplicationUser>> GetCurrentUserAsync() { - string email = _httpContextAccessor.HttpContext.User.FindFirst(ClaimTypes.Email).Value; - if (string.IsNullOrEmpty(email)) + var email = _httpContextAccessor.HttpContext?.User?.FindFirstValue(ClaimTypes.Email); + if (string.IsNullOrWhiteSpace(email)) { var errors = new List<string>() { - "Can't Access Current user" + "Cannot access current user (missing email claim)." }; - return await ServiceResult<ApplicationUser>.Failure(errors); + return await ServiceResult<ApplicationUser>.Failure(errors); } - var currentAppUser = await _userManager.FindByEmailAsync(email); - - return await ServiceResult<ApplicationUser>.Success(currentAppUser); + var currentAppUser = await _userManager.FindByEmailAsync(email); + if (currentAppUser is null) + { + var errors = new List<string>() { "Current user not found." }; + return await ServiceResult<ApplicationUser>.Failure(errors); + } + return await ServiceResult<ApplicationUser>.Success(currentAppUser); }Optionally, fall back to ClaimTypes.NameIdentifier if email is absent, and consider returning a distinct unauthorized failure to allow the controller to emit 401.
♻️ Duplicate comments (3)
AskFm/AskFm.API/Program.cs (1)
48-49: Register AutoMapper in DI and ensure constructors injectIMapper
IMapperis used in services/controllers; register it. Also verify constructors accept and assignIMapper.builder.Services.AddScoped<IUserService, UserService>(); builder.Services.AddScoped<ICommentLikeService, CommentLikeService>(); builder.Services.AddScoped<ICommentService, CommentService>(); + builder.Services.AddAutoMapper(AppDomain.CurrentDomain.GetAssemblies());#!/bin/bash # Ensure constructors inject IMapper and assignments exist rg -nP --type cs -C2 'class\s+\w+\s*:\s*.*Controller|Service' | sed -n '1,120p' >/dev/null rg -nP --type cs -C2 '\bIMapper\b' rg -nP --type cs -C2 'public\s+\w+\([^)]*IMapper[^)]*\)' rg -nP --type cs -C2 '_mapper\s*='AskFm/AskFm.BLL/Services/ICommentLikeService.cs (1)
5-10: Add CancellationToken to all methods and fix Delete return type to reflect no payloadExpose cancellation and return a simple success flag for delete. Clarify idempotency in XML docs.
public interface ICommentLikeService { - Task<ServiceResult<IEnumerable<CommentLikeDto>>> GetLikesForCommentAsync(int commentId); - Task<ServiceResult<CommentLikeDto>> AddLikeAsync(int commentId, int userId); - Task<ServiceResult<CommentLikeDto>> DeleteLikeAsync(int commentId, int userId); + Task<ServiceResult<IEnumerable<CommentLikeDto>>> GetLikesForCommentAsync(int commentId, CancellationToken ct = default); + Task<ServiceResult<CommentLikeDto>> AddLikeAsync(int commentId, int userId, CancellationToken ct = default); + // Returns true if a like was removed; false if none existed (idempotent delete). + Task<ServiceResult<bool>> DeleteLikeAsync(int commentId, int userId, CancellationToken ct = default); }AskFm/AskFm.BLL/Services/CommentLikeService.cs (1)
14-21: Fix compile error: unused readonly _mapper and non-readonly _unitOfWorkremove IMapper (unused) and mark _unitOfWork readonly. This currently fails to compile.
-using AutoMapper; +// using AutoMapper; public class CommentLikeService : ICommentLikeService { - - private IUnitOfWork _unitOfWork; + private readonly IUnitOfWork _unitOfWork; private readonly ILogger<CommentLikeService> _logger; - private readonly IMapper _mapper; - public CommentLikeService(IUnitOfWork unitOfWork, ILogger<CommentLikeService> logger) + public CommentLikeService(IUnitOfWork unitOfWork, ILogger<CommentLikeService> logger) { _unitOfWork = unitOfWork; _logger = logger; }
🧹 Nitpick comments (27)
AskFm/AskFm.DAL/AppDbContext.cs (2)
64-66: Fix typo and references:_fillTrackableInfromation→_fillTrackableInformationImproves readability and consistency.
- _fillTrackableInfromation(); + _fillTrackableInformation(); ... - _fillTrackableInfromation(); + _fillTrackableInformation(); ... - private void _fillTrackableInfromation() + private void _fillTrackableInformation()Also applies to: 55-62
25-51: Consider moving all timestamps to DB defaults or EF layer, but not bothRight now both DB defaults and EF code set timestamps. Pick one strategy to avoid drift. If DB-side, add an UPDATE trigger for
UpdatedAt; if EF-side, drop DB defaults (except CreatedAt).AskFm/AskFm.API/Program.cs (3)
50-50: DuplicateAddControllers()registrationThis call appears twice. Remove the second one.
- builder.Services.AddControllers();
63-66: Redundant null-check afternew
jwtOptionscannot be null. Remove dead code; add guards for missing env vars instead.- if (jwtOptions == null) - { - throw new Exception("jwtOptions is null"); - } + if (string.IsNullOrEmpty(jwtOptions.SigningKey) + || string.IsNullOrEmpty(jwtOptions.Issuer) + || string.IsNullOrEmpty(jwtOptions.Audience)) + throw new Exception("JWT environment variables are missing");
77-83: Use the same scheme constant for DefaultAuthenticateSchemeMinor consistency/readability improvement.
- builder.Services.AddAuthentication(options => + builder.Services.AddAuthentication(options => { - options.DefaultAuthenticateScheme = "Bearer"; + options.DefaultAuthenticateScheme = JwtBearerDefaults.AuthenticationScheme; options.DefaultChallengeScheme = JwtBearerDefaults.AuthenticationScheme;AskFm/AskFm.API/Controllers/AuthController.cs (4)
33-34: Returning onlyErrorschanges the response shape; ensure clients can handle itSuccess returns
ServiceResult<T>, error now returnsList<string>. Consider a consistent envelope (e.g., RFC7807 ProblemDetails) or always returnServiceResult<T>.- return BadRequest(result.Errors); + return Problem(title: "Registration failed", + detail: string.Join("; ", result.Errors ?? new List<string>()), + statusCode: StatusCodes.Status400BadRequest);
50-51: Same response-shape concern for LoginAlign with the chosen error contract.
73-74: Same response-shape concern for RefreshTokenConsider unified error responses.
94-95: Same response-shape concern for LogoutEnsure consistency for API consumers.
AskFm/AskFm.API/Controllers/UserController.cs (1)
49-58: Prefer AutoMapper (or a mapper method) over manual projection; also fix DTO casing for followerCount.Manual mapping here drifts from conventions and risks divergence. If ReadUserDTO uses PascalCase (FollowerCount), align property name; if it intentionally uses lower camel-case, reconsider to match .NET naming.
Apply within this block:
- ReadUserDTO readUserDTO = new ReadUserDTO() - { - Name = result.Data.Name, - Email = result.Data.Email, - AvatarPath = result.Data.AvatarPath, - Bio = result.Data.Bio, - followerCount = result.Data.FollowersCount, - LastSeen = result.Data.LastSeen, - }; - return Ok(readUserDTO); + var dto = new ReadUserDTO + { + Name = result.Data.Name, + Email = result.Data.Email, + AvatarPath = result.Data.AvatarPath, + Bio = result.Data.Bio, + FollowerCount = result.Data.FollowersCount, // ensure DTO uses PascalCase + LastSeen = result.Data.LastSeen, + }; + return Ok(dto);Outside these lines, inject and use IMapper if already configured:
// ctor add: IMapper mapper private readonly IMapper _mapper; // assign in ctor return Ok(_mapper.Map<ReadUserDTO>(result.Data));AskFm/AskFm.BLL/Services/UserIdentityService/UserService.cs (1)
12-22: Mark injected dependencies as readonly.Makes intent explicit and prevents accidental reassignment.
- private IUnitOfWork _unitOfWork; - private readonly UserManager<ApplicationUser> _userManager; - private readonly IHttpContextAccessor _httpContextAccessor; + private readonly IUnitOfWork _unitOfWork; + private readonly UserManager<ApplicationUser> _userManager; + private readonly IHttpContextAccessor _httpContextAccessor;AskFm/AskFm.DAL/Migrations/20250825225059_AddCreatedAtDefault.cs (1)
14-85: LGTM: sensible CreatedAt defaults across tablesDefaults to GETUTCDATE() on insert are appropriate. Consider DATETIME2 in future for precision, but not blocking.
Also applies to: 87-161
AskFm/AskFm.BLL/Services/CommentService.cs (1)
8-12: Make _unitOfWork readonlyField is never reassigned; mark as readonly to communicate intent and thread-safety.
- private IUnitOfWork _unitOfWork; + private readonly IUnitOfWork _unitOfWork;AskFm/AskFm.BLL/Services/CommentLikeService.cs (4)
122-143: Pass CancellationToken, prefer UTC, and ensure SaveAsync uses ctMinor consistency and cancellation improvements.
- var newLike = new CommentLike + var newLike = new CommentLike { CommentId = commentId, UserId = userId, }; - await _unitOfWork.CommentLikes.AddAsync(newLike); + await _unitOfWork.CommentLikes.AddAsync(newLike, ct); comment.LikeCount++; _unitOfWork.Comments.Update(comment); - await _unitOfWork.SaveAsync(); + await _unitOfWork.SaveAsync(ct); _logger.LogInformation("Like added successfully for comment id: {CommentId}", commentId); return await ServiceResult<CommentLikeDto>.Success(new CommentLikeDto { CommentId = newLike.CommentId, UserId = newLike.UserId, UserName = userName, CreatedAt = newLike.CreatedAt });
1-8: Remove unnecessary usingsSystem.Runtime.InteropServices.JavaScript and Microsoft.VisualBasic are not used here.
-using System.Runtime.InteropServices.JavaScript; -using Microsoft.VisualBasic;
86-101: Decide on idempotent add semanticsConsider returning Success with existing like when already liked (idempotent) instead of Failure; this simplifies client logic and aligns with HTTP PUT-like behavior.
If you choose idempotency, replace the failure branch with:
- var errors = new List<String>() - { - "User has already liked this comment" - }; - return await ServiceResult<CommentLikeDto>.Failure(errors); + return await ServiceResult<CommentLikeDto>.Success(new CommentLikeDto + { + CommentId = existingLike.CommentId, + UserId = existingLike.UserId, + UserName = userName, + CreatedAt = existingLike.CreatedAt + });Update tests accordingly.
120-131: Enforce uniqueness at the DB level to prevent duplicate likes under concurrencyAdd a unique index on (CommentId, UserId) for CommentLikes to avoid races; with soft-delete, reuse the same row.
I can generate a migration adding:
- Unique index on (CommentId, UserId)
- Optional filtered index if you intend to physically delete instead of soft-delete.
AskFm/AskFm.API/Controllers/CommentController.cs (4)
13-13: Use JwtBearerDefaults instead of hard-coded scheme stringAvoid magic strings for auth schemes.
+using Microsoft.AspNetCore.Authentication.JwtBearer; @@ -[Authorize(AuthenticationSchemes = "Bearer")] +[Authorize(AuthenticationSchemes = JwtBearerDefaults.AuthenticationScheme)]
55-65: Normalize error payloads to a standard shapeSome endpoints return
{ message }, others return raw arrays. Prefer a single schema, e.g.,{ errors: [] }or RFC7807 ProblemDetails.
90-93: Minor: simplify route valuesTiny nit, but
new { id }is cleaner and equivalent.- new { id = id }, + new { id },
137-143: Consider 404 for "like not found" to make DELETE idempotent-friendlyIf the service signals "like not found", respond 404 (or 204 for idempotency). Today all failures map to 400.
- if (!result.success) - { - return BadRequest(result.Errors); - } + if (!result.success) + { + var errors = result.Errors ?? Array.Empty<string>(); + if (errors.Any(e => e.Contains("not found", StringComparison.OrdinalIgnoreCase))) + return NotFound(new { errors }); + return BadRequest(new { errors }); + }AskFm/AskFm.DAL/Migrations/20250825225059_AddCreatedAtDefault.Designer.cs (1)
55-60: Prefer datetime2 over DATETIMEFor SQL Server, datetime2 has better precision and range. Consider switching all timestamp columns to
datetime2.Also applies to: 157-161, 204-208, 235-239, 269-273, 316-320, 358-362, 404-407
AskFm/Tests/CommentLikeServiceTest.cs (5)
37-72: Strengthen assertions in AddLike testAlso assert success and LikeCount increment.
- // Act - await _commentLikeService.AddLikeAsync(commentId, userId); + // Arrange initial like count + comment.LikeCount = 0; + // Act + var result = await _commentLikeService.AddLikeAsync(commentId, userId); @@ - _mockUnitOfWork.Verify(uow => uow.SaveAsync(), Times.Once); + _mockUnitOfWork.Verify(uow => uow.SaveAsync(), Times.Once); + Assert.True(result.success); + Assert.Equal(1, comment.LikeCount);
74-97: Test name says "ThrowException" but the service returns a failure resultRename for accuracy and assert the error payload shape.
- public async Task AddLike_WhenCommentNotFoundOrDeleted_ThrowException() + public async Task AddLike_WhenCommentNotFoundOrDeleted_ReturnsFailure() @@ - Assert.False(result.success); + Assert.False(result.success); + Assert.NotNull(result.Errors);
100-151: Misleading test name; it deletes a like, not the commentRename and keep assertions. Consider asserting the comment itself is not marked deleted once the service bug (if any) is settled.
- public async Task DeleteLike_WhenDeleteByAuthor_CommentIsDeleted() + public async Task DeleteLike_WhenDeleteByAuthor_LikeIsRemoved()
153-207: Add minimal verification of DTO mapping (username/createdAt) if exposedCurrent assertions only cover ids. If DTO includes UserName/CreatedAt, assert at least non-null where applicable.
209-241: Also assert success flag for empty resultTiny completeness tweak.
- Assert.Empty(result.Data); + Assert.True(result.success); + Assert.Empty(result.Data);
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (24)
AskFm/AskFm.API/AskFm.API.csproj(1 hunks)AskFm/AskFm.API/Controllers/AuthController.cs(4 hunks)AskFm/AskFm.API/Controllers/CommentController.cs(1 hunks)AskFm/AskFm.API/Controllers/UserController.cs(1 hunks)AskFm/AskFm.API/Program.cs(2 hunks)AskFm/AskFm.API/Properties/launchSettings.json(2 hunks)AskFm/AskFm.BLL/AskFm.BLL.csproj(1 hunks)AskFm/AskFm.BLL/DTO/CommentLikeDto.cs(1 hunks)AskFm/AskFm.BLL/Services/CommentLikeService.cs(1 hunks)AskFm/AskFm.BLL/Services/CommentService.cs(1 hunks)AskFm/AskFm.BLL/Services/ICommentLikeService.cs(1 hunks)AskFm/AskFm.BLL/Services/ICommentService.cs(1 hunks)AskFm/AskFm.BLL/Services/UserIdentityService/IUserService.cs(2 hunks)AskFm/AskFm.BLL/Services/UserIdentityService/UserService.cs(2 hunks)AskFm/AskFm.DAL/AppDbContext.cs(1 hunks)AskFm/AskFm.DAL/Interfaces/IUnitOfWork.cs(1 hunks)AskFm/AskFm.DAL/Migrations/20250825225059_AddCreatedAtDefault.Designer.cs(1 hunks)AskFm/AskFm.DAL/Migrations/20250825225059_AddCreatedAtDefault.cs(1 hunks)AskFm/AskFm.DAL/Migrations/20250825230515_AddUpdatedAtAndDeletedAtDefault.Designer.cs(1 hunks)AskFm/AskFm.DAL/Migrations/20250825230515_AddUpdatedAtAndDeletedAtDefault.cs(1 hunks)AskFm/AskFm.DAL/Migrations/AppDbContextModelSnapshot.cs(13 hunks)AskFm/AskFm.DAL/UnitOfWork.cs(1 hunks)AskFm/Tests/CommentLikeServiceTest.cs(1 hunks)AskFm/Tests/Tests.csproj(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- AskFm/Tests/Tests.csproj
- AskFm/AskFm.BLL/DTO/CommentLikeDto.cs
- AskFm/AskFm.BLL/AskFm.BLL.csproj
- AskFm/AskFm.DAL/Interfaces/IUnitOfWork.cs
- AskFm/AskFm.DAL/UnitOfWork.cs
🔇 Additional comments (6)
AskFm/AskFm.API/AskFm.API.csproj (1)
10-10: No separate AutoMapper.Extensions.Microsoft.DependencyInjection package needed
AddAutoMapper is now included in the AutoMapper core package since v13.0; skip adding the legacy DI extension package. (docs.automapper.io, stackoverflow.com)Likely an incorrect or invalid review comment.
AskFm/AskFm.API/Properties/launchSettings.json (1)
7-9: Good DX improvement: auto-launch SwaggerEnabling
launchBrowserwithlaunchUrl: "swagger"eases manual testing.Also applies to: 17-19
AskFm/AskFm.API/Controllers/UserController.cs (1)
40-40: Route change to "profile" is a breaking API change—coordinate clients and docs.If this replaces "current-user", update API docs and client calls; consider adding an [Obsolete] alias route temporarily to avoid breaking consumers.
AskFm/AskFm.DAL/Migrations/20250825230515_AddUpdatedAtAndDeletedAtDefault.cs (2)
14-21: No action needed: theSaveChangesoverrides invoke_fillTrackableInfromation, which setsUpdatedAtonModifiedentries inAppDbContext—updates are properly stamped.
159-305: No changes required for DeletedAt in Down migrationThe Down method already includes
oldDefaultValueSql: "GETUTCDATE()"on eachDeletedAtcolumn, which correctly reverts the default constraint and restores prior state—no further edits needed.Likely an incorrect or invalid review comment.
AskFm/AskFm.API/Controllers/CommentController.cs (1)
45-51: Return only the Data payload for GET likes
ModifyGetAllLikesto returnOk(likes.Data)and wrap failures asBadRequest(new { errors = likes.Errors })so the GET response shape matches POST.
| var user = await _userService.GetCurrentUserAsync(); | ||
|
|
||
| if (!user.success) | ||
| { | ||
| return BadRequest(user.Errors); | ||
| } | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Map "no current user" to 401, not 400
If the current user cannot be resolved, respond with Unauthorized.
- if (!user.success)
- {
- return BadRequest(user.Errors);
- }
+ if (!user.success)
+ return Unauthorized(new { errors = user.Errors });📝 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.
| var user = await _userService.GetCurrentUserAsync(); | |
| if (!user.success) | |
| { | |
| return BadRequest(user.Errors); | |
| } | |
| var user = await _userService.GetCurrentUserAsync(); | |
| if (!user.success) | |
| return Unauthorized(new { errors = user.Errors }); |
🤖 Prompt for AI Agents
In AskFm/AskFm.API/Controllers/CommentController.cs around lines 77 to 83, the
code returns BadRequest when the current user cannot be resolved; change this to
return an Unauthorized (401) response instead. Replace the
BadRequest(user.Errors) return with Unauthorized(user.Errors) (or at minimum
Unauthorized()) so the API maps "no current user" to HTTP 401.
| if (user==null || !user.success) | ||
| return BadRequest(user.Errors); | ||
|
|
There was a problem hiding this comment.
NRE risk when user is null; also wrong status code
You dereference user.Errors even when user == null. Return 401 for null user, and 400 only for a valid result that failed validation.
- if (user==null || !user.success)
- return BadRequest(user.Errors);
+ if (user is null)
+ return Unauthorized();
+ if (!user.success)
+ return BadRequest(new { errors = user.Errors });📝 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.
| if (user==null || !user.success) | |
| return BadRequest(user.Errors); | |
| if (user is null) | |
| return Unauthorized(); | |
| if (!user.success) | |
| return BadRequest(new { errors = user.Errors }); | |
🤖 Prompt for AI Agents
In AskFm/AskFm.API/Controllers/CommentController.cs around lines 124-126, the
code dereferences user.Errors even when user is null and uses the wrong status
code; change the logic to first check if user == null and return Unauthorized()
(401) in that case, then only if user is non-null and user.success is false
return BadRequest(user.Errors); ensure the null check occurs before accessing
user.Errors so no NRE can occur.
| if (comment == null || !user.success) | ||
| return BadRequest(user.Errors); | ||
|
|
There was a problem hiding this comment.
Incorrect condition; comment null should yield 404, not reuse user.success
You re-check user.success and return user.Errors. Return NotFound for a missing comment.
- if (comment == null || !user.success)
- return BadRequest(user.Errors);
+ if (comment is null)
+ return NotFound(new { message = $"Comment with id {id} not found" });Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In AskFm/AskFm.API/Controllers/CommentController.cs around lines 131 to 133, the
current combined condition incorrectly treats a null comment as a BadRequest
tied to user.success; split the checks so that if comment == null you return
NotFound(), and separately if !user.success you return BadRequest(user.Errors);
update the control flow accordingly to ensure the correct response codes are
returned for each case.
| public async Task<ServiceResult<IEnumerable<CommentLikeDto>>> GetLikesForCommentAsync(int commentId) | ||
| { | ||
| try | ||
| { | ||
| _logger.LogInformation("Retrieving likes for comment id: {CommentId}", commentId); | ||
|
|
||
| var comment = await _unitOfWork.Comments.GetByIdAsync(commentId); | ||
| if (comment == null) | ||
| { | ||
| throw new ArgumentException($"Comment with id {commentId} not found"); | ||
| } | ||
|
|
||
| var likes = await _unitOfWork.CommentLikes.FindAllAsync( | ||
| predicate: cl => cl.CommentId == commentId && !cl.IsDeleted | ||
| ); | ||
|
|
||
| var likeDtos = likes.Select(like => new CommentLikeDto | ||
| { | ||
| CommentId = like.CommentId, | ||
| UserId = like.UserId, | ||
| CreatedAt = like.CreatedAt, | ||
| UserName = like.User?.UserName | ||
| }); | ||
|
|
||
| return await ServiceResult<IEnumerable<CommentLikeDto>>.Success(likeDtos); | ||
| } | ||
| catch (Exception ex) | ||
| { | ||
| _logger.LogError(ex, "Error retrieving likes for comment id: {CommentId}", commentId); | ||
| return await ServiceResult<IEnumerable<CommentLikeDto>>.Failure(new List<string>() { ex.Message }); | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Load usernames reliably and add CancellationToken passthrough
If lazy-loading is off, like.User may be null. Project the username via a join/include or fetch it explicitly. Also add CT.
- public async Task<ServiceResult<IEnumerable<CommentLikeDto>>> GetLikesForCommentAsync(int commentId)
+ public async Task<ServiceResult<IEnumerable<CommentLikeDto>>> GetLikesForCommentAsync(int commentId, CancellationToken ct = default)
{
try
{
_logger.LogInformation("Retrieving likes for comment id: {CommentId}", commentId);
- var comment = await _unitOfWork.Comments.GetByIdAsync(commentId);
+ var comment = await _unitOfWork.Comments.GetByIdAsync(commentId, ct);
if (comment == null)
{
throw new ArgumentException($"Comment with id {commentId} not found");
}
- var likes = await _unitOfWork.CommentLikes.FindAllAsync(
- predicate: cl => cl.CommentId == commentId && !cl.IsDeleted
- );
+ var likes = await _unitOfWork.CommentLikes.FindAllAsync(
+ predicate: cl => cl.CommentId == commentId && !cl.IsDeleted,
+ include: q => q.Include(x => x.User),
+ cancellationToken: ct
+ );📝 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 async Task<ServiceResult<IEnumerable<CommentLikeDto>>> GetLikesForCommentAsync(int commentId) | |
| { | |
| try | |
| { | |
| _logger.LogInformation("Retrieving likes for comment id: {CommentId}", commentId); | |
| var comment = await _unitOfWork.Comments.GetByIdAsync(commentId); | |
| if (comment == null) | |
| { | |
| throw new ArgumentException($"Comment with id {commentId} not found"); | |
| } | |
| var likes = await _unitOfWork.CommentLikes.FindAllAsync( | |
| predicate: cl => cl.CommentId == commentId && !cl.IsDeleted | |
| ); | |
| var likeDtos = likes.Select(like => new CommentLikeDto | |
| { | |
| CommentId = like.CommentId, | |
| UserId = like.UserId, | |
| CreatedAt = like.CreatedAt, | |
| UserName = like.User?.UserName | |
| }); | |
| return await ServiceResult<IEnumerable<CommentLikeDto>>.Success(likeDtos); | |
| } | |
| catch (Exception ex) | |
| { | |
| _logger.LogError(ex, "Error retrieving likes for comment id: {CommentId}", commentId); | |
| return await ServiceResult<IEnumerable<CommentLikeDto>>.Failure(new List<string>() { ex.Message }); | |
| } | |
| } | |
| public async Task<ServiceResult<IEnumerable<CommentLikeDto>>> GetLikesForCommentAsync(int commentId, CancellationToken ct = default) | |
| { | |
| try | |
| { | |
| _logger.LogInformation("Retrieving likes for comment id: {CommentId}", commentId); | |
| var comment = await _unitOfWork.Comments.GetByIdAsync(commentId, ct); | |
| if (comment == null) | |
| { | |
| throw new ArgumentException($"Comment with id {commentId} not found"); | |
| } | |
| var likes = await _unitOfWork.CommentLikes.FindAllAsync( | |
| predicate: cl => cl.CommentId == commentId && !cl.IsDeleted, | |
| include: q => q.Include(x => x.User), | |
| cancellationToken: ct | |
| ); | |
| var likeDtos = likes.Select(like => new CommentLikeDto | |
| { | |
| CommentId = like.CommentId, | |
| UserId = like.UserId, | |
| CreatedAt = like.CreatedAt, | |
| UserName = like.User?.UserName | |
| }); | |
| return await ServiceResult<IEnumerable<CommentLikeDto>>.Success(likeDtos); | |
| } | |
| catch (Exception ex) | |
| { | |
| _logger.LogError(ex, "Error retrieving likes for comment id: {CommentId}", commentId); | |
| return await ServiceResult<IEnumerable<CommentLikeDto>>.Failure(new List<string> { ex.Message }); | |
| } | |
| } |
| public async Task<ServiceResult<CommentLikeDto>> AddLikeAsync(int commentId, int userId) | ||
| { | ||
| try | ||
| { | ||
| _logger.LogInformation("Adding like for comment id: {CommentId} by user id: {UserId}", | ||
| commentId, userId); | ||
|
|
||
| var comment = await _unitOfWork.Comments.GetByIdAsync(commentId); | ||
| var user = await _unitOfWork.Users.GetByIdAsync(userId); | ||
| if (user == null) | ||
| { | ||
| var errors = new List<String>() | ||
| { | ||
| $"User with id {userId} not found" | ||
| }; | ||
| return await ServiceResult<CommentLikeDto>.Failure(errors); | ||
| } | ||
|
|
||
| string userName = user.UserName; | ||
| if (comment == null) | ||
| { | ||
| var errors = new List<String>() | ||
| { | ||
| $"Comment with id {commentId} not found" | ||
| }; | ||
| return await ServiceResult<CommentLikeDto>.Failure(errors); | ||
| } | ||
|
|
||
| var existingLike = await _unitOfWork.CommentLikes.FindAsync( | ||
| cl => cl.CommentId == commentId && cl.UserId == userId | ||
| ); | ||
|
|
||
| // if the CommentLike already exit in the DB | ||
| if (existingLike != null) | ||
| { | ||
| // if the The use already liked this comment | ||
| if (!existingLike.IsDeleted) | ||
| { | ||
| var errors = new List<String>() | ||
| { | ||
| "User has already liked this comment" | ||
| }; | ||
| return await ServiceResult<CommentLikeDto>.Failure(errors); | ||
| } | ||
|
|
||
| // otherwise , the user liked the commend , then unliked it , and then wants to like it again | ||
| existingLike.IsDeleted = false; | ||
| comment.LikeCount++; | ||
| _unitOfWork.Comments.Update(comment); | ||
| await _unitOfWork.SaveAsync(); | ||
| _logger.LogInformation("Like added successfully for comment id: {CommentId}", commentId); | ||
|
|
||
|
|
||
| // updating the createdAt column to Now , ignoring the first time the user liked the comment | ||
| existingLike.CreatedAt = DateTime.Now; | ||
| return await ServiceResult<CommentLikeDto>.Success(new CommentLikeDto | ||
| { | ||
| CommentId = existingLike.CommentId, | ||
| UserId = existingLike.UserId, | ||
| UserName = userName, | ||
| CreatedAt = existingLike.CreatedAt | ||
| }); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Reactivation path doesn’t persist IsDeleted/CreatedAt; use UtcNow and update the like entity
When re-liking, you never call Update(existingLike), and you set CreatedAt after SaveAsync (not persisted). Also prefer UTC.
- public async Task<ServiceResult<CommentLikeDto>> AddLikeAsync(int commentId, int userId)
+ public async Task<ServiceResult<CommentLikeDto>> AddLikeAsync(int commentId, int userId, CancellationToken ct = default)
{
try
{
_logger.LogInformation("Adding like for comment id: {CommentId} by user id: {UserId}",
commentId, userId);
- var comment = await _unitOfWork.Comments.GetByIdAsync(commentId);
- var user = await _unitOfWork.Users.GetByIdAsync(userId);
+ var comment = await _unitOfWork.Comments.GetByIdAsync(commentId, ct);
+ var user = await _unitOfWork.Users.GetByIdAsync(userId, ct);
if (user == null)
{
var errors = new List<String>()
{
$"User with id {userId} not found"
};
return await ServiceResult<CommentLikeDto>.Failure(errors);
}
string userName = user.UserName;
if (comment == null)
{
var errors = new List<String>()
{
$"Comment with id {commentId} not found"
};
return await ServiceResult<CommentLikeDto>.Failure(errors);
}
var existingLike = await _unitOfWork.CommentLikes.FindAsync(
- cl => cl.CommentId == commentId && cl.UserId == userId
+ cl => cl.CommentId == commentId && cl.UserId == userId,
+ ct
);
// if the CommentLike already exit in the DB
if (existingLike != null)
{
// if the The use already liked this comment
if (!existingLike.IsDeleted)
{
var errors = new List<String>()
{
"User has already liked this comment"
};
return await ServiceResult<CommentLikeDto>.Failure(errors);
}
// otherwise , the user liked the commend , then unliked it , and then wants to like it again
- existingLike.IsDeleted = false;
+ existingLike.IsDeleted = false;
+ existingLike.CreatedAt = DateTime.UtcNow;
+ _unitOfWork.CommentLikes.Update(existingLike);
comment.LikeCount++;
_unitOfWork.Comments.Update(comment);
- await _unitOfWork.SaveAsync();
+ await _unitOfWork.SaveAsync(ct);
_logger.LogInformation("Like added successfully for comment id: {CommentId}", commentId);
-
-
- // updating the createdAt column to Now , ignoring the first time the user liked the comment
- existingLike.CreatedAt = DateTime.Now;
return await ServiceResult<CommentLikeDto>.Success(new CommentLikeDto
{
CommentId = existingLike.CommentId,
UserId = existingLike.UserId,
UserName = userName,
CreatedAt = existingLike.CreatedAt
});
}| .HasColumnType("bit"); | ||
|
|
||
| b.Property<string>("Message") | ||
| .IsRequired() | ||
| .HasColumnType("NVARCHAR"); | ||
|
|
||
| b.Property<int>("ResourceId") |
There was a problem hiding this comment.
Invalid SQL type "NVARCHAR" without length for Notification.Message
SQL Server requires NVARCHAR(n) or NVARCHAR(MAX). Current mapping risks truncation or invalid DDL.
- b.Property<string>("Message")
- .IsRequired()
- .HasColumnType("NVARCHAR");
+ b.Property<string>("Message")
+ .IsRequired()
+ .HasColumnType("NVARCHAR(MAX)");Fix in your entity configuration (preferred), then regenerate migrations:
// In Notification configuration
builder.Property(n => n.Message)
.HasColumnType("nvarchar(max)")
.IsRequired();🤖 Prompt for AI Agents
In AskFm/AskFm.DAL/Migrations/20250825225059_AddCreatedAtDefault.Designer.cs
around lines 283-289 the Notification.Message column is configured with
HasColumnType("NVARCHAR") which is invalid without a length; update the
Notification entity configuration to specify a valid SQL Server type (for
example set HasColumnType("nvarchar(max)") or use .IsUnicode().HasMaxLength(...)
instead) and then regenerate the migration so the designer file gets the
corrected column type.
| migrationBuilder.AlterColumn<DateTime>( | ||
| name: "DeletedAt", | ||
| table: "Thread", | ||
| type: "DATETIME", | ||
| nullable: false, | ||
| defaultValueSql: "GETUTCDATE()", | ||
| oldClrType: typeof(DateTime), | ||
| oldType: "DATETIME"); |
There was a problem hiding this comment.
Do not default DeletedAt to GETUTCDATE() and make it nullable (soft-delete semantics)
Current migration sets DeletedAt as non-null with a UTC default, which marks every row as “deleted” at insert time. DeletedAt should be nullable with no default.
Apply the pattern below to all DeletedAt alterations in this migration:
- migrationBuilder.AlterColumn<DateTime>(
+ migrationBuilder.AlterColumn<DateTime>(
name: "DeletedAt",
table: "Thread",
type: "DATETIME",
- nullable: false,
- defaultValueSql: "GETUTCDATE()",
+ nullable: true,
oldClrType: typeof(DateTime),
- oldType: "DATETIME");
+ oldType: "DATETIME",
+ oldNullable: false,
+ oldDefaultValueSql: "GETUTCDATE()");And for Comments/CommentLikes similarly:
- migrationBuilder.AlterColumn<DateTime>(
+ migrationBuilder.AlterColumn<DateTime>(
name: "DeletedAt",
table: "Comments",
type: "DATETIME",
- nullable: false,
- defaultValueSql: "GETUTCDATE()",
+ nullable: true,
oldClrType: typeof(DateTime),
- oldType: "DATETIME");
+ oldType: "DATETIME",
+ oldNullable: false,
+ oldDefaultValueSql: "GETUTCDATE()");Also applies to: 113-121, 131-139, 149-157
🤖 Prompt for AI Agents
In AskFm/AskFm.DAL/Migrations/20250825230515_AddUpdatedAtAndDeletedAtDefault.cs
around lines 41 to 48 (and also for the other DeletedAt alterations at 113-121,
131-139, 149-157), the migration currently changes DeletedAt to non-nullable
with defaultValueSql "GETUTCDATE()", which effectively marks all new rows as
deleted; change the AlterColumn calls so DeletedAt is nullable (nullable: true),
remove any defaultValue or defaultValueSql for DeletedAt, and update
oldClrType/oldType/oldNullable fields accordingly so the migration represents
nullable soft-delete semantics; apply the same change pattern to the Comment and
CommentLike DeletedAt alterations in the listed line ranges.
| b.Property<DateTime>("DeletedAt") | ||
| .ValueGeneratedOnAdd() | ||
| .HasColumnType("DATETIME") | ||
| .HasDefaultValueSql("GETUTCDATE()"); | ||
|
|
There was a problem hiding this comment.
Critical: DeletedAt defaulting to GETUTCDATE() marks new rows as deleted
Soft-delete semantics break if DeletedAt is auto-populated on insert. Remove the default and allow null until deletion.
- b.Property<DateTime>("DeletedAt")
- .ValueGeneratedOnAdd()
- .HasColumnType("DATETIME")
- .HasDefaultValueSql("GETUTCDATE()");
+ b.Property<DateTime?>("DeletedAt")
+ .HasColumnType("datetime2");Apply to all entities above. Change CLR type to nullable in your models and fluent config, then add a corrective migration to drop defaults and set nulls.
Also applies to: 166-170, 217-221, 252-256, 290-294, 341-345, 387-391, 436-440
🤖 Prompt for AI Agents
In
AskFm/AskFm.DAL/Migrations/20250825230515_AddUpdatedAtAndDeletedAtDefault.Designer.cs
around lines 60-64 (and similarly at 166-170, 217-221, 252-256, 290-294,
341-345, 387-391, 436-440), the DeletedAt property is incorrectly given a
GETUTCDATE() default causing new rows to be marked deleted; remove the
.HasDefaultValueSql("GETUTCDATE()") from the migration and from any model
builder configuration, change the CLR type for DeletedAt properties in your
entity models to DateTime? (nullable) and update the fluent configuration to
allow nulls (no default), then create a corrective migration that drops the
default constraint(s) on the DeletedAt columns and updates existing rows if
needed (set unintended non-null defaults to NULL), and apply the new migration
so the database and EF model are consistent.
| b.Property<DateTime>("UpdatedAt") | ||
| .ValueGeneratedOnAdd() | ||
| .HasColumnType("DATETIME") | ||
| .HasDefaultValueSql("GETUTCDATE()"); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
UpdatedAt default only on insert; ensure it changes on update
ValueGeneratedOnAdd + default sets UpdatedAt once. Use an EF interceptor/SaveChanges override or a DB trigger to update on modification; alternatively configure ValueGeneratedOnAddOrUpdate and set the value in code.
Example SaveChanges override:
public override Task<int> SaveChangesAsync(CancellationToken ct = default)
{
var now = DateTime.UtcNow;
foreach (var e in ChangeTracker.Entries().Where(e => e.State is EntityState.Added or EntityState.Modified))
{
if (e.Properties.Any(p => p.Metadata.Name == "UpdatedAt"))
e.CurrentValues["UpdatedAt"] = now;
if (e.State == EntityState.Added && e.Properties.Any(p => p.Metadata.Name == "CreatedAt"))
e.CurrentValues["CreatedAt"] = now;
}
return base.SaveChangesAsync(ct);
}Also applies to: 185-189, 228-231, 265-269, 313-317, 406-410, 446-450
🤖 Prompt for AI Agents
In
AskFm/AskFm.DAL/Migrations/20250825230515_AddUpdatedAtAndDeletedAtDefault.Designer.cs
around lines 122-126 (and also update occurrences at 185-189, 228-231, 265-269,
313-317, 406-410, 446-450), the UpdatedAt property is configured with
ValueGeneratedOnAdd and a default GETUTCDATE(), which only sets the timestamp on
insert; change this so UpdatedAt is refreshed on every update by either (A)
replacing ValueGeneratedOnAdd with ValueGeneratedOnAddOrUpdate and ensure
application code sets UpdatedAt on SaveChanges/SaveChangesAsync before saving,
or (B) implement an EF SaveChanges override or interceptor that sets UpdatedAt =
DateTime.UtcNow for Added and Modified entries (and sets CreatedAt only for
Added), or (C) add a database trigger to update the column on UPDATE; pick one
approach and apply it consistently to all listed locations.
| b.Property<DateTime>("DeletedAt") | ||
| .HasColumnType("DATETIME"); | ||
| .ValueGeneratedOnAdd() | ||
| .HasColumnType("DATETIME") | ||
| .HasDefaultValueSql("GETUTCDATE()"); | ||
|
|
There was a problem hiding this comment.
Defaulting DeletedAt to GETUTCDATE() and treating UpdatedAt as ValueGeneratedOnAdd is incorrect.
- DeletedAt should be nullable and default to NULL; setting a non-null default implies every row is “deleted” at insert time.
- UpdatedAt needs to change on updates; GETUTCDATE() as a default only runs on insert. ValueGeneratedOnAdd prevents updates.
Action:
- Make DeletedAt nullable with no default.
- Keep CreatedAt default GETUTCDATE().
- For UpdatedAt, either:
- Update in SaveChanges/SaveChangesAsync, or
- Use a computed column or trigger (if DB-driven updates are required).
Example model config (adjusted in entity configurations, then regenerate migrations):
// CreatedAt
b.Property(x => x.CreatedAt)
.HasColumnType("datetime2")
.HasDefaultValueSql("GETUTCDATE()")
.ValueGeneratedOnAdd();
// UpdatedAt
b.Property(x => x.UpdatedAt)
.HasColumnType("datetime2")
.ValueGeneratedOnAddOrUpdate();
// DeletedAt
b.Property(x => x.DeletedAt)
.HasColumnType("datetime2")
.IsRequired(false); // nullable, no defaultExample SaveChanges override:
public override Task<int> SaveChangesAsync(CancellationToken ct = default)
{
var now = DateTime.UtcNow;
foreach (var e in ChangeTracker.Entries().Where(e => e.State is EntityState.Added or EntityState.Modified))
{
if (e.Metadata.FindProperty("UpdatedAt") != null)
e.CurrentValues["UpdatedAt"] = now;
}
return base.SaveChangesAsync(ct);
}Follow up with new migrations to drop defaults on DeletedAt and fix UpdatedAt semantics.
Also applies to: 119-123, 163-167, 182-186, 214-228, 249-268, 289-306, 337-351, 383-406, 432-446
🤖 Prompt for AI Agents
AskFm/AskFm.DAL/Migrations/AppDbContextModelSnapshot.cs lines 57-61 (and
similarly at 119-123, 163-167, 182-186, 214-228, 249-268, 289-306, 337-351,
383-406, 432-446): DeletedAt is incorrectly non-nullable with a GETUTCDATE()
default and UpdatedAt is incorrectly configured as ValueGeneratedOnAdd; change
DeletedAt to be nullable with no default, keep CreatedAt as ValueGeneratedOnAdd
with GETUTCDATE(), and change UpdatedAt to be updated on each update (either
mark as ValueGeneratedOnAddOrUpdate if using a DB-side computed/trigger solution
or remove ValueGeneratedOnAdd and set UpdatedAt in SaveChanges/SaveChangesAsync
by assigning DateTime.UtcNow on Added/Modified entries). After adjusting the
entity configuration, regenerate migrations to remove the GETUTCDATE() default
from DeletedAt and correct UpdatedAt semantics across all affected model
snapshot locations.
4 participants
Summary by CodeRabbit