Feature/correspondence error handling

[erlendoksvoll]

Description

Documentation

• Doc updated

Summary by CodeRabbit

• Refactor
  • Optimized internal certificate handling and debug logging configuration to improve code maintainability.

[coderabbitai]

Walkthrough

AddAltinn3Messaging method refactored to inline-encode Settings.AltinnCertificate as Base64-encoded PKCS#12 format for MaskinportenSettings.EncodedX509 instead of using a precomputed certificate value. EnableDebugLogging simplified to a direct boolean expression. A redundant local variable removed.

Changes

Cohort / File(s)	Summary
Certificate Encoding Refactor Dan.Core/Program.cs	Modified AddAltinn3Messaging to derive MaskinportenSettings.EncodedX509 by exporting Settings.AltinnCertificate to PKCS#12 and Base64-encoding inline. Simplified EnableDebugLogging boolean logic. Removed redundant local variable.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• changed cert loading to support oed-messaging #279: Makes certificates exportable (X509KeyStorageFlags.Exportable) to support exporting Settings.AltinnCertificate as PKCS#12, directly enabling the changes in this PR.

Suggested reviewers

• SondreJDigdir

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Title check	⚠️ Warning	The pull request title 'Feature/correspondence error handling' does not match the actual changes shown in the summary, which involve certificate loading modifications and PKCS#12 handling in Maskinporten configuration.	Update the PR title to accurately reflect the actual changes, such as 'Update certificate loading to use PKCS#12 export and simplify debug logging configuration' or similar.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feature/correspondence-error-handling

⚔️ Resolve merge conflicts

• Resolve merge conflict in branch feature/correspondence-error-handling

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

Dan.Core/Program.cs (1)

261-274: Consider extracting the repeated environment check.

Settings.MaskinportenUrl.Contains("test") is evaluated three times (lines 268, 269, 273). While not a correctness issue, extracting it to a local variable would improve readability and ensure consistency if the logic ever changes.

♻️ Suggested refactor

 void AddAltinn3Messaging(IServiceCollection services)
 {    
+    var isTestEnvironment = Settings.MaskinportenUrl.Contains("test");
     services.AddDdCorrespondenceService(options =>
     {
         options.MaskinportenSettings = new MaskinportenSettings
         {
             ClientId = Settings.MaskinportenClientId,
-            Environment = Settings.MaskinportenUrl.Contains("test") ? "test" : "prod",
-            EnableDebugLogging = Settings.MaskinportenUrl.Contains("test"),
+            Environment = isTestEnvironment ? "test" : "prod",
+            EnableDebugLogging = isTestEnvironment,
             EncodedX509 = Convert.ToBase64String(Settings.AltinnCertificate.Export(X509ContentType.Pkcs12))
         };
         options.ResourceId = "digdir-data-altinn-no-melding";        
-        options.Environment = Settings.MaskinportenUrl.Contains("test") ? ApiEnvironment.Staging : ApiEnvironment.Production;       
+        options.Environment = isTestEnvironment ? ApiEnvironment.Staging : ApiEnvironment.Production;       
     });
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@Dan.Core/Program.cs` around lines 261 - 274, Extract the repeated
Settings.MaskinportenUrl.Contains("test") boolean check into a local variable at
the start of AddAltinn3Messaging (e.g., bool isTest =
Settings.MaskinportenUrl.Contains("test")) and use that variable for Environment
selection, EnableDebugLogging, and options.Environment assignments so the logic
is evaluated once and the code is clearer and consistent across the
MaskinportenSettings and options assignments.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@Dan.Core/Program.cs`:
- Around line 261-274: Extract the repeated
Settings.MaskinportenUrl.Contains("test") boolean check into a local variable at
the start of AddAltinn3Messaging (e.g., bool isTest =
Settings.MaskinportenUrl.Contains("test")) and use that variable for Environment
selection, EnableDebugLogging, and options.Environment assignments so the logic
is evaluated once and the code is clearer and consistent across the
MaskinportenSettings and options assignments.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: bbd97ac0-581e-436d-a8dc-b8870d32c28c

📥 Commits

Reviewing files that changed from the base of the PR and between 625d0bf and cf26eca.

📒 Files selected for processing (1)

• Dan.Core/Program.cs