Problem:

• The Get All (Accounts) endpoint's Gateway does not make use of provided Account Type parameter when filtering entities (see bellow), ...
  

  account-api/AccountsApi/V1/Gateways/AccountApiGateway.cs

  Lines 38 to 49 in 069bfa1

  	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):
  

  account-api/AccountsApi/V1/Controllers/AccountApiController.cs

  Lines 78 to 92 in 069bfa1

  	/// <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.

Problem:

Option 1: The name of the endpoint is "GetAll" and it is accessed via \accounts url. Additionally, it's parameters are being provided via the Query String (rather than via Route) and are not validated for being Required (nor are described in the XML description as required).

All these facts together imply that the endpoint was designed with the thought of Getting All Accounts by default, with the optional parameters of targetId (tenureId) and the accountType for filtering purposes should it be needed.

If that is the case, the problem is that When a user of the API does not provide the 'targetId', they will get NO Results, meanwhile they should be expected to get ALL existing accounts. This is because the filtering condition in the endpoint's Gateway will look for all accounts with targetId == Guid.Empty (see Line 45):

Option 2: The intent of the endpoint is: to retrieve ALL accounts WITH specific 'targetId', which would mean that the following are wrong:

• Name of the endpoint (should be "GetAccountsByTargetId")
• Url of the endpoint (it doesn't follow Hackney's API url standards for REST resources: see here for how it should be implemented)
• Parameters retrieved from Query rather than from Route (relates to url)
• Endpoint's XML documentation (should explicitly say which parameters are required)
• There being no Required validation on the parameters (it most likely does not make much sense for the user to search for accounts based on "Guid.Empty" targetId, so required param validation might be useful. If it turns out that it does make sense to do such search, then it should be mentioned under the parameter's XML documentation that 'when targetId value is not provided, the endpoint retrieves all accounts with no targetId under them.')

Potential Fixes:

• If it's Option 1, then it means that we have a bug preventing a user from retrieving ALL Accounts with no filtering. The fix in this case would be similar to that of mentioned in the previous issue relating to unused "Account Type" (See: #88 ).
• If it's Option 2, then it means that at the bare minimum, the XML documentation needs to be changed & Required parameter validation needs to be added. Of course, a full fix of fixing the bullet list items above would break the V1 contract, so going into the future, the V2 version of this endpoint should be created to address these problems.