fix(webapi): Only allow transmissionId on TransmissionOpened activities #1631
Conversation
📝 WalkthroughWalkthroughThe pull request introduces modifications to the validation logic for dialog activities, specifically focusing on the Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
tests/Digdir.Domain.Dialogporten.Application.Unit.Tests/Features/V1/ServiceOwner/Activities/ActivityValidatorTests.cs (2)
20-22: Consider enhancing test data generation.The current implementation tests all activity types, but consider adding specific test cases for edge cases such as:
- Null TransmissionId
- Invalid TransmissionId format
- Empty TransmissionId
55-59: Strengthen error message assertions.The error message assertions could be more specific by checking the exact error message content instead of just checking if it contains "TransmissionOpened".
-createValidation.Errors.First().ErrorMessage.Should().Contain("TransmissionOpened"); -updateValidation.Errors.First().ErrorMessage.Should().Contain("TransmissionOpened"); +createValidation.Errors.First().ErrorMessage.Should().Be("Only activities of type TransmissionOpened can reference a transmission."); +updateValidation.Errors.First().ErrorMessage.Should().Be("Only activities of type TransmissionOpened can reference a transmission.");
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Dialogs/Commands/Create/CreateDialogCommandValidator.cs(1 hunks)src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Dialogs/Commands/Update/UpdateDialogCommandValidator.cs(1 hunks)tests/Digdir.Domain.Dialogporten.Application.Unit.Tests/Features/V1/ServiceOwner/Activities/ActivityValidatorTests.cs(1 hunks)
🔇 Additional comments (2)
src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Dialogs/Commands/Update/UpdateDialogCommandValidator.cs (1)
403-404: LGTM! Validation rules correctly implement the requirement.
The validation rules properly enforce that:
- Only TransmissionOpened activities can have a TransmissionId
- TransmissionOpened activities must have a TransmissionId
Also applies to: 407-408
src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Dialogs/Commands/Create/CreateDialogCommandValidator.cs (1)
437-438: Verify consistent validation behavior.
The validation rules match those in UpdateDialogCommandValidator. Let's verify the behavior is consistent across both validators.
Also applies to: 441-442
✅ Verification successful
Validation rules for TransmissionId are consistent across both validators
After reviewing both validator implementations, I can confirm that the validation rules for TransmissionId in activities are identical in both CreateDialogCommandValidator and UpdateDialogCommandValidator. Both validators enforce:
- TransmissionId must be null when activity type is not TransmissionOpened
- TransmissionId must not be empty when activity type is TransmissionOpened
- The error messages are exactly the same in both validators
The validation behavior is consistent and correctly implemented in both cases.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that both validators implement identical validation rules
# Compare the validation rules in both validators
diff <(rg -A 3 "TransmissionId.*Null" src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Dialogs/Commands/Create/CreateDialogCommandValidator.cs) <(rg -A 3 "TransmissionId.*Null" src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Dialogs/Commands/Update/UpdateDialogCommandValidator.cs)
# Compare the error messages in both validators
diff <(rg -A 3 "WithMessage.*TransmissionOpened" src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Dialogs/Commands/Create/CreateDialogCommandValidator.cs) <(rg -A 3 "WithMessage.*TransmissionOpened" src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Dialogs/Commands/Update/UpdateDialogCommandValidator.cs)
Length of output: 887
Script:
#!/bin/bash
# Let's try a different approach to compare the validation rules
# First, let's verify both files exist and get their content
echo "=== CreateDialogCommandValidator.cs ==="
rg -l "CreateDialogCommandValidator" src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Dialogs/Commands/Create/
cat src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Dialogs/Commands/Create/CreateDialogCommandValidator.cs
echo -e "\n=== UpdateDialogCommandValidator.cs ==="
rg -l "UpdateDialogCommandValidator" src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Dialogs/Commands/Update/
cat src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Dialogs/Commands/Update/UpdateDialogCommandValidator.cs
Length of output: 35966
oskogstad
deleted the
fix/only-allow-transmissionid-on-transmissionopened-activities
branch
🤖 I have created a release *beep* *boop* --- ## [1.42.1](v1.42.0...v1.42.1) (2024-12-25) ### Bug Fixes * **webapi:** Only allow transmissionId on TransmissionOpened activities ([#1631](#1631)) ([80261d1](80261d1)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Description
The validator activity validator allows setting TransmissionId on every activity type except DialogOpened.
It should only allow on TransmissionOpened.
Related Issue(s)
Verification
Documentation
docs-directory, Altinnpedia or a separate linked PR in altinn-studio-docs., if applicable)