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

Unused "AccountType" search parameter in the "Get All Accounts" endpoint. #88

Open
Duslerke opened this issue May 18, 2022 · 0 comments
Labels
bug Something isn't working

Comments

@Duslerke
Copy link
Contributor

Problem:

  • The Get All (Accounts) endpoint's Gateway does not make use of provided Account Type parameter when filtering entities (see bellow), ...
    public async Task<List<Account>> GetAllAsync(Guid targetId, AccountType accountType)
    {
    if (targetId == null)
    throw new ArgumentException("Invalid targetId");
    IQueryable<AccountDbEntity> data = _accountDbContext
    .AccountEntities
    .Where(p => p.TargetId == targetId);
    return await data.Select(p => p.ToDomain())
    .ToListAsync()
    .ConfigureAwait(false);
    }

    ... while it is specified as available, active parameter on the endpoint controller's xml comment documentation (see Line 82):
    /// <summary>
    /// Get a list of Account models
    /// </summary>
    /// <param name="targetId">The target id by which we are looking for account</param>
    /// <param name="accountType">The account type by which we are looking for accounts</param>
    /// <response code="200">Success. Account models was received successfully</response>
    /// <response code="400">Bad Request</response>
    /// <response code="404">Accounts with provided id cannot be found</response>
    /// <response code="500">Internal Server Error</response>
    [ProducesResponseType(typeof(AccountResponses), StatusCodes.Status200OK)]
    [ProducesResponseType(typeof(BaseErrorResponse), StatusCodes.Status404NotFound)]
    [ProducesResponseType(typeof(BaseErrorResponse), StatusCodes.Status400BadRequest)]
    [ProducesResponseType(typeof(BaseErrorResponse), StatusCodes.Status500InternalServerError)]
    [HttpGet]
    public async Task<IActionResult> GetAllAsync([FromQuery] Guid targetId, [FromQuery] AccountType accountType)

Potential Solution:

  • If missing account type filter is an oversight, then when assuming it is optional it could be fixed by adding a conditional addon onto the already existing queryable object (see bellow comment):
 public async Task<List<Account>> GetAllAsync(Guid targetId, AccountType accountType) 
 { 
     if (targetId == null) 
         throw new ArgumentException("Invalid targetId"); 
  
     var data = _accountDbContext 
         .AccountEntities 
         .Where(p => p.TargetId == targetId);
         
     // something like:
     if (accountType != null)
         data = data.Where(a => a.AccountType == accountType);

     return await data.Select(p => p.ToDomain()) 
         .ToListAsync() 
         .ConfigureAwait(false); 
 } 
  • However, if the "Account Type" search parameter is not needed, then it needs to be removed from the Gateway signature, Use Case signature & their interfaces, as well as from Controller parameters and its XML documentation. Depending on how well this API is tested, the tests might need to change as well.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant