Feat/Payment process task and some process engine refactoring #448

bjorntore · 2024-02-19T07:59:05Z

Description

Adding payment process task. Refactored the process engine code somewhat.

Related Issue(s)

Brukerbetaling av en Altinn 3 tjeneste #399

Verification

Your code builds clean without any errors or warnings
Manual testing done (required)
Relevant automated test added (if you find this hard, leave it and we'll help out)
All tests run green

Documentation

User documentation is updated with a separate linked PR in altinn-studio-docs. (if applicable)

…ource

…ent related code.

…wards

…per early edition.

…ct logic. Update unit tests.

…roller. Should never have been there.

…ayment status.

bjorntore · 2024-02-20T12:41:09Z

src/Altinn.App.Core/Internal/Process/ProcessTasks/Common/ProcessTaskInitializer.cs

+    /// <inheritdoc/>
+    public async Task Initialize(string taskId, Instance instance, Dictionary<string, string> prefill)
+    {
+        _logger.LogDebug("OnStartProcessTask for {InstanceId}", instance.Id);


Thoughts? These debugs already existed. I guess debug logs only run on local dev?

We have lots of these issues. The problem is that if you log to standard out with a newline separating log entries, this pattern is problematic, because the user might provide two a new forged log entry between two newlines. That trick won't fool application insights, so I think we can safely ignore these.

The other issue is that C# might have a similar issue as log4j (log4shell), but after java was caught, lots of people checked dotnet for similar issues, and none was reported.

test/Altinn.App.Core.Tests/Internal/Process/ProcessTasks/PaymentProcessTaskTests.cs

...Altinn.App.Core.Tests/Internal/Process/EventHandlers/ProcessTask/EndTaskEventHandlerTests.cs

+{
+    private Mock<IProcessTaskDataLocker> _processTaskDataLocker;
+    private Mock<IProcessTaskFinalizer> _processTaskFinisher;
+    private Mock<IServiceTask> _pdfServiceTask;


...Altinn.App.Core.Tests/Internal/Process/EventHandlers/ProcessTask/EndTaskEventHandlerTests.cs

+    private Mock<IProcessTaskDataLocker> _processTaskDataLocker;
+    private Mock<IProcessTaskFinalizer> _processTaskFinisher;
+    private Mock<IServiceTask> _pdfServiceTask;
+    private Mock<IServiceTask> _eformidlingServiceTask;


src/Altinn.App.Core/Internal/Process/ProcessEventHandlingDelegator.cs

+            foreach (var processTaskType in _processTasks)
+            {
+                if (processTaskType.Type == altinnTaskType)
+                {
+                    return processTaskType;
+                }
+            }


src/Altinn.App.Core/Features/Payment/Providers/Nets/NetsPaymentProcessor.cs

test/Altinn.App.Core.Tests/Features/Payment/PaymentServiceTests.cs

# Conflicts: # test/Altinn.App.Api.Tests/Mocks/DataClientMock.cs

…CurrentTask is null. StartEvent, EndEvent etc.

bjorntore · 2024-02-20T12:29:21Z

src/Altinn.App.Core/Features/Payment/Providers/Nets/NetsPaymentProcessor.cs

+    }
+
+    /// <inheritdoc />
+    public async Task<PaymentStatus?> GetPaymentStatus(Instance instance, string paymentReference, decimal expectedTotalIncVat)


Any input on what kinds of verifications we should do to check if the payment is in good health? Right now I'm just doing a simple sum comparison. Charged amount == our local calculation of total inc VAT.

src/Altinn.App.Core/Internal/Data/DataService.cs

src/Altinn.App.Core/Internal/Process/EventHandlers/EndEventEventHandler.cs

src/Altinn.App.Core/Internal/Process/EventHandlers/ProcessTask/AbandonTaskEventHandler.cs

src/Altinn.App.Core/Internal/Process/EventHandlers/ProcessTask/EndTaskEventHandler.cs

src/Altinn.App.Core/Internal/Process/EventHandlers/ProcessTask/StartTaskEventHandler.cs

src/Altinn.App.Core/Internal/Process/ProcessEventDispatcher.cs

src/Altinn.App.Core/Internal/Process/ProcessTasks/Common/ProcessTaskDataLocker.cs

src/Altinn.App.Core/Internal/Process/ProcessTasks/Common/ProcessTaskFinalizer.cs

src/Altinn.App.Core/Internal/Process/ProcessTasks/Common/ProcessTaskInitializer.cs

src/Altinn.App.Core/Internal/Process/ServiceTasks/PdfServiceTask.cs

src/Altinn.App.Core/Internal/Process/ServiceTasks/EformidlingServiceTask.cs

bjorntore · 2024-02-20T12:55:29Z

test/Altinn.App.Core.Tests/Implementation/DefaultTaskEventsTests.cs

@@ -20,7 +19,7 @@

 namespace Altinn.App.PlatformServices.Tests.Implementation;

-public class DefaultTaskEventsTests: IDisposable
+public class DefaultTaskEventsTests : IDisposable


TODO: Remove file. Consider if more tests should be kept.

…ementAndModel_WhenDataElementExists

…payment status check. Remove setting for integration type.

…ction extension, so it's fully opt in.

sonarqubecloud · 2024-02-21T14:44:43Z

Quality Gate failed

Failed conditions
63.1% Coverage on New Code (required ≥ 65%)
58.42% Condition Coverage on New Code (required ≥ 65%)

See analysis details on SonarCloud

ivarne · 2024-03-01T06:28:24Z

src/Altinn.App.Core/Internal/Process/ProcessTasks/Common/Interfaces/IProcessTaskFinalizer.cs

+        /// <summary>
+        /// Removes data elements generated from a task, if the data elements are tagged with the task ID.
+        /// </summary>
+        /// <param name="instance"></param>
+        /// <param name="taskId"></param>
+        /// <returns></returns>
+        Task RemoveDataElementsGeneratedFromTask(Instance instance, string taskId);
+
+        /// <summary>
+        /// Remove hidden data from the instance.
+        /// </summary>
+        /// <param name="instance"></param>
+        /// <param name="instanceGuid"></param>
+        /// <param name="connectedDataTypes"></param>
+        /// <returns></returns>
+        Task RemoveHiddenData(Instance instance, Guid instanceGuid, List<DataType>? connectedDataTypes);
+
+        /// <summary>
+        /// Remove shadow fields from the instance.
+        /// </summary>
+        /// <param name="instance"></param>
+        /// <param name="instanceGuid"></param>
+        /// <param name="connectedDataTypes"></param>
+        /// <returns></returns>
+        Task RemoveShadowFields(Instance instance, Guid instanceGuid, List<DataType> connectedDataTypes);


Hvorfor ligger disse metodene i interfacet? De kalles da bare fra Finalize?

Det tror jeg var en glipp. Takk!

ivarne · 2024-03-01T06:29:01Z

src/Altinn.App.Core/Internal/Process/ProcessTasks/Common/Interfaces/IProcessTaskInitializer.cs

+        /// <summary>
+        /// Updates presentation texts on the instance for a given data type and data.
+        /// </summary>
+        /// <param name="instance"></param>
+        /// <param name="dataType"></param>
+        /// <param name="data"></param>
+        /// <returns></returns>
+        Task UpdatePresentationTextsOnInstance(Instance instance, string dataType, dynamic data);
+
+        /// <summary>
+        /// Updates data values on the instance for a given data type and data.
+        /// </summary>
+        /// <param name="instance"></param>
+        /// <param name="dataType"></param>
+        /// <param name="data"></param>
+        /// <returns></returns>
+        Task UpdateDataValuesOnInstance(Instance instance, string dataType, object data);


Samme her. De har vel ikke noe i interfacet å gjøre?

ivarne · 2024-03-01T06:30:02Z

src/Altinn.App.Core/Internal/Process/ProcessTasks/Common/ProcessTaskFinalizer.cs

+        IAppModel appModel,
+        IAppResources appResources,
+        LayoutEvaluatorStateInitializer layoutEvaluatorStateInitializer,
+        IOptions<AppSettings>? appSettings = null)


Hvorfor er AppSettings nullable?

bjorntore · 2024-03-01T10:22:38Z

Creating new PR.

tjololo and others added 16 commits February 12, 2024 08:50

First round of refactoring to process logic

e6e295b

Add makefile for building app-libs locally and creating local nuget s…

c637290

…ource

Split process event handling and dispatching. Restructure process ev…

a3c060b

…ent related code.

Removed dryrun and explisitly call HandleEventsAndUpdateStorage after…

86234e4

…wards

Make the code easier to test and add some tests

623c9a2

Minor rename and code docs adjustments.

9e76f29

Draft for payments

5a9e67c

Some minor adjustments to payment code and added PaymentUserAction su…

8308dae

…per early edition.

Payment related changes.

a0986c5

Correct instanceId for dataclient

1cdda07

Move more payment logic to PaymentService. Adjust callback and redire…

8921ef0

…ct logic. Update unit tests.

Fix PdfServiceTask and related unit tests.

caec565

Remove redirect response from webook callback handler in payment cont…

353bc69

…roller. Should never have been there.

Rewrite payment to depend on frontend to make an http call to check p…

ec766e8

…ayment status.

Some payment related refactoring.

c0d6d04

Remove empty folder.

c5f1ea3

bjorntore requested review from ivarne, tjololo and RonnyB71 February 19, 2024 07:59

github-advanced-security bot found potential problems Feb 19, 2024

View reviewed changes

bjorntore added 4 commits February 19, 2024 09:28

CodeQL stuff.

1262dd6

Tests process task.

17ca3ca

NetsPaymentProcessor tests.

bb2cee5

HttpApiResult tests.

d7f8f18

bjorntore linked an issue Feb 20, 2024 that may be closed by this pull request

Brukerbetaling av en Altinn 3 tjeneste #399

Closed

6 tasks

bjorntore added 5 commits February 20, 2024 12:19

DataService tests.

a56556c

Merge branch 'v8' into feat/payment-process-task

b37e603

# Conflicts: # test/Altinn.App.Api.Tests/Mocks/DataClientMock.cs

Fix data service tests after NotFoundException was removed.

bcb0558

Implement DataClientMock.UnlockDataElement.

09b4857

Fix bug in ProcessEventHandlingDelegator. Allow taskId to be null if …

ea51625

…CurrentTask is null. StartEvent, EndEvent etc.