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

Refactored SFA.DAS.Payments.ScheduledJobs Az Function #6

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

Conversation

pomtom
Copy link
Collaborator

@pomtom pomtom commented Jan 7, 2025

#4
This Pull Request includes updates related to the .NET 6 upgrade and the removal of legacy components such as Autofac and NServiceBus configuration. It also incorporates the use of output bindings for Azure Service Bus queues. The primary goal is to streamline future upgrade processes and leverage the Microsoft ecosystem libraries.

By isolating Azure Functions, services, models, DbContexts, DTOs, configurations, enums, validators, and dependency injections into separate folders, the project structure is now more organized and easier to understand. This reorganization aims to facilitate smoother transitions to future .NET versions (e.g., .NET 8, .NET 9) with minimal changes required in the repository. Given Microsoft's evolving practices and patterns, this approach minimizes the challenges associated with third-party libraries.

David-B-Read and others added 30 commits June 6, 2024 17:54
…y for apprenticeship data processing and validation, and the configuration of new database contexts and services. These changes enhance the codebase by improving readability, adding new features, and ensuring proper configuration and dependency injection for the new functionalities.
Updated using directives in AuditDataCleanUpService.cs to include Newtonsoft.Json. Refactored SplitBatchAndEnqueueMessages method to serialize messages using JsonConvert.SerializeObject before sending to the queue. This ensures consistent message format and improves code maintainability.
Introduced a try-catch block around the deserialization of
`message.Body` and the call to `_auditDataCleanUpService.
DataLockEventAuditDataCleanUp(batch)`. This change ensures
that any exceptions during these operations are caught and
logged with an error message, including the exception details
and the `MessageId` of the message being processed. This
improves the robustness of the `Run` method by preventing
silent failures or crashes, aiding in better issue diagnosis
and application stability.
Pramod Lawate added 4 commits January 9, 2025 11:11
Renamed namespace `SFA.DAS.Payments.ScheduledJobs.DTOS` to `SFA.DAS.Payments.ScheduledJobs.Dtos` across multiple files. Removed unused using directives from various files,
Pramod Lawate and others added 10 commits January 10, 2025 12:08
The `ServiceBusConnectionString` property has been removed from the `AppsettingsOptions` class. Correspondingly, the retrieval of the `ServiceBusConnectionString` configuration value has been removed from the `DependencyInjection` class within the `SFA.DAS.Payments.ScheduledJobs.Ioc` namespace. This indicates that the application no longer requires the `ServiceBusConnectionString` configuration setting.
Corrected class name from AppsettingsOptions to AppSettingsOptions
in SFA.DAS.Payments.ScheduledJobs.Configuration and
SFA.DAS.Payments.ScheduledJobs.Ioc namespaces. Added
ServiceBusConnectionString property to AppSettingsOptions class
for storing the Service Bus connection string.
@@ -0,0 +1,54 @@
namespace SFA.DAS.Payments.ScheduledJobs.Configuration
{
public class AppSettingsOptions : IAppSettingsOptions

Choose a reason for hiding this comment

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

Can the capitalization on the filename for this and the IAppsettingsOptions file be modified to match the class/interface names please?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I have renamed the file name.

public string CurrentCollectionPeriod { get; set; }
public string CurrentAcademicYear { get; set; }
public string PreviousAcademicYearCollectionPeriod { get; set; }
public string PerviousAcademicYear { get; set; }

Choose a reason for hiding this comment

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

Typo in property name

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Renamed the variable

public string PreviousAcademicYear { get; set; }
}

public class Connectionstrings

Choose a reason for hiding this comment

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

Can this be capitalized to ConnectionStrings please?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I have change the variable names

LevyAccountSchedule = configHelper.GetValue<string>("LevyAccountSchedule"),
LevyAccountValidationSchedule = configHelper.GetValue<string>("LevyAccountValidationSchedule"),
LogLevel = configHelper.GetValue<string>("LogLevel"),
PerviousAcademicYear = configHelper.GetValue<string>("PerviousAcademicYear"),

Choose a reason for hiding this comment

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

Typo in property / config variable

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I have fixed the name

{
services.AddScoped<IAuditDataCleanUpService, AuditDataCleanUpService>();
services.AddScoped<IPaymentLogger, PaymentLogger>();
//services.AddScoped<IEndpointInstanceFactory, EndpointInstanceFactory>();

Choose a reason for hiding this comment

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

Can this be removed now?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I have removed

Corrected typos in `AppSettingsOptions` and `IAppSettingsOptions`:
- Renamed `Connectionstrings` to `ConnectionStrings`
- Renamed `PerviousAcademicYear` to `PreviousAcademicYear`

Updated `DependencyInjection.cs`:
- Fixed instantiation to use `ConnectionStrings`
- Corrected configuration key `PerviousAcademicYear`
- Removed unused commented-out line
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.

3 participants