feat(webapi): Add option to disable Altinn event generation #1633
Conversation
📝 WalkthroughWalkthroughThis pull request introduces a comprehensive set of changes to the Dialogporten API, focusing on enhancing event management, particularly for Altinn events. The modifications span multiple components of the application, including domain events, command structures, and API endpoints. A key addition is the ability to disable Altinn events for various dialog operations (create, update, delete, purge) with admin-level permissions. The changes also involve restructuring how dialog commands are handled, introducing a more explicit DTO (Data Transfer Object) pattern and adding metadata capabilities to domain events. Changes
Possibly related PRs
Suggested reviewers
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
|
oskogstad
force-pushed
the
feat/add-flag-for-disabling-event-generation
branch
from
5a8801d to
e0433fc
Compare
oskogstad
force-pushed
the
feat/add-flag-for-disabling-event-generation
branch
from
e0433fc to
80839b4
Compare
oskogstad
force-pushed
the
feat/add-flag-for-disabling-event-generation
branch
from
797e755 to
35b7d6f
Compare
oskogstad
force-pushed
the
feat/add-flag-for-disabling-event-generation
branch
from
35b7d6f to
e365cba
Compare
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🔭 Outside diff range comments (5)
tests/Digdir.Domain.Dialogporten.Application.Integration.Tests/Features/V1/ServiceOwner/Dialogs/Commands/PurgeDialogTests.cs (1)
Line range hint
13-13: Add test coverage for authorization requirements.Based on the PR changes, admin-level permissions are required for disabling Altinn events. Consider adding test cases to verify:
- Unauthorized users cannot disable events
- Admin users can successfully disable events
Would you like me to help generate these additional test cases?
tests/Digdir.Domain.Dialogporten.Application.Integration.Tests/Features/V1/ServiceOwner/Dialogs/Commands/UpdateDialogTests.cs (1)
Line range hint
1-135: Enhance test coverage for the new feature.The test class should include additional test cases to verify the new event generation control feature:
- Test disabling events during dialog update
- Test explicit enabling of events
- Test permission requirements for disabling events
- Test event generation side effects when disabled
This ensures the new feature is thoroughly tested and behaves as expected.
src/Digdir.Domain.Dialogporten.Domain/Common/DomainEvents/DomainEvent.cs (1)
Properties should remain mutable for event processing
The properties need to remain mutable as they are modified during event processing in the infrastructure layer:
OccuredAtis set to the transaction timeMetadatais populated with domain event context🔗 Analysis chain
Line range hint
6-15: Verify the impact of init-only propertiesLet's verify that making properties init-only won't break existing event handlers.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for direct assignments to OccuredAt and Metadata properties echo "Checking for direct property assignments..." rg -l "OccuredAt\s*=\s*" --type cs echo "Checking for Metadata modifications..." rg -l "Metadata\s*=\s*" --type cs rg -l "Metadata\.(Add|Clear|Remove)" --type cs # Search for event handler implementations echo "Checking event handler implementations..." ast-grep --pattern 'class $_ : INotificationHandler<$_> { $$ }'Length of output: 857
Script:
#!/bin/bash # Check the actual usage in the interceptor echo "Checking interceptor implementation..." rg -A 5 -B 5 "OccuredAt\s*=|Metadata\s*=" src/Digdir.Domain.Dialogporten.Infrastructure/Persistence/Interceptors/ConvertDomainEventsToOutboxMessagesInterceptor.cs # Check Metadata dictionary modifications echo "Checking Metadata modifications context..." rg -A 5 -B 5 "Metadata\.(Add|Clear|Remove)" src/Digdir.Domain.Dialogporten.WebApi/Program.cs src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/Common/Extensions/RouteHandlerBuilderExtensions.csLength of output: 3834
tests/Digdir.Domain.Dialogporten.Application.Integration.Tests/Features/V1/ServiceOwner/Dialogs/Commands/CreateDialogTests.cs (1)
Line range hint
1-429: Add test coverage for DisableAltinnEvents feature.The test suite lacks coverage for the new DisableAltinnEvents feature introduced in this PR.
Add test cases to verify:
- Dialog creation with DisableAltinnEvents=true for admin users
- Dialog creation with DisableAltinnEvents=true for non-admin users (should fail)
- Dialog creation with DisableAltinnEvents=false (default behavior)
src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Dialogs/Commands/Create/CreateDialogCommandValidator.cs (1)
Line range hint
28-156: Add validation for DisableAltinnEvents property.The CreateDialogDtoValidator lacks validation rules for the new DisableAltinnEvents feature.
Add validation rules to ensure:
- DisableAltinnEvents is only allowed for admin users
- Appropriate error messages when non-admin users attempt to disable events
Example validation rule:
+ RuleFor(x => x.DisableAltinnEvents) + .Equal(false) + .When(x => !currentUser.IsAdmin) + .WithMessage("Only admin users can disable Altinn events.");
🧹 Nitpick comments (26)
src/Digdir.Domain.Dialogporten.Domain/Common/Constants.cs (2)
11-11: Add XML documentation for the new constant.While the constant name is clear, consider adding XML documentation to explain its purpose, usage requirements (admin permissions), and impact on the system when enabled.
+ /// <summary> + /// Feature flag to disable Altinn event generation. Requires admin service owner permissions. + /// </summary> public const string DisableAltinnEvents = "DisableAltinnEvents";
11-11: Consider using a more specific string value.The current implementation uses the constant name as its value. Consider using a more specific string value (e.g., "altinn.events.disabled") to maintain flexibility if the constant name needs to change in the future while preserving backward compatibility with existing configurations.
- public const string DisableAltinnEvents = "DisableAltinnEvents"; + public const string DisableAltinnEvents = "altinn.events.disabled";tests/Digdir.Domain.Dialogporten.Application.Integration.Tests/Features/V1/ServiceOwner/Transmissions/Commands/UpdateTransmissionTests.cs (2)
95-97: LGTM! Consider enhancing error validation.The changes correctly adapt to the new DTO structure. The test effectively validates that old transmissions cannot be included in update commands.
Consider enhancing the error validation to also verify the specific error code/type, not just the error message content. This would make the test more robust against message text changes.
Line range hint
1-134: Consider test class organization with new feature.The test class maintains good separation of concerns and consistent structure. However, with the addition of the event generation disabling feature, consider:
- Creating a separate test class specifically for event generation scenarios
- Or adding a new test category within this class if the scenarios are closely related to transmission updates
This would help maintain clear test organization as new features are added.
tests/Digdir.Domain.Dialogporten.Application.Integration.Tests/Features/V1/ServiceOwner/Dialogs/Commands/PurgeDialogTests.cs (1)
62-65: Optimize the test for non-existing dialog scenario.The current test creates and purges a dialog to test the "not found" scenario. Consider using a new ID directly for a more efficient test:
-var createCommand = DialogGenerator.GenerateFakeCreateDialogCommand(id: expectedDialogId); -await Application.Send(createCommand); -var purgeCommand = new PurgeDialogCommand { DialogId = expectedDialogId }; -await Application.Send(purgeCommand); +var purgeCommand = new PurgeDialogCommand { DialogId = IdentifiableExtensions.CreateVersion7() };tests/Digdir.Domain.Dialogporten.Application.Integration.Tests/Features/V1/ServiceOwner/Dialogs/Commands/UpdateDialogTests.cs (2)
88-90: Consider renaming the test for better clarity.The current test name
Cannot_Include_Old_Transmissions_In_UpdateCommandcould be more specific. Consider renaming it to better describe the validation being performed, e.g.,UpdateDialog_Should_Reject_Command_When_Including_Existing_Transmission_Ids.
127-131: Document the helper method's behavior.The
GenerateDialogWithActivitymethod setsactivity.PerformedBy.ActorNameto null without explaining why. Consider adding XML documentation to explain:
- The method's purpose
- Why it forces a person actor
- Why the actor name is explicitly set to null
Example documentation:
/// <summary> /// Generates a dialog with an activity performed by a person actor. /// </summary> /// <returns>A tuple containing the create command and its result.</returns> /// <remarks> /// The actor name is intentionally set to null to [explain reason]. /// </remarks>tests/Digdir.Domain.Dialogporten.Application.Integration.Tests/Features/V1/ServiceOwner/Dialogs/Queries/GetDialogTests.cs (1)
19-32: Remove unused variable and consider consolidating similar tests.
- The
dtovariable on line 21 is declared but never used in the test.- This test is very similar to
Get_ReturnsDialog_WhenDialogExists. Consider either:
- Consolidating them if they're testing the same behavior
- Or clearly documenting the difference between "simple" and regular dialog commands
- var dto = createDialogCommand.Dto;src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/ServiceOwner/Dialogs/Purge/PurgeDialogEndpoint.cs (1)
59-61: Add documentation and validation for the admin-only feature.While the
[HideFromDocs]attribute suggests this is an admin-only feature, we should:
- Add XML documentation explaining the purpose and impact of this property
- Consider adding validation attributes to enforce admin-level permissions
Apply this diff to improve documentation and validation:
+ /// <summary> + /// Admin-only feature to disable Altinn event generation during dialog purge. + /// Setting this to true requires admin service owner permissions. + /// </summary> [HideFromDocs] + [AdminServiceOwnerOnly] // Add this attribute if available in your codebase public bool? DisableAltinnEvents { get; init; }tests/Digdir.Domain.Dialogporten.Application.Integration.Tests/Features/V1/ServiceOwner/Transmissions/Commands/CreateTransmissionTests.cs (2)
82-82: LGTM! Consider additional security test cases.The test properly validates HTTPS requirement. Consider adding test cases for:
- URLs with mixed case "HtTpS://"
- URLs with port numbers
- URLs with query parameters
Also applies to: 92-92
108-108: LGTM! Consider additional edge cases.The test properly validates automatic ID generation. Consider adding test cases for:
- Multiple levels of related transmissions
- Circular relationships
- Multiple transmissions with null IDs
Also applies to: 117-117
tests/Digdir.Domain.Dialogporten.Application.Unit.Tests/Features/V1/ServiceOwner/Dialogs/Commands/CreateDialogTests.cs (1)
69-69: Consider extracting common test setup.The setup code is duplicated between test methods. Consider extracting the common setup into a test fixture or setup method to improve maintainability.
public class CreateDialogTests { + private readonly IDialogDbContext _dialogDbContextSub; + private readonly IMapper _mapper; + private readonly IUnitOfWork _unitOfWorkSub; + private readonly IDomainContext _domainContextSub; + private readonly IResourceRegistry _resourceRegistrySub; + private readonly IUserResourceRegistry _userResourceRegistrySub; + private readonly IServiceResourceAuthorizer _serviceAuthorizationSub; + private readonly IUser _userSub; + private readonly CreateDialogCommandHandler _commandHandler; + + public CreateDialogTests() + { + _dialogDbContextSub = Substitute.For<IDialogDbContext>(); + _mapper = new MapperConfiguration(cfg => + { + cfg.AddMaps(typeof(CreateDialogCommandHandler).Assembly); + }).CreateMapper(); + _unitOfWorkSub = Substitute.For<IUnitOfWork>(); + _domainContextSub = Substitute.For<IDomainContext>(); + _resourceRegistrySub = Substitute.For<IResourceRegistry>(); + _userResourceRegistrySub = Substitute.For<IUserResourceRegistry>(); + _serviceAuthorizationSub = Substitute.For<IServiceResourceAuthorizer>(); + _userSub = Substitute.For<IUser>(); + + _commandHandler = new CreateDialogCommandHandler(_userSub, _dialogDbContextSub, + _mapper, _unitOfWorkSub, _domainContextSub, + _resourceRegistrySub, _userResourceRegistrySub, _serviceAuthorizationSub); + }Also applies to: 84-84
src/Digdir.Domain.Dialogporten.Domain/Common/DomainEvents/DomainEvent.cs (1)
Line range hint
6-15: Add XML documentation for the domain event class+/// <summary> +/// Base class for all domain events in the system. +/// </summary> public abstract record DomainEvent : IDomainEvent { + /// <summary> + /// Gets the unique identifier for this event. + /// </summary> [JsonInclude] public Guid EventId { get; private set; } = Guid.NewGuid(); + /// <summary> + /// Gets the UTC timestamp when this event occurred. + /// </summary> [JsonInclude] public DateTimeOffset OccuredAt { get; init; } = DateTimeOffset.UtcNow; + /// <summary> + /// Gets the metadata associated with this event. + /// </summary> [JsonInclude] public Dictionary<string, string> Metadata { get; init; } = []; }src/Digdir.Domain.Dialogporten.Domain/Common/EventPublisher/IDomainEvent.cs (1)
20-23: Enhance XML documentation for the Metadata property.The current documentation could be more descriptive. Consider adding:
- Purpose and typical usage scenarios
- Important keys that might be stored
- Any constraints or conventions for key/value pairs
src/Digdir.Domain.Dialogporten.Domain/DomainExtensions.cs (1)
14-15: Add XML documentation for the AddDomain method.The implementation looks good, but consider adding XML documentation to describe:
- Purpose of the method
- When it should be called in the DI setup
- Any dependencies or prerequisites
src/Digdir.Domain.Dialogporten.Application/Common/Behaviours/DomainAltinnEventOptOutBehaviour.cs (2)
12-15: Add null check for constructor parameter.Consider adding a null check for the domainEventContext parameter:
public DomainAltinnEventOptOutBehaviour(IDomainEventContext domainEventContext) { + _domainEventContext = domainEventContext ?? throw new ArgumentNullException(nameof(domainEventContext)); - _domainEventContext = domainEventContext; }
28-31: Enhance interface documentation.Consider adding XML documentation to the interface to:
- Describe its purpose
- Explain when it should be implemented
- Document the implications of setting DisableAltinnEvents to true
src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/ServiceOwner/Dialogs/Create/CreateDialogEndpoint.cs (1)
51-58: Consider documenting the admin-only feature.While
HideFromDocsis appropriate for internal features, consider adding internal documentation about this admin capability for maintainability.Add XML documentation:
public sealed class CreateDialogRequest { + /// <summary> + /// Admin-only feature to disable Altinn event generation. + /// Requires service owner admin scope. + /// </summary> [HideFromDocs] public bool? DisableAltinnEvents { get; init; }src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/ServiceOwner/Dialogs/Update/UpdateDialogEndpoint.cs (1)
70-71: Consider documenting the admin-only feature.Similar to CreateDialogRequest, consider adding internal documentation about this admin capability.
Add XML documentation:
+ /// <summary> + /// Admin-only feature to disable Altinn event generation. + /// Requires service owner admin scope. + /// </summary> [HideFromDocs] public bool? DisableAltinnEvents { get; init; }src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/ServiceOwner/DialogTransmissions/Create/CreateDialogTransmissionEndpoint.cs (1)
98-99: Consider documenting the purpose of this feature.While the
[HideFromDocs]attribute is appropriate for an internal feature, consider adding a code comment explaining the purpose and impact of disabling Altinn events for maintainability.[HideFromDocs] +// Controls whether Altinn events should be generated for this operation. +// When set to true, no events will be forwarded to Altinn. public bool? DisableAltinnEvents { get; init; }src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/ServiceOwner/DialogActivities/Create/CreateDialogActivityEndpoint.cs (1)
100-101: Consider documenting the feature consistently.For consistency with CreateDialogTransmissionEndpoint, add the same documentation comment here.
[HideFromDocs] +// Controls whether Altinn events should be generated for this operation. +// When set to true, no events will be forwarded to Altinn. public bool? DisableAltinnEvents { get; init; }src/Digdir.Domain.Dialogporten.Application/Features/V1/Common/Events/AltinnForwarders/DialogEventToAltinnForwarder.cs (1)
22-26: Consider extracting the common event handling pattern.The same pattern of checking
ShouldNotBeSentToAltinn()is repeated across all event handlers. Consider extracting this to a base method to reduce duplication and improve maintainability.+private async Task HandleEvent<T>(T domainEvent, Func<T, CloudEvent> createCloudEvent, CancellationToken cancellationToken) where T : IDomainEvent +{ + if (domainEvent.ShouldNotBeSentToAltinn()) + { + return; + } + + var cloudEvent = createCloudEvent(domainEvent); + await CloudEventBus.Publish(cloudEvent, cancellationToken); +} public async Task Handle(DialogCreatedDomainEvent domainEvent, CancellationToken cancellationToken) { - if (domainEvent.ShouldNotBeSentToAltinn()) - { - return; - } - - var cloudEvent = new CloudEvent - { - Id = domainEvent.EventId, - Type = CloudEventTypes.Get(nameof(DialogCreatedDomainEvent)), - Time = domainEvent.OccuredAt, - Resource = domainEvent.ServiceResource, - ResourceInstance = domainEvent.DialogId.ToString(), - Subject = domainEvent.Party, - Source = $"{SourceBaseUrl()}{domainEvent.DialogId}", - Data = GetCloudEventData(domainEvent) - }; - - await CloudEventBus.Publish(cloudEvent, cancellationToken); + await HandleEvent(domainEvent, e => new CloudEvent + { + Id = e.EventId, + Type = CloudEventTypes.Get(nameof(DialogCreatedDomainEvent)), + Time = e.OccuredAt, + Resource = e.ServiceResource, + ResourceInstance = e.DialogId.ToString(), + Subject = e.Party, + Source = $"{SourceBaseUrl()}{e.DialogId}", + Data = GetCloudEventData(e) + }, cancellationToken); }Also applies to: 45-49, 68-72, 91-95
src/Digdir.Domain.Dialogporten.Infrastructure/Persistence/Interceptors/ConvertDomainEventsToOutboxMessagesInterceptor.cs (1)
64-65: Consider adding null check for metadata.While the metadata is retrieved and set correctly, consider adding a null check to handle cases where the domain event context might not have metadata.
- var domainEventMetadata = _domainEventContext.GetMetadata(); + var domainEventMetadata = _domainEventContext?.GetMetadata(); EnsureLazyLoadedServices(); foreach (var domainEvent in _domainEvents) { - domainEvent.Metadata = domainEventMetadata; + if (domainEventMetadata is not null) + { + domainEvent.Metadata = domainEventMetadata; + } domainEvent.OccuredAt = _transactionTime.Value; }Also applies to: 69-69
tests/Digdir.Domain.Dialogporten.Application.Integration.Tests/Features/V1/ServiceOwner/Dialogs/Commands/CreateDialogTests.cs (1)
Line range hint
313-331: Improve readability of the activity generation logic.The lambda expressions for setting activity properties could be more readable.
Consider this refactoring for better readability:
- .RuleFor(o => o.PerformedBy, - f => new ActorDto { ActorType = ActorType.Values.PartyRepresentative, ActorName = f.Name.FullName() }) - .RuleFor(o => o.Description, - (f, o) => o.Type == DialogActivityType.Values.Information - ? GenerateFakeLocalizations(f.Random.Number(4, 8)) - : null) + .RuleFor(o => o.PerformedBy, f => new ActorDto + { + ActorType = ActorType.Values.PartyRepresentative, + ActorName = f.Name.FullName() + }) + .RuleFor(o => o.Description, (f, o) => + o.Type == DialogActivityType.Values.Information + ? GenerateFakeLocalizations(f.Random.Number(4, 8)) + : null)src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Dialogs/Commands/Create/CreateDialogCommandValidator.cs (1)
48-48: Improve error message consistency.The error messages for CreatedAt and UpdatedAt validation could be more consistent.
Apply these changes for better message consistency:
- .WithMessage($"{{PropertyName}} must not be empty when '{nameof(CreateDialogDto.UpdatedAt)} is set.") + .WithMessage($"'{nameof(CreateDialogDto.CreatedAt)}' must not be empty when '{nameof(CreateDialogDto.UpdatedAt)}' is set.") - .WithMessage($"'{{PropertyName}}' must be greater than or equal to '{nameof(CreateDialogDto.CreatedAt)}'.") + .WithMessage($"'{nameof(CreateDialogDto.UpdatedAt)}' must be greater than or equal to '{nameof(CreateDialogDto.CreatedAt)}'.")Also applies to: 54-54
src/Digdir.Tool.Dialogporten.GenerateFakeData/DialogGenerator.cs (1)
211-213: Improve readability of party generation logic.The ternary operator makes the party generation logic less readable.
Consider this refactoring:
- return r.Bool() && !forcePerson - ? $"urn:altinn:organization:identifier-no:{GenerateFakeOrgNo()}" - : $"urn:altinn:person:identifier-no:{GenerateFakePid()}"; + var shouldGenerateOrganization = r.Bool() && !forcePerson; + return shouldGenerateOrganization + ? $"urn:altinn:organization:identifier-no:{GenerateFakeOrgNo()}" + : $"urn:altinn:person:identifier-no:{GenerateFakePid()}";
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (50)
docs/schema/V1/swagger.verified.json(3 hunks)src/Digdir.Domain.Dialogporten.Application/ApplicationExtensions.cs(3 hunks)src/Digdir.Domain.Dialogporten.Application/Common/Authorization/Constants.cs(1 hunks)src/Digdir.Domain.Dialogporten.Application/Common/Authorization/IServiceResourceAuthorizer.cs(1 hunks)src/Digdir.Domain.Dialogporten.Application/Common/Behaviours/DomainAltinnEventOptOutBehaviour.cs(1 hunks)src/Digdir.Domain.Dialogporten.Application/Features/V1/Common/Events/AltinnForwarders/DialogActivityEventToAltinnForwarder.cs(2 hunks)src/Digdir.Domain.Dialogporten.Application/Features/V1/Common/Events/AltinnForwarders/DialogEventToAltinnForwarder.cs(7 hunks)src/Digdir.Domain.Dialogporten.Application/Features/V1/Common/Events/AltinnForwarders/DomainEventToAltinnForwarderBase.cs(1 hunks)src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Dialogs/Commands/Create/CreateDialogCommand.cs(8 hunks)src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Dialogs/Commands/Create/CreateDialogCommandValidator.cs(3 hunks)src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Dialogs/Commands/Create/CreateDialogDto.cs(1 hunks)src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Dialogs/Commands/Delete/DeleteDialogCommand.cs(3 hunks)src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Dialogs/Commands/Purge/PurgeDialogCommand.cs(3 hunks)src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Dialogs/Commands/Update/UpdateDialogCommand.cs(4 hunks)src/Digdir.Domain.Dialogporten.Domain/Common/Constants.cs(1 hunks)src/Digdir.Domain.Dialogporten.Domain/Common/DomainEvents/DomainEvent.cs(2 hunks)src/Digdir.Domain.Dialogporten.Domain/Common/DomainEvents/DomainEventContext.cs(1 hunks)src/Digdir.Domain.Dialogporten.Domain/Common/DomainEvents/DomainEventExtensions.cs(1 hunks)src/Digdir.Domain.Dialogporten.Domain/Common/DomainEvents/IDomainEventContext.cs(1 hunks)src/Digdir.Domain.Dialogporten.Domain/Common/EventPublisher/IDomainEvent.cs(1 hunks)src/Digdir.Domain.Dialogporten.Domain/Dialogs/Events/Activities/DialogActivityCreatedDomainEvent.cs(1 hunks)src/Digdir.Domain.Dialogporten.Domain/Dialogs/Events/DialogCreatedDomainEvent.cs(1 hunks)src/Digdir.Domain.Dialogporten.Domain/Dialogs/Events/DialogDeletedDomainEvent.cs(1 hunks)src/Digdir.Domain.Dialogporten.Domain/Dialogs/Events/DialogSeenDomainEvent.cs(1 hunks)src/Digdir.Domain.Dialogporten.Domain/Dialogs/Events/DialogUpdatedDomainEvent.cs(1 hunks)src/Digdir.Domain.Dialogporten.Domain/Digdir.Domain.Dialogporten.Domain.csproj(1 hunks)src/Digdir.Domain.Dialogporten.Domain/DomainExtensions.cs(2 hunks)src/Digdir.Domain.Dialogporten.Infrastructure/Persistence/Interceptors/ConvertDomainEventsToOutboxMessagesInterceptor.cs(3 hunks)src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/ServiceOwner/DialogActivities/Create/CreateDialogActivityEndpoint.cs(2 hunks)src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/ServiceOwner/DialogTransmissions/Create/CreateDialogTransmissionEndpoint.cs(2 hunks)src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/ServiceOwner/Dialogs/Create/CreateDialogEndpoint.cs(3 hunks)src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/ServiceOwner/Dialogs/Delete/DeleteDialogEndpoint.cs(2 hunks)src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/ServiceOwner/Dialogs/Purge/PurgeDialogEndpoint.cs(2 hunks)src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/ServiceOwner/Dialogs/Update/UpdateDialogEndpoint.cs(2 hunks)src/Digdir.Tool.Dialogporten.GenerateFakeData/DialogGenerator.cs(8 hunks)tests/Digdir.Domain.Dialogporten.Application.Integration.Tests/Features/V1/Common/Events/DomainEventsTests.cs(9 hunks)tests/Digdir.Domain.Dialogporten.Application.Integration.Tests/Features/V1/EndUser/Dialogs/Queries/ActivityLogTests.cs(2 hunks)tests/Digdir.Domain.Dialogporten.Application.Integration.Tests/Features/V1/EndUser/Dialogs/Queries/DeletedDialogTests.cs(1 hunks)tests/Digdir.Domain.Dialogporten.Application.Integration.Tests/Features/V1/EndUser/Dialogs/Queries/SeenLogTests.cs(5 hunks)tests/Digdir.Domain.Dialogporten.Application.Integration.Tests/Features/V1/ServiceOwner/Dialogs/Commands/CreateDialogTests.cs(22 hunks)tests/Digdir.Domain.Dialogporten.Application.Integration.Tests/Features/V1/ServiceOwner/Dialogs/Commands/DeleteDialogTests.cs(3 hunks)tests/Digdir.Domain.Dialogporten.Application.Integration.Tests/Features/V1/ServiceOwner/Dialogs/Commands/PurgeDialogTests.cs(3 hunks)tests/Digdir.Domain.Dialogporten.Application.Integration.Tests/Features/V1/ServiceOwner/Dialogs/Commands/UpdateDialogTests.cs(3 hunks)tests/Digdir.Domain.Dialogporten.Application.Integration.Tests/Features/V1/ServiceOwner/Dialogs/Queries/ActivityLogTests.cs(2 hunks)tests/Digdir.Domain.Dialogporten.Application.Integration.Tests/Features/V1/ServiceOwner/Dialogs/Queries/GetDialogTests.cs(2 hunks)tests/Digdir.Domain.Dialogporten.Application.Integration.Tests/Features/V1/ServiceOwner/Dialogs/Queries/SeenLogTests.cs(5 hunks)tests/Digdir.Domain.Dialogporten.Application.Integration.Tests/Features/V1/ServiceOwner/NotificationCondition/NotificationConditionTests.cs(4 hunks)tests/Digdir.Domain.Dialogporten.Application.Integration.Tests/Features/V1/ServiceOwner/Transmissions/Commands/CreateTransmissionTests.cs(7 hunks)tests/Digdir.Domain.Dialogporten.Application.Integration.Tests/Features/V1/ServiceOwner/Transmissions/Commands/UpdateTransmissionTests.cs(3 hunks)tests/Digdir.Domain.Dialogporten.Application.Unit.Tests/Features/V1/ServiceOwner/Dialogs/Commands/CreateDialogTests.cs(2 hunks)
✅ Files skipped from review due to trivial changes (7)
- src/Digdir.Domain.Dialogporten.Domain/Dialogs/Events/DialogCreatedDomainEvent.cs
- src/Digdir.Domain.Dialogporten.Application/Features/V1/Common/Events/AltinnForwarders/DomainEventToAltinnForwarderBase.cs
- src/Digdir.Domain.Dialogporten.Domain/Dialogs/Events/DialogSeenDomainEvent.cs
- src/Digdir.Domain.Dialogporten.Domain/Dialogs/Events/DialogUpdatedDomainEvent.cs
- src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Dialogs/Commands/Create/CreateDialogDto.cs
- src/Digdir.Domain.Dialogporten.Domain/Dialogs/Events/Activities/DialogActivityCreatedDomainEvent.cs
- src/Digdir.Domain.Dialogporten.Domain/Dialogs/Events/DialogDeletedDomainEvent.cs
👮 Files not reviewed due to content moderation or server errors (4)
- tests/Digdir.Domain.Dialogporten.Application.Integration.Tests/Features/V1/ServiceOwner/Dialogs/Commands/DeleteDialogTests.cs
- src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Dialogs/Commands/Delete/DeleteDialogCommand.cs
- tests/Digdir.Domain.Dialogporten.Application.Integration.Tests/Features/V1/ServiceOwner/Dialogs/Queries/ActivityLogTests.cs
- src/Digdir.Domain.Dialogporten.Application/ApplicationExtensions.cs
🔇 Additional comments (47)
tests/Digdir.Domain.Dialogporten.Application.Integration.Tests/Features/V1/EndUser/Dialogs/Queries/DeletedDialogTests.cs (1)
16-16: Great architectural improvement using command pattern!The change from
GenerateSimpleFakeDialog()toGenerateSimpleFakeCreateDialogCommand()better reflects the command-based architecture and provides a more accurate representation of the actual dialog creation flow. This makes the test setup more closely mirror the production code path.tests/Digdir.Domain.Dialogporten.Application.Integration.Tests/Features/V1/ServiceOwner/Dialogs/Queries/SeenLogTests.cs (2)
20-20: LGTM! Consistent update of command generation across test methods.The changes consistently update all test methods to use
GenerateSimpleFakeCreateDialogCommand(), aligning with the new command structure introduced in this PR.Also applies to: 47-47, 76-76, 106-106
56-56: LGTM! Updated property access through DTO.The property access has been correctly updated to use
Dto.ServiceResource, reflecting the new DTO-based command structure.tests/Digdir.Domain.Dialogporten.Application.Integration.Tests/Features/V1/ServiceOwner/Transmissions/Commands/UpdateTransmissionTests.cs (1)
56-58: LGTM! Changes maintain test integrity.The updates correctly adapt to the new DTO structure while preserving the test's original purpose of validating transmission hierarchy handling.
tests/Digdir.Domain.Dialogporten.Application.Integration.Tests/Features/V1/EndUser/Dialogs/Queries/ActivityLogTests.cs (2)
45-45: LGTM! Property access updated to use DTO.The change correctly adapts to the new command structure where properties are accessed through the DTO object.
91-95: LGTM! Test helper updated to use new command structure.The changes correctly adapt the test helper to use the new DTO-based command structure while maintaining its core functionality. The test coverage remains appropriate for verifying activity log behavior.
Let's verify the test coverage for the new command structure:
✅ Verification successful
Test coverage verified: DTO-based command structure consistently used
The changes in the test helper align with the usage patterns across all test files in the codebase. The DTO-based command structure is consistently implemented in both unit and integration tests.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check test coverage for dialog command DTOs # Look for other test files that might need similar updates # Search for test files using dialog commands fd -e cs -t f "Tests.cs$" | xargs rg "GenerateSimpleFakeDialog|CreateDialogCommand" # Search for potential missed DTO property accesses rg "ServiceResource\s+=\s+\[(?!.*Dto)" -t csLength of output: 15690
tests/Digdir.Domain.Dialogporten.Application.Integration.Tests/Features/V1/ServiceOwner/Dialogs/Commands/PurgeDialogTests.cs (1)
45-45: LGTM!The test adequately covers the concurrency scenario, and the command generation change is consistent with other tests.
tests/Digdir.Domain.Dialogporten.Application.Integration.Tests/Features/V1/EndUser/Dialogs/Queries/SeenLogTests.cs (5)
19-19: LGTM! Command generation update aligns with new architecture.The change from
GenerateSimpleFakeDialog()toGenerateSimpleFakeCreateDialogCommand()properly reflects the new command-based dialog creation pattern.
41-41: Property access updated to use DTO pattern.The changes correctly implement the new DTO pattern:
- Dialog creation uses the command-based approach
- Service resource is now accessed through the DTO property, maintaining consistency with the new architecture
Also applies to: 50-50
70-70: Consistent with previous command generation changes.
100-100: Consistent with previous command generation changes.
Line range hint
1-120: Verify test coverage for event disabling feature.While these tests thoroughly cover seen log ID hashing, I notice that the new feature to disable event generation (mentioned in PR objectives) isn't covered in this test file. Consider adding test cases to verify:
- Dialog operations with events disabled
- Dialog operations with events enabled (control case)
- Permission checks for disabling events
✅ Verification successful
Test coverage for event disabling feature is complete.
The feature is thoroughly tested in
DomainEventsTests.cswith comprehensive test cases covering dialog creation, update, deletion, and purge operations with events both enabled and disabled.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for existing test coverage of event disabling feature rg -l "DisableAltinnEvents|IAltinnEventDisabler" "tests/"Length of output: 167
Script:
#!/bin/bash # Check the contents of the domain events test file cat "tests/Digdir.Domain.Dialogporten.Application.Integration.Tests/Features/V1/Common/Events/DomainEventsTests.cs"Length of output: 13434
tests/Digdir.Domain.Dialogporten.Application.Integration.Tests/Features/V1/ServiceOwner/Dialogs/Queries/GetDialogTests.cs (2)
Line range hint
54-54: Add test coverage for event generation disable option.Given that this PR adds the ability to disable event generation, please add test cases to verify:
- Dialog retrieval behavior when events are disabled
- Dialog retrieval behavior when events are enabled
- Edge cases and error scenarios related to the event generation feature
Would you like me to help generate these additional test cases?
Line range hint
1-54: Verify test coverage for event generation feature.Let's analyze the test coverage for the new event generation disable option across the codebase.
✅ Verification successful
Test coverage for event generation disable feature is sufficient
The feature is well-covered by comprehensive integration tests in
DomainEventsTests.csthat verify the event disabling behavior for all supported commands (Create, Update, Delete, and Purge). The test ensures no cloud events are published when the feature is enabled.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for test files related to event generation echo "Searching for existing test coverage of event generation..." rg -l "DisableAltinnEvents" --type cs --glob "*Tests.cs" # Search for potential missing test scenarios echo "Checking for potential untested scenarios..." rg "IAltinnEventDisabler" --type cs --glob "*.cs" --glob "!*Tests.cs"Length of output: 1587
Script:
#!/bin/bash # Check test coverage details echo "Examining test implementation..." rg -A 20 "DisableAltinnEvents" tests/Digdir.Domain.Dialogporten.Application.Integration.Tests/Features/V1/Common/Events/DomainEventsTests.cs echo "Checking behavior tests..." rg -l "DomainAltinnEventOptOutBehaviour" --type cs --glob "*Tests.cs" echo "Checking command tests for event disabling..." for cmd in "CreateDialog" "DeleteDialog" "PurgeDialog" "UpdateDialog"; do echo "=== $cmd tests ===" rg "DisableAltinnEvents.*=.*true" --type cs --glob "*${cmd}Tests.cs" doneLength of output: 3413
src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/ServiceOwner/Dialogs/Purge/PurgeDialogEndpoint.cs (1)
36-42: LGTM! Verify consistent behavior across dialog operations.The command creation with the new
DisableAltinnEventsproperty looks good. The null-coalescing operator ensures a safe default offalse, maintaining backward compatibility.Let's verify that this pattern is consistent across all dialog operations:
✅ Verification successful
Verified: Implementation is consistent across all dialog operations.
The
DisableAltinnEventsproperty is consistently implemented across all dialog operations (Create, Update, Delete, Purge) with:
- Uniform null-coalescing pattern (
?? false)- Common security checks requiring service owner admin scope
- Consistent [HideFromDocs] attribute usage
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for similar command creation patterns in other dialog operations ast-grep --pattern 'new $_DialogCommand { $$$ DisableAltinnEvents = $_.DisableAltinnEvents ?? false $$$ }'Length of output: 114
Script:
#!/bin/bash # Find all dialog command files echo "=== Dialog Command Files ===" fd "Dialog.*Command.cs$" echo -e "\n=== DisableAltinnEvents Usage ===" # Search for DisableAltinnEvents property/field declarations and usages rg "DisableAltinnEvents" -A 2 -B 2Length of output: 22008
tests/Digdir.Domain.Dialogporten.Application.Integration.Tests/Features/V1/ServiceOwner/Transmissions/Commands/CreateTransmissionTests.cs (2)
45-45: LGTM! Comprehensive test coverage maintained.The updates preserve the thorough validation of both transmission creation and embeddable content handling while adapting to the new command structure.
Also applies to: 58-58
23-23: LGTM! Verify command structure changes across the codebase.The updates to use
GenerateSimpleFakeCreateDialogCommandandDto.Transmissionsalign with the new command structure. The test maintains its original validation purpose while adapting to the new patterns.Let's verify the consistency of this command structure change:
Also applies to: 26-26
✅ Verification successful
Command structure changes are consistently implemented across the codebase
All test files have been properly updated to use
GenerateSimpleFakeCreateDialogCommandand theDto.Transmissionsproperty access pattern. The oldGenerateSimpleFakeDialogmethod exists only in the generator tool implementation as expected.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any remaining old-style command generation rg "GenerateSimpleFakeDialog\(" --type cs # Verify the consistent usage of Dto property ast-grep --pattern 'createCommand.Transmissions =' --lang csLength of output: 700
Script:
#!/bin/bash # Search for all transmission property access patterns rg "\.Transmissions" --type cs -A 2 # Find all test files using DialogGenerator rg "DialogGenerator\." --type cs --glob "*Tests.cs" # Search for CreateDialogCommand usage patterns ast-grep --pattern 'var $_ = DialogGenerator.GenerateSimpleFakeCreateDialogCommand($_)'Length of output: 67955
Script:
#!/bin/bash # Search for old and new command generation patterns in test files rg "GenerateSimpleFakeDialog|GenerateSimpleFakeCreateDialogCommand" --type cs --glob "*Tests.cs" -A 1 # Look for transmission property access patterns in test files rg "\.Transmissions" --type cs --glob "*Tests.cs" -A 1 # Find all test files that use DialogGenerator fd -e cs -g "*Tests.cs" -x rg "DialogGenerator" {} -lLength of output: 29791
tests/Digdir.Domain.Dialogporten.Application.Unit.Tests/Features/V1/ServiceOwner/Dialogs/Commands/CreateDialogTests.cs (2)
41-42: Verify complete migration to DTO pattern.The service resource access has been updated to use the DTO pattern. Ensure that all relevant properties have been properly moved to the DTO class and that there are no remaining direct property accesses.
Also applies to: 79-80
✅ Verification successful
DTO migration is complete and consistent
All properties are properly accessed through the DTO pattern, with no remaining direct property accesses found in the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any remaining direct property accesses ast-grep --pattern 'createCommand.$_'Length of output: 2121
34-34: Verify the generated command includes event disabling capability.The command generation has been updated to use
GenerateSimpleFakeCreateDialogCommand(). Ensure that the generated command includes the newDisableAltinnEventsproperty and test both enabled and disabled scenarios.Also applies to: 72-72
✅ Verification successful
Event disabling capability is properly implemented
The
CreateDialogCommandincludes theDisableAltinnEventsproperty, and the codebase shows comprehensive support for this feature including permission checks, domain event handling, and integration tests verifying the functionality.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for the command generator implementation ast-grep --pattern 'GenerateSimpleFakeCreateDialogCommand() { $$$ }'Length of output: 72
Script:
#!/bin/bash # Search with a more flexible pattern for the generator method ast-grep --pattern '$_GenerateSimpleFakeCreateDialogCommand($_) { $$$ }' # Fallback to ripgrep to find any mentions of this method rg "GenerateSimpleFakeCreateDialogCommand" -A 5Length of output: 47084
Script:
#!/bin/bash # Search for CreateDialogCommand class definition ast-grep --pattern 'class CreateDialogCommand { $$$ }' # Search for GenerateSimpleFakeDialog method ast-grep --pattern 'GenerateSimpleFakeDialog($_) { $$$ }' # Fallback to find any mentions of DisableAltinnEvents rg "DisableAltinnEvents"Length of output: 6236
src/Digdir.Domain.Dialogporten.Domain/Common/EventPublisher/IDomainEvent.cs (1)
23-23: Consider thread-safety implications of mutable Metadata.The mutable Dictionary property could lead to thread-safety issues in concurrent scenarios. Consider:
- Making the setter private/protected and initializing in constructor
- Using an immutable dictionary
- Documenting thread-safety expectations
src/Digdir.Domain.Dialogporten.Application/Common/Authorization/Constants.cs (1)
15-16: LGTM! Clear and well-placed constant.The error message is descriptive and the constant follows the existing naming patterns.
src/Digdir.Domain.Dialogporten.Application/Common/Behaviours/DomainAltinnEventOptOutBehaviour.cs (1)
21-21: Verify Constants.DisableAltinnEvents accessibility.The code references Constants.DisableAltinnEvents but this constant is not visible in the current context.
✅ Verification successful
The constant is properly accessible and well-defined
The
Constants.DisableAltinnEventsis defined as a public constant in the Domain project (Digdir.Domain.Dialogporten.Domain.Common.Constants) and is properly referenced by the Application project. No accessibility issues exist.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for Constants.DisableAltinnEvents definition rg -l "DisableAltinnEvents\s*="Length of output: 923
Script:
#!/bin/bash # Check Constants.cs content cat src/Digdir.Domain.Dialogporten.Domain/Common/Constants.cs # Check Application project references cat src/Digdir.Domain.Dialogporten.Application/Digdir.Domain.Dialogporten.Application.csprojLength of output: 2339
src/Digdir.Domain.Dialogporten.Application/Features/V1/Common/Events/AltinnForwarders/DialogActivityEventToAltinnForwarder.cs (1)
19-22: LGTM! Early validation prevents unnecessary processing.The check for
ShouldNotBeSentToAltinn()is well-placed before any cloud event processing, ensuring efficient handling of disabled events.Let's verify the existence of the extension method:
✅ Verification successful
Extension method verified and correctly implemented
The
ShouldNotBeSentToAltinn()extension method is properly defined inDomainEventExtensions.csand is used consistently across event forwarders for early validation. The implementation checks a metadata flag, making it a clean and efficient way to control event forwarding to Altinn.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for the ShouldNotBeSentToAltinn extension method ast-grep --pattern 'static $_? ShouldNotBeSentToAltinn($_)'Length of output: 61
Script:
#!/bin/bash # Search for any occurrence of ShouldNotBeSentToAltinn rg "ShouldNotBeSentToAltinn" -A 2 # Search for files containing "DialogActivity" to find related code fd "DialogActivity" --type fLength of output: 4819
src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/ServiceOwner/Dialogs/Create/CreateDialogEndpoint.cs (1)
13-13: LGTM! Good separation of API contract from command.The change to use
CreateDialogRequestinstead of direct command improves the API design by separating concerns.Also applies to: 34-37
src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/ServiceOwner/Dialogs/Update/UpdateDialogEndpoint.cs (1)
41-42: LGTM! Consistent with CreateDialogEndpoint implementation.The command creation follows the same pattern as CreateDialogEndpoint, maintaining consistency across endpoints.
src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Dialogs/Commands/Purge/PurgeDialogCommand.cs (2)
16-16: LGTM! Clear interface implementation.The command properly implements IAltinnEventDisabler, maintaining consistency with other dialog commands.
Also applies to: 20-20
44-47: LGTM! Proper authorization check.The admin permission check is well-placed before any processing and uses a clear error message.
Let's verify the constant and admin check implementation:
src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/ServiceOwner/DialogTransmissions/Create/CreateDialogTransmissionEndpoint.cs (1)
64-70: LGTM! Command construction handles the nullable property correctly.The null-coalescing operator ensures a non-null value is passed to the command, defaulting to
falsewhen not specified.src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/ServiceOwner/DialogActivities/Create/CreateDialogActivityEndpoint.cs (1)
64-70: LGTM! Consistent implementation with CreateDialogTransmissionEndpoint.The command construction follows the same pattern, maintaining consistency across endpoints.
src/Digdir.Domain.Dialogporten.Infrastructure/Persistence/Interceptors/ConvertDomainEventsToOutboxMessagesInterceptor.cs (1)
20-20: LGTM! Constructor dependency injection is handled correctly.The new dependency is properly injected and validated through constructor injection.
Also applies to: 28-29, 35-35
tests/Digdir.Domain.Dialogporten.Application.Integration.Tests/Features/V1/ServiceOwner/NotificationCondition/NotificationConditionTests.cs (2)
Line range hint
249-322: LGTM! Comprehensive test coverage for disabling Altinn events.The test thoroughly verifies that Altinn events are disabled across all dialog operations (create, update, delete, purge) when
DisableAltinnEventsis set to true. The test structure is clear and follows a good pattern of arrange-act-assert for each operation.
31-32: LGTM! Consistent updates to use the new DTO-based command structure.The changes consistently update dialog command generation and property access to use the new DTO-based structure across all test methods. This refactoring aligns with the broader changes in the codebase to use DTOs for better encapsulation.
Also applies to: 47-48, 65-67, 70-77, 81-83, 95-95, 103-111, 121-123, 127-129, 132-134, 146-148, 182-183, 194-194, 211-212, 224-224, 242-243
src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Dialogs/Commands/Create/CreateDialogCommand.cs (2)
24-28: LGTM! Clean implementation of IAltinnEventDisabler interface.The command class correctly implements the
IAltinnEventDisablerinterface with theDisableAltinnEventsproperty and encapsulates the DTO in a dedicated property.
68-72: LGTM! Proper permission check for disabling Altinn events.The handler correctly verifies that only admin service owners can disable Altinn events, returning a
Forbiddenresult with a clear error message for unauthorized attempts.tests/Digdir.Domain.Dialogporten.Application.Integration.Tests/Features/V1/Common/Events/DomainEventsTests.cs (2)
63-67: LGTM! Consistent updates to use DTOs across all test methods.The changes consistently update property access to use the new DTO-based structure, improving encapsulation and maintainability.
Also applies to: 72-77, 81-83, 95-95, 103-111, 121-123, 127-129, 132-134, 146-148, 182-183, 194-194, 211-212, 224-224, 242-243
249-322: LGTM! Comprehensive test coverage for disabling Altinn events.The test thoroughly verifies that cloud events are not published when
DisableAltinnEventsis set to true across all dialog operations (create, update, delete, purge). The test structure is clear and follows a good pattern of arrange-act-assert for each operation.src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Dialogs/Commands/Update/UpdateDialogCommand.cs (2)
27-27: LGTM! Clean implementation of IAltinnEventDisabler interface.The command class correctly implements the
IAltinnEventDisablerinterface with theDisableAltinnEventsproperty.Also applies to: 32-32
70-74: LGTM! Proper permission check for disabling Altinn events.The handler correctly verifies that only admin service owners can disable Altinn events, returning a
Forbiddenresult with a clear error message for unauthorized attempts.tests/Digdir.Domain.Dialogporten.Application.Integration.Tests/Features/V1/ServiceOwner/Dialogs/Commands/CreateDialogTests.cs (1)
27-27: LGTM! Method calls updated consistently.The changes consistently update the test data generation method calls from
GenerateSimpleFakeDialogtoGenerateSimpleFakeCreateDialogCommandacross all test cases, maintaining the test structure while adapting to the new command pattern.Also applies to: 44-44, 61-61, 78-78, 94-94, 109-109, 126-126, 153-153, 170-170, 187-187, 204-204, 221-222, 243-244, 266-267, 293-294, 312-313, 334-335, 360-361, 391-392, 418-418
src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Dialogs/Commands/Create/CreateDialogCommandValidator.cs (1)
20-26: LGTM! Well-structured command validator.The CreateDialogCommandValidator properly delegates validation to the DTO validator while ensuring the DTO itself is not empty.
src/Digdir.Tool.Dialogporten.GenerateFakeData/DialogGenerator.cs (1)
21-75: LGTM! Well-structured command generation methods.The new methods
GenerateSimpleFakeCreateDialogCommandandGenerateFakeCreateDialogCommandprovide good flexibility for test data generation.src/Digdir.Domain.Dialogporten.Domain/Digdir.Domain.Dialogporten.Domain.csproj (1)
5-5: Verify the package version compatibility.The package version 9.0.0 for Microsoft.Extensions.DependencyInjection.Abstractions seems unusually high and might be a pre-release version.
Please verify:
- The compatibility with other packages in the solution
- Whether a stable version should be used instead
✅ Verification successful
Package version 9.0.0 is verified and compatible
All Microsoft.Extensions packages in the solution are consistently using version 9.0.0, which is the current stable version for .NET 9.0.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check other package versions in the solution rg -g "*.csproj" "Microsoft\.Extensions\..*Version"Length of output: 3004
src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/ServiceOwner/Dialogs/Delete/DeleteDialogEndpoint.cs (2)
36-41: LGTM! Command construction handles the new property correctly.The command construction properly sets the DisableAltinnEvents property with a safe default to false when null.
62-64: Verify admin-only access for DisableAltinnEvents.The HideFromDocs attribute suggests this is an admin-only feature. Ensure proper authorization checks are in place.
✅ Verification successful
Authorization checks for DisableAltinnEvents are properly implemented
The property is secured with two layers of authorization:
- Base endpoint protection with ServiceProvider policy
- Explicit admin-only check in command handler that verifies service owner admin scope
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for authorization checks related to DisableAltinnEvents rg -A 5 "DisableAltinnEvents.*admin"Length of output: 38
Script:
#!/bin/bash # Search for all occurrences of DisableAltinnEvents rg "DisableAltinnEvents" -B 2 -A 2 # Search for authorization attributes in the endpoint file rg "Authorize|Authorization" "src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/ServiceOwner/Dialogs/Delete/DeleteDialogEndpoint.cs" -B 2 -A 2 # Search for any policy definitions related to service owner rg "AddPolicy.*ServiceOwner" -B 2 -A 2Length of output: 22033
src/Digdir.Domain.Dialogporten.Application/Common/Authorization/IServiceResourceAuthorizer.cs (1)
Line range hint
78-82: LGTM! Error message updated to reference DTO instead of command.The change aligns with the architectural shift to use DTOs for data management while maintaining the same error handling logic.
docs/schema/V1/swagger.verified.json (2)
5876-5881: LGTM! Schema updated to use DTO-based request structure.The schema changes properly reflect the architectural change to use DTOs for dialog creation.
3256-3264: LGTM! New request schema definition added.The V1ServiceOwnerDialogsCreate_DialogRequest schema is properly defined with the DTO property.
...Integration.Tests/Features/V1/ServiceOwner/Transmissions/Commands/UpdateTransmissionTests.cs
Show resolved
Hide resolved
....Application.Integration.Tests/Features/V1/ServiceOwner/Dialogs/Commands/PurgeDialogTests.cs
Show resolved
Hide resolved
...Application.Integration.Tests/Features/V1/ServiceOwner/Dialogs/Commands/UpdateDialogTests.cs
Show resolved
Hide resolved
...ten.Application.Integration.Tests/Features/V1/ServiceOwner/Dialogs/Queries/GetDialogTests.cs
Outdated
Show resolved
Hide resolved
...porten.Application.Unit.Tests/Features/V1/ServiceOwner/Dialogs/Commands/CreateDialogTests.cs
Outdated
Show resolved
Hide resolved
src/Digdir.Domain.Dialogporten.Domain/Common/DomainEvents/IDomainEventContext.cs
Outdated
Show resolved
Hide resolved
src/Digdir.Domain.Dialogporten.Domain/Common/DomainEvents/DomainEventContext.cs
Outdated
Show resolved
Hide resolved
src/Digdir.Domain.Dialogporten.Domain/Common/DomainEvents/DomainEventExtensions.cs
Outdated
Show resolved
Hide resolved
src/Digdir.Domain.Dialogporten.Domain/Common/DomainEvents/DomainEvent.cs
Show resolved
Hide resolved
...Digdir.Domain.Dialogporten.Application/Common/Behaviours/DomainAltinnEventOptOutBehaviour.cs
Outdated
Show resolved
Hide resolved
src/Digdir.Domain.Dialogporten.Domain/Common/DomainEvents/DomainEventExtensions.cs
Outdated
Show resolved
Hide resolved
...ogporten.Application/Features/V1/ServiceOwner/Dialogs/Commands/Create/CreateDialogCommand.cs
Outdated
Show resolved
Hide resolved
src/Digdir.Domain.Dialogporten.Domain/Common/DomainEvents/DomainEventContext.cs
Outdated
Show resolved
Hide resolved
oskogstad
changed the title
🤖 I have created a release *beep* *boop* --- ## [1.46.0](v1.45.1...v1.46.0) (2025-01-21) ### Features * **webapi:** Add option to disable Altinn event generation ([#1633](#1633)) ([dda7c1f](dda7c1f)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Description
Related Issue(s)
Verification
Documentation
docs-directory, Altinnpedia or a separate linked PR in altinn-studio-docs., if applicable)