[ADMINAPI-1060] - Admin Console - Instances endpoint (/adminconsole/Instances) #178
Conversation
4bda675 to
ba26f08
Compare
ba26f08 to
392cdce
Compare
| var adminConsoleIsEnabled = builder.Configuration.GetValue<bool>("AppSettings:EnableAdminConsoleAPI"); | ||
| if (adminConsoleIsEnabled) | ||
| { | ||
| ServiceRegistration.AddServices(builder.Services, builder.Configuration); | ||
| } | ||
|
|
||
| builder.Services.AddAutoMapper(typeof(AdminConsoleMappingProfile)); |
| @@ -23,6 +26,13 @@ | |||
| builder.Services.AddSingleton<IRateLimitConfiguration, RateLimitConfiguration>(); | |||
| builder.Services.AddInMemoryRateLimiting(); | |||
|
|
|||
| builder.Services.Configure<AdminConsoleSettings>(builder.Configuration.GetSection("AdminConsoleSettings")); | |||
There was a problem hiding this comment.
the code from line 29 to line 35 has to be moved to our static class for service registration like in line 39:
ServiceRegistration.AddServices(builder.Services, builder.Configuration);
because this has to be enabled only when the "EnableAdminConsoleAPI" flag is in true.
Also, the "AdminConsoleSettings" section is duplicated because the appsettings has already have it but with different, you will find it like this (It's up to you if you want to rename it):
"AdminConsole": {
"CorsSettings": {
"EnableCors": false,
"AllowedOrigins": [
"https://localhost"
]
}
}
| @@ -25,6 +25,9 @@ | |||
| "EnableSwagger": false, | |||
| "DefaultTenant": "" | |||
| }, | |||
| "AdminConsoleSettings": { | |||
| "EncryptionKey": "abcdefghi" | |||
There was a problem hiding this comment.
Move to the section "AdminConsole"
| internal async Task<IResult> GetInstances([FromServices] IGetInstancesQuery getInstancesQuery) | ||
| { | ||
| var instances = await getInstancesQuery.Execute(); | ||
| return Results.Ok(instances); |
There was a problem hiding this comment.
I'm not sure if makes sense to return the entire payload, I would say and in terms to be consistent with adminconsole, we should return only the property "document" or a list of them, with this, it might help us to integrate it easily in the adminconsole webapp.
See the example
https://github.com/Ed-Fi-Alliance-OSS/AdminAPI-2.x/blob/main/docs/design/adminconsole/notes.md#endpoints
| int? DocId { get; set; } | ||
| int InstanceId { get; set; } | ||
| int TenantId { get; set; } | ||
| int EdOrgId { get; set; } |
| public int? DocId { get; set; } | ||
| public required int InstanceId { get; set; } | ||
| public required int TenantId { get; set; } | ||
| public required int EdOrgId { get; set; } |
| var instances = await getInstancesQuery.Execute(); | ||
| return Results.Ok(instances); | ||
| } | ||
| internal async Task<IResult> GetInstance([FromServices] IGetInstanceQuery getInstanceQuery, int docId) |
There was a problem hiding this comment.
The adminconsole sent the parameter tenantId, so I think is better to look for the tenantid instead of the docId
…nstances) (#178) * Add Db Contexts * Add correct injection of context * Add GenericRepository * Add tenantId * Fix program.cs * Fix github actions * Fix single tenant pg * Fix github actions * Admin Console - Instances endpoint * Fixes after rebase. Added db migration for pg. * Fixes after rabase * Some final cleanup * Fix on program.cs file * Fixes based on comments. --------- Co-authored-by: Danny Fernandez A <dfernandeza@growthaccelerationpartners.com>
No description provided.