Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Aait.bac.g2a.fikremariam.allfeatures #267

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

fikreanteneh
Copy link
Contributor

No description provided.

var validationResult = await validator.ValidateAsync(request.UpdateCommentDto);

Console.WriteLine(request.UpdateCommentDto);
Copy link

@abrahamshimekt abrahamshimekt Aug 27, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove this line:
Console.WriteLine(request.UpdateCommentDto);

@@ -21,11 +21,13 @@ public UpdateCommentCommandHandler(IUnitOfWork unitOfWork, IMapper mapper)
public async Task<Unit> Handle(UpdateCommentCommand request, CancellationToken cancellationToken)
{
var validator = new UpdateCommentDtoValidator(_unitOfWork.commentRepository);
Console.WriteLine("reached handler");
Copy link

@abrahamshimekt abrahamshimekt Aug 27, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove this line :
Console.WriteLine("reached handler");

{
throw new ValidationException(validationResult.Errors);
{
Console.WriteLine("validation");

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove :
Console.WriteLine("validation");

throw new ValidationException(validationResult.Errors);
{
Console.WriteLine("validation");
throw new Exception();

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please specify the exception type.

@@ -19,6 +19,7 @@ public DeleteLikeCommandHandler(IUnitOfWork unitOfWork, IMapper mapper)

public async Task<Unit> Handle(DeleteLikeCommand request, CancellationToken cancellationToken)
{
Console.WriteLine("-------------------$");

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove : Console.WriteLine("-------------------$");

@@ -19,6 +19,7 @@ public DeleteLikeCommandHandler(IUnitOfWork unitOfWork, IMapper mapper)

public async Task<Unit> Handle(DeleteLikeCommand request, CancellationToken cancellationToken)
{
Console.WriteLine("-------------------$");
await _unitOfWork.likeRepository.UnlikePost(_mapper.Map<Domain.Entities.Like>(request.like));
await _unitOfWork.Save();

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

check change was made in the db

}

public async Task<List<PostDto>> Handle(GetLikedPostRequest request, CancellationToken cancellationToken){
var posts = await _unitOfWork.likeRepository.GetLikedPost(request.Id);
Copy link

@abrahamshimekt abrahamshimekt Aug 27, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check whether the posts list is null or empty before mapping it. null pointer exception will be thrown if you try to map a null. change you query and queryHandler

private readonly IMapper _mapper;

public GetLikedPostRequestHandler(IUnitOfWork unitOfWork, IMapper mapper){
_unitOfWork = unitOfWork;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove unit of work injection


namespace Application.Features.Like.Handlers.Query;

public class GetLikedPostRequestHandler : IRequestHandler<GetLikedPostRequest, List<PostDto>>{

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if you want a liked post from likeRepository wtih post id , the dto you're gonna return is not a list of postDto

// throw Exception("Post Not Found");

if (post == null || post.UserId != request.UserId){
throw new Exception("You Don't Have Access");
}
await _unitOfWork.postRepository.Delete(post);
await _unitOfWork.Save();

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

check whether the db was actually affected

throw new ValidationException(validationResult);
}
var user = _mapper.Map<Domain.Entities.User>(request.CreateUser);
await _unitOfWork.userRepository.Add(user);
Console.WriteLine("reached Here");
Copy link

@abrahamshimekt abrahamshimekt Aug 27, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove : Console.WriteLine("reached Here"); don't push print codes to remote repository. remove them once you're ready to push to remote repo.

throw new ValidationException(validationResult);
}
var user = _mapper.Map<Domain.Entities.User>(request.CreateUser);
await _unitOfWork.userRepository.Add(user);
Console.WriteLine("reached Here");
var token = await _authService.Register(request.CreateUser, _unitOfWork.userRepository);
Console.WriteLine("---------------AuthServise");

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove:
Console.WriteLine("---------------AuthServise");

var token = await _authService.Register(request.CreateUser, _unitOfWork.userRepository);
Console.WriteLine("---------------AuthServise");
if (token is null){
throw new Exception();

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tell the exception type that was thrown

if (token is null){
throw new Exception();
}

await _unitOfWork.Save();

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

check if the user was created in the db

var user = await _unitOfWork.userRepository.Get(request.updateUser.Id);
_mapper.Map(request.updateUser, user);
// var user = _mapper.Map<Domain.Entities.User>(request.updateUser);
if (await _authService.Update(request.updateUser, user.Email)){
throw new Exception("Can't be Comleted");

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tell the exception type

throw new Exception("Can't be Comleted");

}
user = _mapper.Map<Domain.Entities.User>(request.updateUser);
await _unitOfWork.userRepository.Update(user);
await _unitOfWork.Save();

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

check whether the user was updated or not

public async Task<UserProfileDTO> Handle(GetUserProfile request, CancellationToken cancellationToken){
var user = await _unitOfWork.userRepository.Get(request.Id);
if (user is null){
throw new Exception("Not Found");

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tell the exception type to be thrown


public async Task<List<UserDto>> Handle(GetUsers request, CancellationToken cancellationToken){
var user = await _unitOfWork.userRepository.GetAll();
return _mapper.Map<List<UserDto>>(user);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

change the variable user to users and check it if it is null or empty


public async Task<AuthResponse> Handle(Login request, CancellationToken cancellationToken){
Console.WriteLine("------------Started");
var token = await _authService.Login(new AuthRequest(){

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove: Console.WriteLine("---------------AuthServise");


public LoginHandler(IUnitOfWork unitOfWork, IMapper mapper, IAuthService authService)
{
_mapper = mapper;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove the mapper

@@ -0,0 +1,223 @@
using System;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

never push migration files. delete them before pushing.

{
var user = await _userManager.FindByEmailAsync(request.Email);
if (user is null){
Console.WriteLine("-------------------User Not Found ");

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove : Console.WriteLine("-------------------User Not Found ");

// throw new NotFoundException("Account",req.Email);
}

var isCorrect = await _signInManager.PasswordSignInAsync(userName: user.UserName, request.Password,isPersistent:true, lockoutOnFailure:false);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

change the isCorrect variable to something like loginResponse


var isCorrect = await _signInManager.PasswordSignInAsync(userName: user.UserName, request.Password,isPersistent:true, lockoutOnFailure:false);
if (!isCorrect.Succeeded){
Console.WriteLine("-------------------Creditial Not Found ");

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove : Console.WriteLine("-------------------Creditial Not Found ");

// throw new Exception($"invalid credentials for user: {request.Email}");
}

Console.WriteLine("-------------UseR Repository started");

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove:
Console.WriteLine("-------------UseR Repository started");


var customeUser = await userRepository.GetUserByEmail(user.Email);

Console.WriteLine("---------------OKKKKKKKKKKKKK JWT");

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove: Console.WriteLine("---------------OKKKKKKKKKKKKK JWT");

Console.WriteLine("---------------OKKKKKKKKKKKKK JWT");
JwtSecurityToken token = await GenerateToken(user, customeUser.Id.ToString());
var Token = new JwtSecurityTokenHandler().WriteToken(token);
Console.WriteLine("---------------OKKKKKKKKKKKKK JWT");

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove : Console.WriteLine("---------------OKKKKKKKKKKKKK JWT");

var alreadyExist = await _userManager.FindByEmailAsync(req.Email);
if (alreadyExist is not null){
Console.WriteLine("----Email Exist");
return null;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if user already exist , email already exist error has to thrown

var creatingUser = await _userManager.CreateAsync(user, req.Password);
if (!creatingUser.Succeeded){
Console.WriteLine("Create Error ---------------");
Console.WriteLine(creatingUser.ToString());

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove the print lines

@@ -0,0 +1,8 @@
namespace Application.Model;

public class @string{

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is generally recommended to use meaningful and descriptive names that do not conflict with reserved keywords to improve code readability and maintainability.

await _unitOfWork.Save();
return user.Id;

return new AuthResponse(){ Id = user.Id, Email = user.Email, Username = user.UserName};

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for registering the user you can return the Id of the user instead of the AuthResponse

using Domain;
using MediatR;

namespace Application.Features.User.Request.Commands
{
public class CreateUserCommand : IRequest<int>
public class CreateUserCommand : IRequest<AuthResponse>
Copy link

@abrahamshimekt abrahamshimekt Aug 27, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here you can promise to return registration response like : public class RegistrationResponse
{
public string UserId { get; set; }
}

@@ -14,16 +14,16 @@ public class MappingProfile : AutoMapper.Profile

CreateMap<Post, PostDto>().ReverseMap();
CreateMap<Post, UpdatePostDto>().ReverseMap();
CreateMap<Post, CreateUserDTO>().ReverseMap();
CreateMap<Post, CreatePostDto>().ReverseMap();

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if there is not scenario that you are going to map Entity to CreateEntityDto or UpdateEntityDto , map the creeateEntityDto or UpdateEntityDto to Entity. you don't need to reverseMap(). example
CreateMap<CreatePostDto,Post>();

public required int CreatedAt{ get; set; }
public required int UpdatedAt{ get; set; }
public required DateTime CreatedAt{ get; set; }
public required DateTime UpdatedAt{ get; set; }

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of having CreatedAt and UpdateAt fields in the Comment, you can create a BaseEntity with Id, createdAt and updatedAt fields . Then make all of the Entities tracked in the database as an entry to extend from the base entity.

@@ -8,8 +8,8 @@ public class Comment
//The User That Commented
public required int UserId { get; set; }

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can remove required keyword if you have validation for creating and updating

public required int CreatedAt{ get; set; }
public required int UpdatedAt{ get; set; }
public required DateTime CreatedAt{ get; set; }
public required DateTime UpdatedAt{ get; set; }

//Navigation Property
public virtual User? User{ get; set; }

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If there is no derived class from Comment, there won't be a class to override the virtual property . so you can remove the virtual keyword

return null;
// throw new Exception($"Failed to register \n");
}
Console.WriteLine("User Registered");

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove: Console.WriteLine("User Registered");

// throw new Exception($"Failed to register \n");
}
Console.WriteLine("User Registered");
AuthRequest request = new AuthRequest {Email = req.Email, Password = req.Password };

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since you aren't using "AuthRequest request = new AuthRequest {Email = req.Email, Password = req.Password }; " delete

@@ -18,6 +18,7 @@ public PostRepository(SocialSyncDbContext context) : base(context)
}

public async Task<List<Post>> GetBytag(string tag){
Console.WriteLine("");

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove the print line

[HttpPost("register")]
public async Task<IActionResult> CreateUser(CreateUserDTO userDto)
[HttpPost("Register")]
public async Task<IActionResult> CreateUser( CreateUserDTO userDto)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To deserialize the JSON data from the request body and bind it to the CreateUserDTO object, you have to include [FromBody] to the CreateUser function.

{
var command = new GetUserProfile() { Id = UserId };
var token = await _mediator.Send(command);
return Ok(token);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The getProfile Action must return the user details

{
var command = new GetUsers();
var token = await _mediator.Send(command);
return Ok(token);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GetUsers function must return the list of users

[HttpGet("{Search}")]
public async Task<IActionResult> GetByContent( string search)
[HttpGet]
public async Task<IActionResult> GetPosts([FromQuery] string? search, [FromQuery] string? tag)
{

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the search is by the content of the post or the tag it has, you should a separate command that contains two fields : example
public classs SearchCommand : IRequest<List>{
public string Content {get;set;}
public string Tag {get;set;}
}
create a handler for this command:
create a search method in your post repository that filers posts by content or tag:
the search you implemented is totally incorrect :let pick one simple example if the user pass content and tag to search , you code is not going to search posts that have the tag provided , because it just return the posts that contain the given content only.

  • instead of if else condition, a single command handler can make your life easier



[HttpPut("{PostId}")]
public async Task<IActionResult> UpdatePost([FromBody] UpdatePostDto updatePost, int UserId, int PostId){

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you don't need to pass the userId and PostId to updatePost Action , you can define them in UpdatePostDto as field.



[HttpPost("{PostId}")]
public async Task<ActionResult> AddLike( int PostId)
Copy link

@abrahamshimekt abrahamshimekt Aug 27, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add [frombody] attribute to bind the postId from request body to postId paramater
whenever you use post or put ,please use frombody since the data is stored in the request body.

}

[HttpPost("follow/{UserId}")]
public async Task<IActionResult> Follow(int UserId)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add frombody

Copy link

@abrahamshimekt abrahamshimekt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resolve the issues

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants