From 6999a3ac8e7749a7c5d510b728c1421577086501 Mon Sep 17 00:00:00 2001 From: voommen-livefront Date: Mon, 23 Jun 2025 09:39:05 -0500 Subject: [PATCH] PM-20576 added logs and updated errors as per PR comments --- .../AddOrganizationReportCommand.cs | 27 +++++++++++++------ .../DropOrganizationReportCommand.cs | 15 +++++++++-- .../GetOrganizationReportQuery.cs | 8 +++++- .../DeleteOrganizationReportCommandTests.cs | 12 ++++----- 4 files changed, 45 insertions(+), 17 deletions(-) diff --git a/src/Core/Dirt/Reports/ReportFeatures/AddOrganizationReportCommand.cs b/src/Core/Dirt/Reports/ReportFeatures/AddOrganizationReportCommand.cs index 00a56b6bc5..132e3bc819 100644 --- a/src/Core/Dirt/Reports/ReportFeatures/AddOrganizationReportCommand.cs +++ b/src/Core/Dirt/Reports/ReportFeatures/AddOrganizationReportCommand.cs @@ -4,6 +4,7 @@ using Bit.Core.Dirt.Reports.ReportFeatures.Requests; using Bit.Core.Dirt.Repositories; using Bit.Core.Exceptions; using Bit.Core.Repositories; +using Microsoft.Extensions.Logging; namespace Bit.Core.Dirt.Reports.ReportFeatures; @@ -11,20 +12,26 @@ public class AddOrganizationReportCommand : IAddOrganizationReportCommand { private readonly IOrganizationRepository _organizationRepo; private readonly IOrganizationReportRepository _organizationReportRepo; + private ILogger _logger; public AddOrganizationReportCommand( IOrganizationRepository organizationRepository, - IOrganizationReportRepository organizationReportRepository) + IOrganizationReportRepository organizationReportRepository, + ILogger logger) { _organizationRepo = organizationRepository; _organizationReportRepo = organizationReportRepository; + _logger = logger; } public async Task AddOrganizationReportAsync(AddOrganizationReportRequest request) { - var (req, IsValid, errorMessage) = await ValidateRequestAsync(request); - if (!IsValid) + _logger.LogInformation("Adding organization report for organization {organizationId}", request.OrganizationId); + + var (isValid, errorMessage) = await ValidateRequestAsync(request); + if (!isValid) { + _logger.LogInformation("Failed to add organization {organizationId} report: {errorMessage}", request.OrganizationId, errorMessage); throw new BadRequestException(errorMessage); } @@ -40,25 +47,29 @@ public class AddOrganizationReportCommand : IAddOrganizationReportCommand organizationReport.SetNewId(); var data = await _organizationReportRepo.CreateAsync(organizationReport); + + _logger.LogInformation("Successfully added organization report for organization {organizationId}, {organizationReportId}", + request.OrganizationId, data.Id); + return data; } - private async Task> ValidateRequestAsync( + private async Task<(bool IsValid, string errorMessage)> ValidateRequestAsync( AddOrganizationReportRequest request) { // verify that the organization exists var organization = await _organizationRepo.GetByIdAsync(request.OrganizationId); if (organization == null) { - return new Tuple(request, false, "Invalid Organization"); + return (false, "Invalid Organization"); } - // ensure that we have a URL + // ensure that we have report data if (string.IsNullOrWhiteSpace(request.ReportData)) { - return new Tuple(request, false, "Report Data is required"); + return (false, "Report Data is required"); } - return new Tuple(request, true, string.Empty); + return (true, string.Empty); } } diff --git a/src/Core/Dirt/Reports/ReportFeatures/DropOrganizationReportCommand.cs b/src/Core/Dirt/Reports/ReportFeatures/DropOrganizationReportCommand.cs index b629c365f0..e382788273 100644 --- a/src/Core/Dirt/Reports/ReportFeatures/DropOrganizationReportCommand.cs +++ b/src/Core/Dirt/Reports/ReportFeatures/DropOrganizationReportCommand.cs @@ -2,29 +2,40 @@ using Bit.Core.Dirt.Reports.ReportFeatures.Requests; using Bit.Core.Dirt.Repositories; using Bit.Core.Exceptions; +using Microsoft.Extensions.Logging; namespace Bit.Core.Dirt.Reports.ReportFeatures; public class DropOrganizationReportCommand : IDropOrganizationReportCommand { private IOrganizationReportRepository _organizationReportRepo; + private ILogger _logger; public DropOrganizationReportCommand( - IOrganizationReportRepository organizationReportRepository) + IOrganizationReportRepository organizationReportRepository, + ILogger logger) { _organizationReportRepo = organizationReportRepository; + _logger = logger; } public async Task DropOrganizationReportAsync(DropOrganizationReportRequest request) { + _logger.LogInformation("Dropping organization report for organization {organizationId}", + request.OrganizationId); + var data = await _organizationReportRepo.GetByOrganizationIdAsync(request.OrganizationId); if (data == null || data.Count() == 0) { - throw new BadRequestException("Organization does not have any records."); + _logger.LogInformation("No organization reports found for organization {organizationId}", request.OrganizationId); + throw new BadRequestException("No data found."); } data.Where(_ => request.OrganizationReportIds.Contains(_.Id)).ToList().ForEach(async _ => { + _logger.LogInformation("Dropping organization report {organizationReportId} for organization {organizationId}", + _.Id, request.OrganizationId); + await _organizationReportRepo.DeleteAsync(_); }); } diff --git a/src/Core/Dirt/Reports/ReportFeatures/GetOrganizationReportQuery.cs b/src/Core/Dirt/Reports/ReportFeatures/GetOrganizationReportQuery.cs index cd5393f46b..e536fdfddc 100644 --- a/src/Core/Dirt/Reports/ReportFeatures/GetOrganizationReportQuery.cs +++ b/src/Core/Dirt/Reports/ReportFeatures/GetOrganizationReportQuery.cs @@ -2,17 +2,21 @@ using Bit.Core.Dirt.Reports.ReportFeatures.Interfaces; using Bit.Core.Dirt.Repositories; using Bit.Core.Exceptions; +using Microsoft.Extensions.Logging; namespace Bit.Core.Dirt.Reports.ReportFeatures; public class GetOrganizationReportQuery : IGetOrganizationReportQuery { private IOrganizationReportRepository _organizationReportRepo; + private ILogger _logger; public GetOrganizationReportQuery( - IOrganizationReportRepository organizationReportRepo) + IOrganizationReportRepository organizationReportRepo, + ILogger logger) { _organizationReportRepo = organizationReportRepo; + _logger = logger; } public async Task> GetOrganizationReportAsync(Guid organizationId) @@ -22,6 +26,7 @@ public class GetOrganizationReportQuery : IGetOrganizationReportQuery throw new BadRequestException("OrganizationId is required."); } + _logger.LogInformation("Fetching organization reports for organization {organizationId}", organizationId); return await _organizationReportRepo.GetByOrganizationIdAsync(organizationId); } @@ -32,6 +37,7 @@ public class GetOrganizationReportQuery : IGetOrganizationReportQuery throw new BadRequestException("OrganizationId is required."); } + _logger.LogInformation("Fetching latest organization report for organization {organizationId}", organizationId); return await _organizationReportRepo.GetLatestByOrganizationIdAsync(organizationId); } } diff --git a/test/Core.Test/Dirt/ReportFeatures/DeleteOrganizationReportCommandTests.cs b/test/Core.Test/Dirt/ReportFeatures/DeleteOrganizationReportCommandTests.cs index 78311f4287..f6a5c13be9 100644 --- a/test/Core.Test/Dirt/ReportFeatures/DeleteOrganizationReportCommandTests.cs +++ b/test/Core.Test/Dirt/ReportFeatures/DeleteOrganizationReportCommandTests.cs @@ -115,7 +115,7 @@ public class DeleteOrganizationReportCommandTests // Act & Assert var exception = await Assert.ThrowsAsync(async () => await sutProvider.Sut.DropOrganizationReportAsync(request)); - Assert.Equal("Organization does not have any records.", exception.Message); + Assert.Equal("No data found.", exception.Message); } [Theory, BitAutoData] @@ -131,7 +131,7 @@ public class DeleteOrganizationReportCommandTests // Act & Assert var exception = await Assert.ThrowsAsync(async () => await sutProvider.Sut.DropOrganizationReportAsync(request)); - Assert.Equal("Organization does not have any records.", exception.Message); + Assert.Equal("No data found.", exception.Message); } [Theory, BitAutoData] @@ -146,7 +146,7 @@ public class DeleteOrganizationReportCommandTests // Act & Assert var exception = await Assert.ThrowsAsync(async () => await sutProvider.Sut.DropOrganizationReportAsync(request)); - Assert.Equal("Organization does not have any records.", exception.Message); + Assert.Equal("No data found.", exception.Message); } [Theory, BitAutoData] @@ -161,7 +161,7 @@ public class DeleteOrganizationReportCommandTests // Act & Assert var exception = await Assert.ThrowsAsync(async () => await sutProvider.Sut.DropOrganizationReportAsync(request)); - Assert.Equal("Organization does not have any records.", exception.Message); + Assert.Equal("No data found.", exception.Message); } [Theory, BitAutoData] @@ -176,7 +176,7 @@ public class DeleteOrganizationReportCommandTests // Act & Assert var exception = await Assert.ThrowsAsync(async () => await sutProvider.Sut.DropOrganizationReportAsync(request)); - Assert.Equal("Organization does not have any records.", exception.Message); + Assert.Equal("No data found.", exception.Message); } [Theory, BitAutoData] @@ -188,7 +188,7 @@ public class DeleteOrganizationReportCommandTests // Act & Assert var exception = await Assert.ThrowsAsync(async () => await sutProvider.Sut.DropOrganizationReportAsync(request)); - Assert.Equal("Organization does not have any records.", exception.Message); + Assert.Equal("No data found.", exception.Message); } }