feat(webapi): Combine actorDtos #1374
Conversation
📝 Walkthrough📝 WalkthroughWalkthroughThe pull request introduces significant changes to the Dialogporten API, focusing on the restructuring of actor-related Data Transfer Objects (DTOs) and their mappings. New common actor schemas, Changes
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🔇 Additional comments (3)docs/schema/V1/swagger.verified.json (3)
The new common actor schemas (
Also applies to: 1832-1857
The new actor schemas are properly referenced across all DTOs with:
Also applies to: 456-456, 479-479, 502-502, 763-763, 1027-1027, 1089-1089, 1383-1383, 1430-1430, 1892-1892, 1943-1943, 2043-2043, 2523-2523, 2657-2657, 3124-3124, 3240-3240, 3261-3261, 3538-3538, 3802-3802, 3865-3865, 4181-4181, 4228-4228, 4297-4297, 4457-4457, 4617-4617 Line range hint Let's confirm that the old actor schemas have been completely removed and replaced. ✅ Verification successfulThe verification script results show no matches for any of the old actor schema names in the JSON files, confirming that they have been completely removed from the codebase. The schemas have been properly replaced with the new actor model. All old actor schemas have been successfully removed 🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Description: Search for any remaining references to old actor schemas
# These should return no results if properly removed
echo "Checking for old actor schema references..."
# Check for old EndUser actor schemas
echo "EndUser schemas:"
rg -i "PerformedByActor|LabelAssignmentLogActor|SeenLogSeenByActor" --type json
# Check for old ServiceOwner actor schemas
echo "ServiceOwner schemas:"
rg -i "ActivityPerformedByActor|SenderActor" --type json
echo "Done checking."
Length of output: 372 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
|
Fargekritt
force-pushed
the
feat/combine-actordtos
branch
from
a511f82 to
c006376
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: 16
🧹 Outside diff range and nitpick comments (21)
src/Digdir.Domain.Dialogporten.Application/Features/V1/EndUser/Common/Actors/ActorDto.cs (1)
5-10: Add XML documentation and validation attributes.As this DTO is part of the public API and represents a core concept (Actor), it should have:
- XML documentation explaining the purpose and usage
- Data validation attributes to ensure API consistency
Consider applying these improvements:
+/// <summary> +/// Represents an actor in the system, which can be either an organization or a person. +/// </summary> public sealed class ActorDto { + /// <summary> + /// The type of the actor (e.g., Organization, Person). + /// </summary> + [Required] public ActorType.Values ActorType { get; set; } + /// <summary> + /// The display name of the actor. + /// </summary> + [MaxLength(256)] public string? ActorName { get; set; } + /// <summary> + /// The unique identifier of the actor. + /// </summary> + [MaxLength(128)] public string? ActorId { get; set; } }src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Common/Actors/ActorDto.cs (1)
5-10: Add XML documentation for better API understanding.Consider adding XML documentation to describe the purpose of the DTO and its properties, including when nullable properties are expected to be null.
+/// <summary> +/// Represents a common actor data transfer object used across the application. +/// </summary> public sealed class ActorDto { + /// <summary> + /// Gets or sets the type of the actor. + /// </summary> public ActorType.Values ActorType { get; set; } + + /// <summary> + /// Gets or sets the name of the actor. Can be null for system actors or when name is not available. + /// </summary> public string? ActorName { get; set; } + + /// <summary> + /// Gets or sets the unique identifier of the actor. Can be null for anonymous or system actors. + /// </summary> public string? ActorId { get; set; } }src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/DialogSeenLogs/Queries/Get/MappingProfile.cs (1)
Line range hint
1-14: Add unit tests for the updated mappingGiven the significant changes in DTO structure, we should ensure the mapping behavior is tested, especially the conversion to the new common
ActorDto.Would you like me to help create unit tests for this mapping profile to verify the correct mapping of actor-related properties?
src/Digdir.Domain.Dialogporten.Application/Features/V1/EndUser/DialogSeenLogs/Queries/Get/MappingProfile.cs (1)
Line range hint
8-14: Add tests for the updated mapping configuration.Since this PR involves significant changes to DTO mappings, please ensure that unit tests are added to verify:
- The correct mapping of DialogSeenLog to SeenLogDto
- The proper handling of actor-related properties with the new ActorDto
Would you like me to help generate the unit tests for these mapping scenarios?
src/Digdir.Domain.Dialogporten.Application/Features/V1/EndUser/DialogTransmissions/Queries/Search/MappingProfile.cs (2)
Line range hint
15-17: Consider adding XML documentationSince this is a public class handling important DTO mappings, consider adding XML documentation to describe its purpose and the mapping configurations it contains.
+/// <summary> +/// Configures AutoMapper mappings for dialog transmission-related DTOs in the end-user context. +/// </summary> public sealed class MappingProfile : Profile
Missing mapping for Sender property in TransmissionDto
The TransmissionDto class clearly has a Sender property of type ActorDto, but the mapping configuration in MappingProfile.cs doesn't include the mapping for this property. This needs to be addressed to ensure proper object mapping.
- Add mapping configuration for the Sender property in
src/Digdir.Domain.Dialogporten.Application/Features/V1/EndUser/DialogTransmissions/Queries/Search/MappingProfile.cs:CreateMap<DialogTransmission, TransmissionDto>() .ForMember(dest => dest.Type, opt => opt.MapFrom(src => src.TypeId)) .ForMember(dest => dest.DeletedAt, opt => opt.MapFrom(src => src.Dialog.DeletedAt)) .ForMember(dest => dest.Sender, opt => opt.MapFrom(src => src.SenderActor)); // Add this line🔗 Analysis chain
Line range hint
13-27: Verify actor mapping configuration for TransmissionDtoThe mapping profile seems to be missing the configuration for the Sender property of TransmissionDto, which should now be of type ActorDto according to the PR objectives.
Let's verify the TransmissionDto structure and ensure we're not missing any required mappings:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check TransmissionDto and related actor mappings # Look for TransmissionDto definition echo "Checking TransmissionDto definition:" ast-grep --pattern 'class TransmissionDto { $$$ }' # Look for any remaining references to old actor DTOs echo -e "\nChecking for any remaining old actor DTO references:" rg -l "SenderActorDto|PerformedByActorDto"Length of output: 478
Script:
#!/bin/bash # Let's check the TransmissionDto definition and its mapping configuration more thoroughly # Find and show TransmissionDto definition echo "=== TransmissionDto Definition ===" rg -A 10 "class TransmissionDto" # Check the mapping configuration in MappingProfile echo -e "\n=== MappingProfile Configuration ===" rg -A 10 "CreateMap<DialogTransmission, TransmissionDto>" # Check if there are any actor-related mappings in the same file echo -e "\n=== Actor-related Mappings ===" rg "CreateMap.*Actor" src/Digdir.Domain.Dialogporten.Application/Features/V1/EndUser/DialogTransmissions/Queries/Search/MappingProfile.cs # Check DialogTransmission entity to understand available properties echo -e "\n=== DialogTransmission Entity Definition ===" rg -A 10 "class DialogTransmission"Length of output: 39251
Script:
#!/bin/bash # Let's check the EndUser TransmissionDto definition specifically and look for any Sender/Actor properties echo "=== EndUser TransmissionDto Full Definition ===" rg -A 30 "class TransmissionDto" src/Digdir.Domain.Dialogporten.Application/Features/V1/EndUser/DialogTransmissions/Queries/Search/TransmissionDto.cs # Check if there are any actor-related mappings in the base mapping profiles echo -e "\n=== Actor Mappings in Base Profiles ===" rg -l "CreateMap.*Actor" src/Digdir.Domain.Dialogporten.Application/Features/V1/Common/ # Check if there are any global actor mappings that might be inherited echo -e "\n=== Global Actor Mappings ===" rg "CreateMap.*Actor" src/Digdir.Domain.Dialogporten.Application/Features/V1/Common/Actors/Length of output: 1583
Script:
#!/bin/bash # Let's check for the Sender property and actor-related properties in TransmissionDto # Find the complete TransmissionDto definition echo "=== Complete TransmissionDto Definition ===" fd -e cs TransmissionDto.cs src/Digdir.Domain.Dialogporten.Application/Features/V1/EndUser/DialogTransmissions/Queries/Search/ -x cat {} # Check for any actor-related mappings in the global profile echo -e "\n=== Global Actor Mappings ===" fd MappingProfile.cs src/Digdir.Domain.Dialogporten.Application/Features/V1/Common/Actors/ -x cat {} # Check if there are any actor mappings in parent classes echo -e "\n=== Parent Profile Mappings ===" rg -A 5 "class MappingProfile.*:" src/Digdir.Domain.Dialogporten.Application/Features/V1/EndUser/DialogTransmissions/Queries/Search/Length of output: 5991
src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/DialogTransmissions/Queries/Get/MappingProfile.cs (1)
Line range hint
11-27: Add unit tests for mapping configurations.Given the significant changes to the actor DTO structure, please add unit tests to verify:
- Correct mapping of transmission properties
- Proper integration with the new common actor DTO
- Edge cases and null handling
This aligns with the PR objectives which note that automated tests are missing.
Would you like me to help generate the unit test structure for these mapping configurations?
src/Digdir.Domain.Dialogporten.Application/Features/V1/EndUser/Common/Actors/MappingProfile.cs (2)
26-28: Document identifier masking rules.While the mapping configuration is correct, consider adding XML documentation to explain:
- When and why identifiers are masked
- The masking rules implemented in
IdentifierMaskerAdd documentation above the mapping:
+/// <summary> +/// Configures the base mapping from Actor to ActorDto. +/// The ActorId is masked according to privacy rules defined in IdentifierMasker. +/// </summary> CreateMap<Actor, ActorDto>()
1-36: Add unit tests for mapping configurations.As noted in the PR objectives, tests are missing. This mapping profile is critical for the actor DTO consolidation effort and should be thoroughly tested to ensure:
- All expected Actor types are discovered
- Mappings work correctly for both base and derived types
- Identifier masking behaves as expected
Would you like me to help create unit tests for this mapping profile?
src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Common/Actors/MappingProfile.cs (2)
1-7: Consider adding XML documentation for the mapping profile.Since this is a crucial component that handles actor mappings across the application, adding XML documentation would help other developers understand its purpose and responsibility.
+/// <summary> +/// AutoMapper profile that configures bidirectional mappings between actor DTOs and domain entities. +/// This profile supports dynamic mapping of all non-abstract Actor derivatives. +/// </summary> internal sealed class MappingProfile : Profile
1-43: Add unit tests for the mapping configurations.Since this profile handles critical DTO-Domain mappings and includes reflection-based type discovery, comprehensive unit tests are essential to verify:
- All derived actor types are correctly discovered
- Bidirectional mappings work for each actor type
- ActorType/ActorTypeId mappings maintain data consistency
Would you like me to help create a test suite for this mapping profile?
src/Digdir.Domain.Dialogporten.GraphQL/EndUser/Common/MappingProfile.cs (1)
Line range hint
1-34: Consider adding XML documentation for mapping configurations.Given that this PR aims to consolidate actor DTOs, it would be beneficial to add XML documentation comments explaining the mapping configurations, especially since there are multiple contexts where ActorDto is used (EndUser and ServiceOwner).
Add documentation like this:
+ /// <summary> + /// Maps the common ActorDto to the GraphQL Actor type. + /// This mapping is used in both end-user and service-owner contexts. + /// </summary> CreateMap<ActorDto, Actor>() .ForMember(dest => dest.ActorType, opt => opt.MapFrom(src => src.ActorType));src/Digdir.Domain.Dialogporten.Application/Features/V1/EndUser/Dialogs/Queries/Search/MappingProfile.cs (3)
Line range hint
15-52: Add unit tests for actor DTO mappings.The PR objectives indicate that automated tests are missing. Please add unit tests to verify:
- Correct mapping of actor-related properties to the new unified ActorDto
- Proper handling of actor identifiers and types
- Edge cases in the mapping configurations
Would you like me to help generate the unit test code for these mapping scenarios?
Line range hint
15-52: Update mapping documentation.Since this is part of a significant restructuring of actor DTOs, please add XML documentation comments to the MappingProfile class explaining:
- The purpose of these mappings
- The relationship with the unified ActorDto structure
- Any special considerations for actor-related mappings
Example documentation:
+/// <summary> +/// Defines AutoMapper mappings for dialog-related DTOs, including the unified actor DTO structure. +/// </summary> +/// <remarks> +/// This profile handles mappings between domain entities and DTOs, ensuring proper translation +/// of actor-related properties using the consolidated ActorDto representation. +/// </remarks> internal sealed class MappingProfile : Profile
Based on the gathered information, I can now provide a final response:
Missing actor mappings for DialogActivity and incomplete mapping for DialogSeenLog
The codebase requires updates to properly map actor-related properties:
- DialogSeenLog: While there's a mapping for
DialogSeenLogSeenByActortoActorDto, it's only present in the ServiceOwner feature. The EndUser feature needs the same mapping.- DialogActivity: No mapping exists for
DialogActivityPerformedByActortoActorDtoin any feature, despite having aDialogActivityPerformedByActorDtoclass.🔗 Analysis chain
Line range hint
45-46: Update DialogSeenLog and DialogActivity mappings for new ActorDto.The mappings for DialogSeenLog and DialogActivity might need updates to accommodate the new unified ActorDto structure. Please verify if these entities contain actor-related properties that should now map to the new ActorDto.
Also applies to: 48-49
From the search results, I can see that both DialogSeenLog and DialogActivity entities have actor-related properties:
- DialogSeenLog has a
SeenByproperty of typeDialogSeenLogSeenByActorwhich inherits fromActor- DialogActivity has a
DialogActivityPerformedByActorwhich also inherits fromActorLet's verify the current mapping configurations for these actor properties:
Let's check one more thing to see if there are any DialogActivityPerformedByActor mappings in other mapping profiles:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for actor-related properties in DialogSeenLog and DialogActivity echo "Checking DialogSeenLog properties:" ast-grep --pattern 'class DialogSeenLog { $$$ Actor $_ { get; set; } $$$ }' echo -e "\nChecking DialogActivity properties:" ast-grep --pattern 'class DialogActivity { $$$ Actor $_ { get; set; } $$$ }'Length of output: 482
Script:
#!/bin/bash # Let's try a different approach to find these classes and their properties echo "Searching for DialogSeenLog class definition:" rg "class DialogSeenLog" -A 10 echo -e "\nSearching for DialogActivity class definition:" rg "class DialogActivity" -A 10 echo -e "\nSearching for any mapping configurations related to these classes:" rg "CreateMap.*Dialog(SeenLog|Activity)" -A 3Length of output: 36022
Script:
#!/bin/bash # Search for actor-related mappings in the mapping profiles echo "Searching for actor mappings in DialogSeenLog:" rg "CreateMap.*DialogSeenLogSeenByActor" -A 3 echo -e "\nSearching for actor mappings in DialogActivity:" rg "CreateMap.*DialogActivityPerformedByActor" -A 3 echo -e "\nSearching for DialogSeenLogDto and DialogActivityDto definitions to see actor properties:" rg "class DialogSeenLogDto|class DialogActivityDto" -B 2 -A 15Length of output: 19847
Script:
#!/bin/bash # Search for any mapping configurations related to DialogActivityPerformedByActor echo "Searching for DialogActivityPerformedByActor mappings in all mapping profiles:" rg "DialogActivityPerformedByActor" -A 5 "src/Digdir.Domain.Dialogporten.Application/Features/" # Also check if DialogActivityDto has a PerformedBy property in any file echo -e "\nSearching for PerformedBy property in DialogActivityDto:" rg -B 2 -A 10 "class DialogActivityDto" | rg -A 15 -B 2 "PerformedBy"Length of output: 1273
src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Dialogs/Queries/Search/MappingProfile.cs (1)
Line range hint
1-35: Consider adding unit tests for the updated mappings.Given that the PR checklist indicates missing automated tests, it would be beneficial to add unit tests to verify the mapping configurations, especially for the consolidated actor mappings.
Would you like me to help generate unit tests for these mapping configurations?
src/Digdir.Domain.Dialogporten.Application/Features/V1/EndUser/Dialogs/Queries/Search/DialogDtoBase.cs (1)
129-129: Update XML documentation for actor propertiesSince we're transitioning to the common
ActorDto, please update the XML documentation forSeenByandPerformedByproperties to reflect any changes in the structure or available fields of the new DTO.Also applies to: 176-176
src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Dialogs/Commands/Update/UpdateDialogCommandValidator.cs (2)
Line range hint
143-159: Consider improving error message clarity for actor validation rules.While the validation logic is correct, the error message for the complex actor validation rule could be more descriptive. Consider breaking down the message to clearly indicate the specific validation failure.
- .WithMessage(ActorValidationErrorMessages.ActorIdActorNameExclusiveOr); + .WithMessage(dto => + dto.ActorType == ActorType.Values.ServiceOwner + ? "Service owner actors must not have ActorId or ActorName" + : "Non-service owner actors must have either ActorId or ActorName, but not both");
Line range hint
433-450: Consider consolidating duplicate actor validation logic.The validation rules in this class are identical to those in
UpdateDialogDialogTransmissionActorDtoValidator. Consider creating a common base validator or shared validator for ActorDto to avoid duplication.You could create a base validator class:
internal abstract class BaseActorDtoValidator : AbstractValidator<ActorDto> { protected BaseActorDtoValidator() { RuleFor(x => x.ActorType) .IsInEnum(); RuleFor(x => x) .Must(dto => (dto.ActorId is null || dto.ActorName is null) && ((dto.ActorType == ActorType.Values.ServiceOwner && dto.ActorId is null && dto.ActorName is null) || (dto.ActorType != ActorType.Values.ServiceOwner && (dto.ActorId is not null || dto.ActorName is not null)))) .WithMessage(ActorValidationErrorMessages.ActorIdActorNameExclusiveOr); RuleFor(x => x.ActorId!) .IsValidPartyIdentifier() .When(x => x.ActorId is not null); } } internal sealed class UpdateDialogDialogTransmissionActorDtoValidator : BaseActorDtoValidator { } internal sealed class UpdateDialogDialogActivityActorDtoValidator : BaseActorDtoValidator { }src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Dialogs/Commands/Create/CreateDialogDto.cs (1)
197-197: Consider enhancing the XML documentation for the Sender property.While the change to use the consolidated
ActorDtois good, consider enhancing the XML documentation to describe what types of actors are valid as senders (e.g., service owner, end user, etc.) and any validation rules that apply.src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Dialogs/Commands/Create/CreateDialogCommandValidator.cs (1)
Line range hint
448-482: Consider consolidating duplicate actor validatorsThe
CreateDialogDialogTransmissionActorDtoValidatorandCreateDialogDialogActivityActorDtoValidatorhave identical validation rules. Consider consolidating them into a single base validator or shared validator to follow the DRY principle.Example refactor:
-internal sealed class CreateDialogDialogTransmissionActorDtoValidator : AbstractValidator<ActorDto> -{ - public CreateDialogDialogTransmissionActorDtoValidator() - { - RuleFor(x => x.ActorType) - .IsInEnum(); - - RuleFor(x => x) - .Must(dto => (dto.ActorId is null || dto.ActorName is null) && - ((dto.ActorType == ActorType.Values.ServiceOwner && dto.ActorId is null && dto.ActorName is null) || - (dto.ActorType != ActorType.Values.ServiceOwner && (dto.ActorId is not null || dto.ActorName is not null)))) - .WithMessage(ActorValidationErrorMessages.ActorIdActorNameExclusiveOr); - - RuleFor(x => x.ActorId!) - .IsValidPartyIdentifier() - .When(x => x.ActorId is not null); - } -} - -internal sealed class CreateDialogDialogActivityActorDtoValidator : AbstractValidator<ActorDto> -{ - public CreateDialogDialogActivityActorDtoValidator() - { - RuleFor(x => x.ActorType) - .IsInEnum(); - - RuleFor(x => x) - .Must(dto => (dto.ActorId is null || dto.ActorName is null) && - ((dto.ActorType == ActorType.Values.ServiceOwner && dto.ActorId is null && dto.ActorName is null) || - (dto.ActorType != ActorType.Values.ServiceOwner && (dto.ActorId is not null || dto.ActorName is not null)))) - .WithMessage(ActorValidationErrorMessages.ActorIdActorNameExclusiveOr); - - RuleFor(x => x.ActorId!) - .IsValidPartyIdentifier() - .When(x => x.ActorId is not null); - } -} +internal sealed class CreateDialogActorDtoValidator : AbstractValidator<ActorDto> +{ + public CreateDialogActorDtoValidator() + { + RuleFor(x => x.ActorType) + .IsInEnum(); + + RuleFor(x => x) + .Must(dto => (dto.ActorId is null || dto.ActorName is null) && + ((dto.ActorType == ActorType.Values.ServiceOwner && dto.ActorId is null && dto.ActorName is null) || + (dto.ActorType != ActorType.Values.ServiceOwner && (dto.ActorId is not null || dto.ActorName is not null)))) + .WithMessage(ActorValidationErrorMessages.ActorIdActorNameExclusiveOr); + + RuleFor(x => x.ActorId!) + .IsValidPartyIdentifier() + .When(x => x.ActorId is not null); + } +}
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (47)
docs/schema/V1/swagger.verified.json(30 hunks)src/Digdir.Domain.Dialogporten.Application/Features/V1/EndUser/Common/Actors/ActorDto.cs(1 hunks)src/Digdir.Domain.Dialogporten.Application/Features/V1/EndUser/Common/Actors/MappingProfile.cs(1 hunks)src/Digdir.Domain.Dialogporten.Application/Features/V1/EndUser/DialogActivities/Queries/Get/ActivityDto.cs(2 hunks)src/Digdir.Domain.Dialogporten.Application/Features/V1/EndUser/DialogActivities/Queries/Get/MappingProfile.cs(0 hunks)src/Digdir.Domain.Dialogporten.Application/Features/V1/EndUser/DialogLabelAssignmentLog/Queries/Search/LabelAssignmentLogDto.cs(2 hunks)src/Digdir.Domain.Dialogporten.Application/Features/V1/EndUser/DialogLabelAssignmentLog/Queries/Search/MappingProfile.cs(1 hunks)src/Digdir.Domain.Dialogporten.Application/Features/V1/EndUser/DialogSeenLogs/Queries/Get/MappingProfile.cs(1 hunks)src/Digdir.Domain.Dialogporten.Application/Features/V1/EndUser/DialogSeenLogs/Queries/Get/SeenLogDto.cs(1 hunks)src/Digdir.Domain.Dialogporten.Application/Features/V1/EndUser/DialogSeenLogs/Queries/Search/MappingProfile.cs(1 hunks)src/Digdir.Domain.Dialogporten.Application/Features/V1/EndUser/DialogSeenLogs/Queries/Search/SeenLogDto.cs(1 hunks)src/Digdir.Domain.Dialogporten.Application/Features/V1/EndUser/DialogTransmissions/Queries/Get/MappingProfile.cs(1 hunks)src/Digdir.Domain.Dialogporten.Application/Features/V1/EndUser/DialogTransmissions/Queries/Get/TransmissionDto.cs(2 hunks)src/Digdir.Domain.Dialogporten.Application/Features/V1/EndUser/DialogTransmissions/Queries/Search/MappingProfile.cs(1 hunks)src/Digdir.Domain.Dialogporten.Application/Features/V1/EndUser/DialogTransmissions/Queries/Search/TransmissionDto.cs(2 hunks)src/Digdir.Domain.Dialogporten.Application/Features/V1/EndUser/Dialogs/Queries/Get/DialogDto.cs(4 hunks)src/Digdir.Domain.Dialogporten.Application/Features/V1/EndUser/Dialogs/Queries/Get/MappingProfile.cs(0 hunks)src/Digdir.Domain.Dialogporten.Application/Features/V1/EndUser/Dialogs/Queries/Search/DialogDtoBase.cs(3 hunks)src/Digdir.Domain.Dialogporten.Application/Features/V1/EndUser/Dialogs/Queries/Search/MappingProfile.cs(1 hunks)src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Common/Actors/ActorDto.cs(1 hunks)src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Common/Actors/MappingProfile.cs(1 hunks)src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/DialogActivities/Queries/Get/ActivityDto.cs(2 hunks)src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/DialogActivities/Queries/Get/MappingProfile.cs(0 hunks)src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/DialogSeenLogs/Queries/Get/MappingProfile.cs(1 hunks)src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/DialogSeenLogs/Queries/Get/SeenLogDto.cs(1 hunks)src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/DialogSeenLogs/Queries/Search/MappingProfile.cs(0 hunks)src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/DialogSeenLogs/Queries/Search/SeenLogDto.cs(1 hunks)src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/DialogTransmissions/Queries/Get/MappingProfile.cs(1 hunks)src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/DialogTransmissions/Queries/Get/TransmissionDto.cs(2 hunks)src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/DialogTransmissions/Queries/Search/MappingProfile.cs(1 hunks)src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/DialogTransmissions/Queries/Search/TransmissionDto.cs(2 hunks)src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Dialogs/Commands/Create/CreateDialogCommandValidator.cs(5 hunks)src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Dialogs/Commands/Create/CreateDialogDto.cs(3 hunks)src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Dialogs/Commands/Create/MappingProfile.cs(0 hunks)src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Dialogs/Commands/Update/MappingProfile.cs(0 hunks)src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Dialogs/Commands/Update/UpdateDialogCommandValidator.cs(5 hunks)src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Dialogs/Commands/Update/UpdateDialogDto.cs(3 hunks)src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Dialogs/Queries/Get/DialogDto.cs(4 hunks)src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Dialogs/Queries/Get/MappingProfile.cs(2 hunks)src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Dialogs/Queries/Search/DialogDtoBase.cs(3 hunks)src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Dialogs/Queries/Search/MappingProfile.cs(1 hunks)src/Digdir.Domain.Dialogporten.GraphQL/EndUser/Common/MappingProfile.cs(2 hunks)src/Digdir.Domain.Dialogporten.GraphQL/EndUser/DialogById/MappingProfile.cs(1 hunks)src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/ServiceOwner/Dialogs/Update/UpdateDialogSwaggerConfig.cs(2 hunks)src/Digdir.Tool.Dialogporten.GenerateFakeData/DialogGenerator.cs(2 hunks)tests/Digdir.Domain.Dialogporten.Application.Integration.Tests/Features/V1/ServiceOwner/Dialogs/Commands/UpdateDialogTests.cs(2 hunks)tests/Digdir.Domain.Dialogporten.Application.Integration.Tests/Features/V1/ServiceOwner/Dialogs/Queries/GetDialogTests.cs(1 hunks)
💤 Files with no reviewable changes (6)
- src/Digdir.Domain.Dialogporten.Application/Features/V1/EndUser/DialogActivities/Queries/Get/MappingProfile.cs
- src/Digdir.Domain.Dialogporten.Application/Features/V1/EndUser/Dialogs/Queries/Get/MappingProfile.cs
- src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/DialogActivities/Queries/Get/MappingProfile.cs
- src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/DialogSeenLogs/Queries/Search/MappingProfile.cs
- src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Dialogs/Commands/Create/MappingProfile.cs
- src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Dialogs/Commands/Update/MappingProfile.cs
✅ Files skipped from review due to trivial changes (2)
- src/Digdir.Domain.Dialogporten.Application/Features/V1/EndUser/DialogSeenLogs/Queries/Search/MappingProfile.cs
- tests/Digdir.Domain.Dialogporten.Application.Integration.Tests/Features/V1/ServiceOwner/Dialogs/Queries/GetDialogTests.cs
🧰 Additional context used
📓 Learnings (2)
src/Digdir.Domain.Dialogporten.Application/Features/V1/EndUser/DialogLabelAssignmentLog/Queries/Search/LabelAssignmentLogDto.cs (4)
Learnt from: elsand
PR: digdir/dialogporten#1192
File: src/Digdir.Domain.Dialogporten.Application/Features/V1/EndUser/DialogLabelAssignmentLog/Queries/Search/MappingProfile.cs:9-13
Timestamp: 2024-10-03T06:11:58.673Z
Learning: `LabelAssignmentLogActor` extends `Actor`, which contains `ActorName` and `ActorId`.
Learnt from: elsand
PR: digdir/dialogporten#1192
File: src/Digdir.Domain.Dialogporten.Application/Features/V1/EndUser/DialogLabelAssignmentLog/Queries/Search/MappingProfile.cs:9-13
Timestamp: 2024-10-08T15:39:27.070Z
Learning: `LabelAssignmentLogActor` extends `Actor`, which contains `ActorName` and `ActorId`.
Learnt from: elsand
PR: digdir/dialogporten#1192
File: src/Digdir.Domain.Dialogporten.Application/Features/V1/EndUser/DialogLabelAssignmentLog/Queries/Search/MappingProfile.cs:9-13
Timestamp: 2024-10-08T15:39:28.388Z
Learning: `LabelAssignmentLogActor` extends `Actor`, which contains `ActorName` and `ActorId`.
Learnt from: elsand
PR: digdir/dialogporten#1192
File: src/Digdir.Domain.Dialogporten.Application/Features/V1/EndUser/DialogLabelAssignmentLog/Queries/Search/MappingProfile.cs:9-13
Timestamp: 2024-10-08T15:39:24.789Z
Learning: `LabelAssignmentLogActor` extends `Actor`, which contains `ActorName` and `ActorId`.
src/Digdir.Domain.Dialogporten.Application/Features/V1/EndUser/DialogLabelAssignmentLog/Queries/Search/MappingProfile.cs (4)
Learnt from: elsand
PR: digdir/dialogporten#1192
File: src/Digdir.Domain.Dialogporten.Application/Features/V1/EndUser/DialogLabelAssignmentLog/Queries/Search/MappingProfile.cs:9-13
Timestamp: 2024-10-08T15:39:27.070Z
Learning: `LabelAssignmentLogActor` extends `Actor`, which contains `ActorName` and `ActorId`.
Learnt from: elsand
PR: digdir/dialogporten#1192
File: src/Digdir.Domain.Dialogporten.Application/Features/V1/EndUser/DialogLabelAssignmentLog/Queries/Search/MappingProfile.cs:9-13
Timestamp: 2024-10-03T06:11:58.673Z
Learning: `LabelAssignmentLogActor` extends `Actor`, which contains `ActorName` and `ActorId`.
Learnt from: elsand
PR: digdir/dialogporten#1192
File: src/Digdir.Domain.Dialogporten.Application/Features/V1/EndUser/DialogLabelAssignmentLog/Queries/Search/MappingProfile.cs:9-13
Timestamp: 2024-10-08T15:39:28.388Z
Learning: `LabelAssignmentLogActor` extends `Actor`, which contains `ActorName` and `ActorId`.
Learnt from: elsand
PR: digdir/dialogporten#1192
File: src/Digdir.Domain.Dialogporten.Application/Features/V1/EndUser/DialogLabelAssignmentLog/Queries/Search/MappingProfile.cs:9-13
Timestamp: 2024-10-08T15:39:24.789Z
Learning: `LabelAssignmentLogActor` extends `Actor`, which contains `ActorName` and `ActorId`.
🔇 Additional comments (71)
src/Digdir.Domain.Dialogporten.Application/Features/V1/EndUser/Common/Actors/ActorDto.cs (2)
1-3: LGTM! Clean and minimal dependencies.
The using directive is appropriate, and the namespace follows the project's convention.
5-10: Verify the impact of nullable properties.
Since this DTO consolidates multiple actor DTOs, we should verify if ActorName and ActorId should be nullable in all scenarios.
Let's check how these properties were defined in the removed DTOs:
✅ Verification successful
Nullable properties are correctly defined based on validation rules
The validation rules in both Create and Update command validators confirm that the nullable properties are correctly defined:
- Either
ActorIdorActorNamemust be null (they are mutually exclusive) - For
ServiceOwnertype:- Both
ActorIdandActorNamemust be null
- Both
- For non-
ServiceOwnertypes:- Either
ActorIdorActorNamemust be non-null
- Either
The nullability is intentional and enforced through validation rules, ensuring the DTO correctly handles all actor scenarios.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for removed actor DTOs and their property definitions
rg -A 10 "class.*ActorDto\s*{" --glob "!**/Common/**"
Length of output: 157
Script:
#!/bin/bash
# Search for removed actor DTOs and their property definitions with fixed regex
rg -A 10 "class.*ActorDto" --glob "!**/Common/**"
Length of output: 9363
Script:
#!/bin/bash
# Search for any usage of ActorDto properties to understand nullability requirements
rg "\.ActorName|\.ActorId" --glob "!**/Common/**"
Length of output: 10767
Script:
#!/bin/bash
# Look for any tests that might show expected values
rg "ActorDto.*=.*new" -A 5
Length of output: 28
Script:
#!/bin/bash
# Check git history for the consolidated DTO changes
git log -p --all --full-history -- "**/ActorDto*.cs"
Length of output: 2830
src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Common/Actors/ActorDto.cs (2)
1-4: LGTM!
The namespace structure and using statement are clean and well-organized.
5-10: Verify the impact of this consolidated DTO.
Since this DTO is replacing multiple specific DTOs, we should verify:
- All required actor information is captured
- No information loss from the previous DTOs
- Compatibility with existing consumers
✅ Verification successful
Based on the search results, I can now generate the final response:
The consolidated ActorDto appears to be properly integrated and maintains compatibility.
The verification shows:
- The consolidated ActorDto captures all essential actor information (type, name, id) used across the codebase
- Proper mapping profiles exist in both ServiceOwner and EndUser contexts (src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Common/Actors/MappingProfile.cs and EndUser equivalent)
- The DTO is consistently used across all relevant contexts:
- Dialog activities (PerformedBy)
- Seen logs (SeenBy)
- Transmissions (Sender)
- Label assignments (PerformedBy)
The only remaining old DTO (DialogActivityPerformedByActorDto) appears to be in a different context and contains the same essential fields, suggesting no information loss in the consolidation.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Search for removed actor DTOs and their usages to ensure complete migration
# Test 1: Find all removed actor DTOs
echo "Searching for removed actor DTOs..."
rg -l "ActivityPerformedByActorDto|DialogSeenLogSeenByActorDto|SenderActorDto"
# Test 2: Find any remaining references to old DTOs that might need updating
echo "Searching for remaining references to old DTOs..."
rg "ActivityPerformedByActorDto|DialogSeenLogSeenByActorDto|SenderActorDto"
# Test 3: Find all current usages of the new ActorDto to verify proper integration
echo "Searching for usages of new ActorDto..."
rg "ActorDto" -A 5
Length of output: 42406
src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/DialogSeenLogs/Queries/Get/SeenLogDto.cs (2)
1-2: LGTM! Using statement aligns with the consolidation effort.
The addition of the Common.Actors namespace import supports the transition to the unified ActorDto.
10-10: Verify mapping configuration for the new ActorDto property.
The property type change from SeenLogSeenByActorDto to ActorDto aligns with the PR objective to consolidate actor DTOs. However, we should verify that the corresponding mapping configuration has been updated.
✅ Verification successful
Based on the search results, I can now generate the final response:
Mapping configuration for ActorDto property is properly configured
The mapping configuration for the SeenBy property of type ActorDto is correctly set up in multiple mapping profiles:
DialogSeenLogSeenByActortoActorDtomapping is configured in ServiceOwner/Dialogs/Queries/Get/MappingProfile.csActortoActorDtomapping is configured in EndUser/Common/Actors/MappingProfile.cs with proper property mappings- The entity relationship between
DialogSeenLogandDialogSeenLogSeenByActoris properly configured in the database schema
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that mapping configuration exists for SeenLogDto.SeenBy property
# Search for mapping configuration in profile classes
rg -A 5 "CreateMap.*SeenLogDto"
# Search for any remaining references to the old DTO
rg "SeenLogSeenByActorDto"
Length of output: 8354
Script:
#!/bin/bash
# Let's check the mapping configuration for DialogSeenLog to SeenLogDto more thoroughly
# and look for ActorDto mapping configurations
# Search for DialogSeenLog to SeenLogDto mapping with more context
rg -A 10 "CreateMap<DialogSeenLog,\s*SeenLogDto"
# Search for ActorDto mapping configurations
rg -A 5 "CreateMap.*ActorDto"
# Search for the SeenBy property in mapping configurations
rg "ForMember.*SeenBy"
Length of output: 8397
Script:
#!/bin/bash
# Let's check the DialogSeenLog entity and DialogSeenLogSeenByActor to understand the source mapping
# Search for DialogSeenLog class definition
ast-grep --pattern 'class DialogSeenLog {
$$$
}'
# Search for DialogSeenLogSeenByActor class/type definition
ast-grep --pattern 'class DialogSeenLogSeenByActor {
$$$
}'
# Look for SeenBy property in DialogSeenLog
rg "SeenBy" --type csharp -A 3 -B 3
Length of output: 127676
src/Digdir.Domain.Dialogporten.Application/Features/V1/EndUser/DialogLabelAssignmentLog/Queries/Search/MappingProfile.cs (2)
2-2: LGTM: Import aligns with actor DTO consolidation.
The addition of the Common.Actors import aligns with the PR's objective to use common actor DTOs.
Line range hint 8-13: Verify mapping requirements for LabelAssignmentLogActor.
While removing the specific actor DTO mapping aligns with the consolidation goal, we need to ensure that LabelAssignmentLogActor is properly mapped to the common ActorDto if needed.
Let's verify the mapping requirements:
src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/DialogSeenLogs/Queries/Get/MappingProfile.cs (1)
Line range hint 1-14: Verify mapping configuration for ActorDto
The removal of DialogSeenLogSeenByActor mapping and addition of the common actors namespace aligns with the PR objective of consolidating actor DTOs. However, we should verify that the SeenLogDto.SeenBy property is properly mapped to the new ActorDto.
✅ Verification successful
Based on the gathered information, I can now generate the final response. The code shows that:
- The
SeenLogDtohas a propertySeenByof typeActorDto - The
DialogSeenLogentity has aSeenByproperty of typeDialogSeenLogSeenByActor - There's a mapping configuration from
DialogSeenLogSeenByActortoActorDto - Common actor mapping profiles exist in both EndUser and ServiceOwner features
Mapping configuration is complete and correct
The mapping configuration is properly set up for the actor consolidation:
- The base mapping
CreateMap<DialogSeenLogSeenByActor, ActorDto>()is defined - Common actor mappings exist in dedicated profiles
- The
SeenByproperty will be automatically mapped due to AutoMapper's convention-based mapping
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the SeenLogDto structure and its mapping configuration
# Expected: SeenLogDto should use ActorDto for its SeenBy property
# Check SeenLogDto definition
rg -l "class SeenLogDto" | xargs rg "SeenBy.*ActorDto"
# Check for any remaining references to SeenLogSeenByActorDto
rg "SeenLogSeenByActorDto"
Length of output: 83
Script:
#!/bin/bash
# Let's try a different approach to verify the mapping configuration
# First, let's find and examine the SeenLogDto class definition
ast-grep --pattern 'class SeenLogDto {
$$$
}'
# Also check the DialogSeenLog entity to understand its properties
ast-grep --pattern 'class DialogSeenLog {
$$$
}'
# Look for any mapping configurations related to DialogSeenLog or SeenLogDto
rg -A 5 "CreateMap.*DialogSeenLog.*SeenLogDto"
Length of output: 6620
Script:
#!/bin/bash
# Let's find the SeenLogDto and DialogSeenLog definitions to understand their properties
rg -A 10 "class SeenLogDto"
rg -A 10 "class DialogSeenLog"
# Also check for any AutoMapper configuration related to SeenBy property
rg -B 2 -A 2 "SeenBy.*MapFrom"
Length of output: 12861
Script:
#!/bin/bash
# Let's check for any mapping configurations related to SeenBy or DialogSeenLogSeenByActor
rg -B 2 -A 2 "CreateMap.*DialogSeenLogSeenByActor"
# Also check for any Actor mapping configurations
rg -B 2 -A 2 "CreateMap.*Actor"
# And check if there's a common Actor mapping profile
fd -e cs "MappingProfile" | xargs rg -l "Actor"
Length of output: 9495
src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/DialogSeenLogs/Queries/Search/SeenLogDto.cs (2)
1-2: Verify the necessity of both actor-related imports.
The file imports both common and service-owner specific actor namespaces. Since we're consolidating to common DTOs, verify if both imports are necessary.
11-11: LGTM! Verify consumers of this breaking change.
The change from SeenLogSeenByActorDto to ActorDto aligns with the PR objective to consolidate actor DTOs. However, this is a breaking change that requires verification of all consumers.
✅ Verification successful
Breaking change properly handled across the codebase
The change from SeenLogSeenByActorDto to ActorDto has been properly implemented across the codebase:
- All
SeenLogDtoimplementations consistently useActorDtofor theSeenByproperty - Proper mapping configurations exist for
ActorDtoin both EndUser and ServiceOwner contexts - The mapping profiles handle the conversion between
ActorandActorDtowith appropriate property mappings - All endpoints using
SeenLogDtoare aligned with this change
The consolidation to ActorDto is complete and consistent throughout the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Find all potential consumers of SeenLogDto to ensure they're updated
# Search for direct references to SeenLogDto
echo "Files referencing SeenLogDto:"
rg -l "SeenLogDto" --type cs
# Search for any remaining references to the old DTO
echo -e "\nChecking for any remaining references to SeenLogSeenByActorDto:"
rg -l "SeenLogSeenByActorDto" --type cs
Length of output: 3342
Script:
#!/bin/bash
# Let's examine the mapping profiles and consumers to verify the breaking change is properly handled
# Check mapping profiles for SeenLogDto to see how ActorDto is mapped
echo "Checking mapping profiles for SeenLogDto:"
rg -A 5 "CreateMap.*SeenLogDto" --type cs
# Check the actual implementations of endpoints using SeenLogDto
echo -e "\nChecking endpoint implementations:"
rg -A 5 "SeenLogDto" src/Digdir.Domain.Dialogporten.WebApi/Endpoints/ --type cs
Length of output: 15668
Script:
#!/bin/bash
# Let's check the actual mapping configurations for ActorDto to ensure proper handling
# Check ActorDto mappings
echo "Checking ActorDto mappings:"
rg -A 5 "CreateMap.*ActorDto" --type cs
# Check if there are any remaining references to the old type in mapping configurations
echo -e "\nChecking for any remaining SeenLogSeenByActorDto mappings:"
rg -A 5 "SeenLogSeenByActorDto" --type cs
# Check the actual SeenLogDto implementations to verify property type
echo -e "\nChecking SeenLogDto implementations:"
rg -A 10 "class SeenLogDto" --type cs
Length of output: 11166
src/Digdir.Domain.Dialogporten.Application/Features/V1/EndUser/DialogLabelAssignmentLog/Queries/Search/LabelAssignmentLogDto.cs (2)
1-2: LGTM! Using directives align with DTO consolidation.
The new imports support the transition to common actor DTOs, which aligns with the PR's objective of streamlining actor representations.
14-14: Verify impact on API consumers.
The change from LabelAssignmentLogActorDto to ActorDto aligns with the PR's objective of consolidating actor DTOs. However, as this is a breaking change in the API contract, we should verify its impact.
Let's verify the usage and potential impact:
✅ Verification successful
Let me try a different search strategy to verify the impact.
Based on the search results, I can now provide a final response:
Breaking change is properly contained within the API version
The change from LabelAssignmentLogActorDto to ActorDto is part of a consistent DTO consolidation pattern across the V1 API namespace. The impact is limited to:
- The endpoint
SearchDialogLabelAssignmentLogEndpointin V1 API - Internal mapping profiles and query handlers within the V1 feature set
- All usages are properly versioned under the
V1namespace
Since this change is contained within a versioned API (V1) and follows a consistent pattern of actor DTO consolidation across the codebase, it's a safe breaking change that follows API versioning best practices.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Search for API consumers that might be affected by the DTO change
# Test 1: Find direct usages of LabelAssignmentLogActorDto
echo "Searching for direct usages of LabelAssignmentLogActorDto..."
rg -l "LabelAssignmentLogActorDto"
# Test 2: Find API controllers or handlers using LabelAssignmentLogDto
echo "Searching for API endpoints using LabelAssignmentLogDto..."
ast-grep --pattern 'class $_ : $Controller {
$$$
$ReturnType $Method($_) {
$$$
LabelAssignmentLogDto
$$$
}
$$$
}'
Length of output: 428
Script:
#!/bin/bash
# Search for any references to LabelAssignmentLogDto and its usage context
echo "Searching for LabelAssignmentLogDto usage..."
rg -A 5 "LabelAssignmentLogDto"
# Search for API endpoints in the controllers directory
echo "Searching in controller files..."
fd -e cs -e cshtml . src/ --exec rg -l "LabelAssignmentLogDto|ActorDto"
# Look for any test files that might be using these DTOs
echo "Searching in test files..."
fd -e cs . test/ --exec rg -l "LabelAssignmentLogDto|ActorDto"
Length of output: 8965
src/Digdir.Domain.Dialogporten.Application/Features/V1/EndUser/DialogSeenLogs/Queries/Get/SeenLogDto.cs (1)
10-10: LGTM! Property type change aligns with PR objectives.
The change from SeenLogSeenByActorDto to ActorDto aligns with the PR's goal of consolidating actor DTOs. The non-null constraint is maintained.
Let's verify the mapping configuration for this change:
✅ Verification successful
Let me verify the mapping configurations for ActorDto to ensure the change is properly handled.
Property type change is properly mapped and safe to use
The mapping configurations are correctly set up for the ActorDto change:
DialogSeenLogSeenByActorinherits fromActorand is mapped toActorDtoin the mapping profiles- Bidirectional mappings exist between
ActorandActorDtoin multiple profiles - The domain entity
DialogSeenLogand its related DTOs maintain consistent mapping configurations
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that mapping profiles have been updated for the new ActorDto
# Expected: Find mapping configurations that handle SeenLogDto.SeenBy
# Search for mapping profiles related to SeenLogDto
ast-grep --pattern 'CreateMap<$_, SeenLogDto>()
$$$'
# Or search for any mapping configuration mentioning SeenBy
rg -A 5 'CreateMap.*SeenLogDto'
Length of output: 9032
Script:
#!/bin/bash
# Search for mapping configurations related to Actor/ActorDto
ast-grep --pattern 'CreateMap<$_, Actor>()
$$$'
ast-grep --pattern 'CreateMap<$_, ActorDto>()
$$$'
# Search for DialogSeenLog entity definition to understand the source property
rg -A 5 "class DialogSeenLog"
Length of output: 5514
src/Digdir.Domain.Dialogporten.Application/Features/V1/EndUser/DialogSeenLogs/Queries/Get/MappingProfile.cs (1)
3-3: Verify actor mapping configuration.
With the transition to common ActorDto, please verify that the mapping for actor-related properties is properly configured, either in this profile or in a common mapping profile.
✅ Verification successful
Let me gather more specific information about the actor mappings and DialogSeenLog structure.
Let me check one final piece of information about the SeenLogDto to complete the verification.
Actor mapping is properly configured - no action needed
The actor mapping configuration is correctly handled through:
- The common
ActortoActorDtomapping inEndUser/Common/Actors/MappingProfile.cswhich handles all actor types includingDialogSeenLogSeenByActor - The
DialogSeenLogtoSeenLogDtomapping correctly maps the timestamp, and theSeenByproperty will be automatically mapped toActorDtothrough the common mapping
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any ActorDto mapping configurations
echo "Searching for ActorDto mapping configurations..."
rg -l "CreateMap.*ActorDto"
# Search for potential unmapped actor properties
echo "Checking for potential unmapped actor properties in DialogSeenLog..."
ast-grep --pattern 'class DialogSeenLog {
$$$
}'
Length of output: 709
Script:
#!/bin/bash
# Check the content of relevant mapping profiles
echo "Content of EndUser/Common/Actors/MappingProfile.cs:"
cat src/Digdir.Domain.Dialogporten.Application/Features/V1/EndUser/Common/Actors/MappingProfile.cs
echo -e "\nContent of DialogSeenLog mapping:"
rg -A 5 "CreateMap.*DialogSeenLog" src/Digdir.Domain.Dialogporten.Application/Features/V1/EndUser/DialogSeenLogs/Queries/Get/MappingProfile.cs
echo -e "\nSearching for DialogSeenLog class definition:"
rg -A 10 "class DialogSeenLog"
Length of output: 9538
Script:
#!/bin/bash
# Check SeenLogDto structure
echo "Searching for SeenLogDto definition:"
rg -A 10 "class SeenLogDto"
# Check if there are any other mappings for DialogSeenLogSeenByActor
echo -e "\nSearching for DialogSeenLogSeenByActor mappings:"
rg "CreateMap.*DialogSeenLogSeenByActor"
Length of output: 5718
src/Digdir.Domain.Dialogporten.Application/Features/V1/EndUser/DialogSeenLogs/Queries/Search/SeenLogDto.cs (1)
1-2: LGTM!
The new imports are correctly added to support the ActorDto usage.
src/Digdir.Domain.Dialogporten.Application/Features/V1/EndUser/DialogActivities/Queries/Get/ActivityDto.cs (1)
2-2: LGTM! Verify mapping configuration exists.
The import change aligns with the PR objective of consolidating actor DTOs.
Let's verify the mapping configuration exists:
✅ Verification successful
Let me gather more information about the mapping configuration.
Mapping configuration exists and is properly set up
The mapping configuration for both ActivityDto and ActorDto is properly configured in multiple mapping profiles:
EndUser.Common.Actors.MappingProfilehandles the mapping betweenActorandActorDtoEndUser.DialogActivities.Queries.Get.MappingProfilehandles the mapping forActivityDto
The change from PerformedByActorDto to ActorDto is supported by the existing mapping configurations, which include proper handling of actor types and properties.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if mapping profile exists for ActorDto in EndUser context
rg -A 10 "CreateMap.*ActorDto"
Length of output: 3710
Script:
#!/bin/bash
# Search for mapping profiles related to ActivityDto and ActorDto
rg -A 5 "CreateMap.*Activity.*Dto|CreateMap.*Actor.*Dto" --type cs
# Search for any AutoMapper profile classes
ast-grep --pattern 'class $_ : Profile { $$$ }'
Length of output: 16137
src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/DialogActivities/Queries/Get/ActivityDto.cs (1)
1-1: LGTM! Import statements properly organized.
The new imports correctly support the consolidated actor DTO structure.
Also applies to: 3-3
src/Digdir.Domain.Dialogporten.Application/Features/V1/EndUser/DialogTransmissions/Queries/Get/MappingProfile.cs (2)
2-2: LGTM: Using directive aligns with actor DTO consolidation.
The addition of the Common.Actors namespace import supports the PR's objective of using consolidated actor DTOs.
Line range hint 12-27: Verify actor mapping configuration after SenderActorDto removal.
The mapping for DialogTransmissionSenderActor has been removed, but there's no visible replacement mapping for the consolidated ActorDto. Please verify that actor mapping is properly configured elsewhere in the codebase.
Let's verify the actor mapping configuration:
✅ Verification successful
Actor mapping is properly configured in dedicated mapping profiles
The actor mapping has been consolidated and is now properly configured in dedicated mapping profiles:
EndUser/Common/Actors/MappingProfile.cshandles mapping fromActortoActorDtofor end-user scenariosServiceOwner/Common/Actors/MappingProfile.cshandles bi-directional mapping betweenActorandActorDtofor service owner scenarios
Both profiles use a dynamic approach to map all derived actor types through base type mapping, eliminating the need for individual mappings like the removed DialogTransmissionSenderActor mapping. The consolidated ActorDto is consistently defined in both contexts with the same properties: ActorType, ActorName, and ActorId.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if actor mapping is configured in other mapping profiles
# and verify the usage of the new ActorDto
# Search for actor-related mapping configurations
echo "Searching for actor mapping configurations..."
rg -l "CreateMap.*Actor" --type cs
# Search for usage of the new ActorDto
echo "Checking usage of consolidated ActorDto..."
rg -l "ActorDto" --type cs
# Look for any remaining references to the old SenderActorDto
echo "Checking for any remaining references to SenderActorDto..."
rg -l "SenderActorDto" --type cs
Length of output: 3776
Script:
#!/bin/bash
# Let's check the actual actor mapping configurations in the relevant files
echo "Checking actor mapping in EndUser/Common/Actors/MappingProfile.cs..."
cat src/Digdir.Domain.Dialogporten.Application/Features/V1/EndUser/Common/Actors/MappingProfile.cs
echo -e "\nChecking actor mapping in ServiceOwner/Common/Actors/MappingProfile.cs..."
cat src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Common/Actors/MappingProfile.cs
echo -e "\nChecking the ActorDto definition in EndUser..."
cat src/Digdir.Domain.Dialogporten.Application/Features/V1/EndUser/Common/Actors/ActorDto.cs
echo -e "\nChecking the ActorDto definition in ServiceOwner..."
cat src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Common/Actors/ActorDto.cs
Length of output: 4432
src/Digdir.Domain.Dialogporten.Application/Features/V1/EndUser/DialogTransmissions/Queries/Search/MappingProfile.cs (1)
2-2: LGTM: Common actors import added
The addition of the Common.Actors import aligns with the PR objective of consolidating actor DTOs.
src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/DialogTransmissions/Queries/Get/MappingProfile.cs (2)
2-2: LGTM: Using directive aligns with actor DTO consolidation.
The addition of the Common.Actors namespace import supports the PR's objective of using a unified actor DTO.
Line range hint 15-26: Verify actor-related properties in TransmissionDto.
While the existing mappings look correct, we should verify that any actor-related properties in TransmissionDto have been updated to use the new common ActorDto.
Let's verify the TransmissionDto structure:
✅ Verification successful
No actor-related property updates needed in TransmissionDto
The TransmissionDto already uses the new common ActorDto for its Sender property, and there are no other actor-related properties that need updating. The mapping configuration in the original file is consistent with the DTO structure.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check TransmissionDto for actor-related properties
# and verify they use the new ActorDto
# Search for TransmissionDto definition
ast-grep --pattern 'class TransmissionDto {
$$$
}'
# Search for any remaining references to old actor DTOs
rg -l "SenderActorDto|ActivityPerformedByActorDto"
Length of output: 282
Script:
#!/bin/bash
# Let's check the content of files that still reference old DTOs
rg -A 10 "SenderActorDto|ActivityPerformedByActorDto"
# Also let's find TransmissionDto definition and usages
fd -e cs -x grep -l "TransmissionDto"
# And check DialogTransmission entity to understand the properties
ast-grep --pattern 'class DialogTransmission {
$$$
}'
Length of output: 6543
Script:
#!/bin/bash
# Let's check the TransmissionDto definition in the same directory as the mapping file
cat src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/DialogTransmissions/Queries/Get/TransmissionDto.cs
# And check if there are any actor-related properties in the DialogTransmission entity
ast-grep --pattern 'class DialogTransmission {
$$$
Actor $$$
$$$
}'
Length of output: 4409
src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/DialogTransmissions/Queries/Search/MappingProfile.cs (2)
2-2: LGTM: Using directive aligns with DTO consolidation.
The addition of the Common.Actors namespace import supports the PR objective of consolidating actor DTOs into a common location.
Line range hint 13-26: Verify complete removal of SenderActorDto mappings.
The removal of DialogTransmissionSenderActor mapping aligns with the PR objective. Let's verify that all related actor mappings have been consistently updated across the codebase.
✅ Verification successful
Let me generate additional verification scripts to check for any remaining references to the old actor-related code:
This will help us verify if there are any remaining references to the old actor-related code and confirm if the migration to Common.Actors is complete.
Let me try one more verification with corrected path patterns:
SenderActorDto mappings have been successfully removed and replaced with Common.Actors
The verification confirms that:
- No SenderActorDto references exist in mapping profiles
- Old DialogTransmissionSenderActor references only remain in migration history files
- All actor-related mappings now use the new Common.Actors types (ActorDto)
- The codebase consistently uses the new Common.Actors namespace across both EndUser and ServiceOwner features
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify complete removal of old actor DTOs and consistent usage of new common DTOs
# Test 1: Check for any remaining references to SenderActorDto
echo "Checking for remaining SenderActorDto references..."
rg -l "SenderActorDto"
# Test 2: Check for any remaining DialogTransmissionSenderActor mappings
echo "Checking for remaining DialogTransmissionSenderActor mappings..."
ast-grep --pattern 'CreateMap<DialogTransmissionSenderActor, $_>'
# Test 3: Verify adoption of new Common.Actors namespace
echo "Verifying adoption of Common.Actors..."
rg -l "using.*Common\.Actors"
Length of output: 4277
Script:
#!/bin/bash
# Description: Deep dive into actor-related mappings and DTOs
# Check for any DialogTransmissionSenderActor type references
echo "Checking for DialogTransmissionSenderActor references..."
rg -l "DialogTransmissionSenderActor"
# Check for any *SenderActor* patterns in mapping profiles
echo "Checking for SenderActor patterns in mapping profiles..."
rg -l "SenderActor" ".*MappingProfile\.cs$"
# Check for any actor-related mapping configurations
echo "Looking for CreateMap patterns with 'Actor' in mapping profiles..."
ast-grep --pattern 'CreateMap<$_, $_>().ForMember($_)' -- '**/MappingProfile.cs' | grep -i "actor"
Length of output: 2905
Script:
#!/bin/bash
# Description: Final verification of actor-related code
# Check for SenderActor patterns in mapping profiles (with correct path pattern)
echo "Checking for SenderActor patterns in mapping profiles..."
fd -e cs -x rg -l "SenderActor" {} \;
# Check for any actor-related mapping configurations in mapping profiles
echo "Looking for actor-related mappings..."
fd "MappingProfile.cs" -x cat {} \; | grep -i "CreateMap.*Actor"
# Check for any remaining actor-related DTOs
echo "Checking for actor DTOs..."
fd -e cs -x rg -l "ActorDto" {} \;
Length of output: 6157
src/Digdir.Domain.Dialogporten.Application/Features/V1/EndUser/Common/Actors/MappingProfile.cs (2)
1-13: LGTM! Class structure follows best practices.
The class is correctly marked as internal and sealed, with appropriate imports and namespace organization.
14-25: Verify all Actor types are discovered correctly.
While using reflection to discover Actor types is flexible, it's important to verify that all expected types are found at runtime.
Let's verify the Actor subclasses:
✅ Verification successful
Let me gather more specific information about the Actor implementations.
Let me try a different approach to find the Actor implementations and structure.
Let me search for specific Actor implementations in the codebase.
Based on the gathered information, I can now provide the final response:
Actor types are properly discovered and implemented in the codebase
The reflection-based approach in MappingProfile correctly discovers all Actor implementations. The codebase shows:
- Base abstract
Actorclass with proper entity properties ActorTypeenum defining valid types (PartyRepresentative, ServiceOwner)- Four concrete Actor implementations:
DialogTransmissionSenderActorDialogActivityPerformedByActorDialogSeenLogSeenByActorLabelAssignmentLogActor
All implementations are properly sealed classes that inherit from Actor and implement IImmutableEntity, following a consistent pattern.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Find all Actor subclasses to verify reflection coverage
# Find all concrete Actor subclasses
echo "Concrete Actor subclasses:"
ast-grep --pattern 'class $name : $$$Actor$$$' | grep -v 'abstract class'
# Find any potential Actor subclasses that might be missed
echo -e "\nPotential missed Actor types:"
rg -l "Actor" | grep -i "actor.*\.cs$"
Length of output: 1546
Script:
#!/bin/bash
# Find concrete Actor class implementations with their inheritance
echo "Actor implementations with inheritance:"
ast-grep --pattern 'class $name : $$$Actor {
$$$
}'
# Find Actor class definition to understand expected structure
echo -e "\nBase Actor class definition:"
ast-grep --pattern 'class Actor {
$$$
}'
# Search for ActorType enum values to cross-reference with implementations
echo -e "\nActorType enum values:"
rg -A 10 "enum ActorType" src/Digdir.Domain.Dialogporten.Domain/Actors/ActorType.cs
Length of output: 409
Script:
#!/bin/bash
# Find Actor class definition with more flexible pattern
echo "Actor class definition:"
ast-grep --pattern 'class Actor'
# Find concrete implementations with more flexible pattern
echo -e "\nActor implementations:"
ast-grep --pattern 'class $_ : Actor'
# Get content of Actor.cs to understand the structure
echo -e "\nActor.cs content:"
cat src/Digdir.Domain.Dialogporten.Domain/Actors/Actor.cs
# Get content of ActorType.cs
echo -e "\nActorType.cs content:"
cat src/Digdir.Domain.Dialogporten.Domain/Actors/ActorType.cs
Length of output: 139488
Script:
#!/bin/bash
# Find concrete Actor implementations in specific files
echo "Actor implementations in domain:"
rg -A 5 "class.*: Actor" src/Digdir.Domain.Dialogporten.Domain/
# Find Actor implementations in dialog-related files
echo -e "\nActor implementations in dialog entities:"
rg -A 5 "class.*: Actor" src/Digdir.Domain.Dialogporten.Domain/Dialogs/
# Find any potential Actor implementations in transmission-related files
echo -e "\nActor implementations in transmission entities:"
rg -A 5 "class.*: Actor" src/Digdir.Domain.Dialogporten.Domain/Dialogs/Entities/Transmissions/
Length of output: 5688
src/Digdir.Domain.Dialogporten.GraphQL/EndUser/DialogById/MappingProfile.cs (2)
1-2: Verify actor mapping configuration.
With the removal of DialogTransmissionSenderActorDto mapping and the introduction of the common actors namespace, ensure that mapping configurations for the new ActorDto are properly set up to prevent runtime mapping errors.
✅ Verification successful
The search results show that proper mapping configurations for ActorDto are in place across the codebase:
- In
src/Digdir.Domain.Dialogporten.GraphQL/EndUser/Common/MappingProfile.cs, there are multipleCreateMap<ActorDto, Actor>()configurations - In both service owner and end user contexts (
Features/V1/ServiceOwner/Common/Actors/MappingProfile.csandFeatures/V1/EndUser/Common/Actors/MappingProfile.cs), bidirectional mappings betweenActorandActorDtoare properly configured - The removed
DialogTransmissionSenderActorDtomapping has been effectively replaced with the consolidatedActorDtomappings
Actor mapping configurations are properly set up.
The consolidation of actor DTOs has been correctly implemented with appropriate mapping configurations in place, ensuring no runtime mapping errors will occur.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if ActorDto is used in any of the mapped types and verify corresponding mapping configurations
# Search for ActorDto usage in related DTOs
echo "Checking ActorDto usage in DTOs:"
rg -l "ActorDto" --type cs
# Search for Actor mapping configurations
echo -e "\nChecking Actor mapping configurations:"
rg "CreateMap.*Actor" --type cs
Length of output: 4660
Line range hint 19-20: Verify DTOs have been updated for the consolidated ActorDto.
The mapped types DialogTransmissionDto, DialogAttachmentDto, and related DTOs might need to be updated to use the new consolidated ActorDto. Please ensure these types have been properly updated to maintain mapping consistency.
Also applies to: 23-24, 26-27
✅ Verification successful
Let me try a different search strategy to find these DTOs and their relationships with ActorDto.
Based on the search results, I can see that the ActorDto is consistently used across the codebase in both EndUser and ServiceOwner features. The DTOs in question (DialogTransmissionDto and DialogAttachmentDto) are properly using the consolidated ActorDto:
- DialogTransmissionDto uses ActorDto for its Sender property:
public sealed class DialogTransmissionDto
{
public ActorDto Sender { get; set; } = null!;
// ...
}- DialogAttachmentDto is properly mapped without any actor-related properties:
public sealed class DialogAttachmentDto
{
public Guid Id { get; set; }
// ...
}The mapping profiles in both EndUser and ServiceOwner features are correctly configured to use the consolidated ActorDto, with proper mapping configurations in place.
The DTOs have already been updated to use the consolidated ActorDto.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check the implementation of transmission and attachment DTOs
# Search for DialogTransmissionDto implementation
echo "Checking DialogTransmissionDto implementation:"
ast-grep --pattern 'class DialogTransmissionDto {
$$$
}'
# Search for DialogAttachmentDto implementation
echo -e "\nChecking DialogAttachmentDto implementation:"
ast-grep --pattern 'class DialogAttachmentDto {
$$$
}'
Length of output: 327
Script:
#!/bin/bash
# Search for any file containing DialogTransmissionDto or DialogAttachmentDto
echo "Files containing DTO definitions:"
rg -l "DialogTransmissionDto|DialogAttachmentDto"
# Search for usages of ActorDto in relation to these DTOs
echo -e "\nChecking relationships with ActorDto:"
rg -A 5 "DialogTransmissionDto|DialogAttachmentDto|ActorDto" --type cs
# Search specifically in Domain project
echo -e "\nChecking Domain models:"
fd "Dialog.*\.cs$" src/Digdir.Domain.Dialogporten
Length of output: 63500
src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Common/Actors/MappingProfile.cs (2)
33-41: Consider validating ActorId mapping.
The explicit mapping of ActorId suggests it might have special handling requirements. Consider adding validation to ensure the ActorId meets any domain constraints during mapping.
23-31: Verify the ActorType mapping behavior.
The mapping configuration ignores ActorType but maps ActorTypeId from ActorType. This suggests they might represent the same concept differently in the DTO vs domain model. Please ensure this mapping aligns with the domain model's invariants.
✅ Verification successful
Let me try a different search pattern to find these classes and their properties.
Based on the search results, I can now provide a definitive response about the ActorType mapping behavior:
The ActorType mapping configuration is correct and follows the domain model design.
The mapping configuration aligns with the domain model where:
-
Actorentity has two related properties:ActorType(navigation property of typeActorType)ActorTypeId(foreign key of typeActorType.Values)
-
ActorDtohas a single property:ActorTypeof typeActorType.Values
The mapping configuration:
- Ignores the navigation property
ActorTypeduring DTO-to-Entity mapping since it's managed by EF Core - Maps the DTO's
ActorTypeto the entity'sActorTypeIdwhich is the correct foreign key field
This is a standard Entity Framework Core pattern for handling foreign key relationships in domain models.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for ActorType usage in domain model
echo "Searching for ActorType property in Actor class:"
ast-grep --pattern 'class Actor { $$$ ActorType $$$ }'
echo -e "\nSearching for ActorTypeId property in Actor class:"
ast-grep --pattern 'class Actor { $$$ ActorTypeId $$$ }'
echo -e "\nSearching for ActorType property in ActorDto:"
ast-grep --pattern 'class ActorDto { $$$ ActorType $$$ }'
Length of output: 506
Script:
#!/bin/bash
# Search for Actor and ActorDto class definitions with broader context
echo "Searching for Actor class definition:"
rg "class Actor" -A 20
echo -e "\nSearching for ActorDto class definition:"
rg "class ActorDto" -A 20
echo -e "\nSearching for any files containing ActorType:"
rg "ActorType" --type cs -A 5
Length of output: 159160
src/Digdir.Domain.Dialogporten.Application/Features/V1/EndUser/Dialogs/Queries/Search/MappingProfile.cs (2)
3-3: LGTM: Using directive aligns with actor DTO consolidation.
The addition of the Common.Actors namespace aligns with the PR objective of consolidating actor DTOs into a unified structure.
Line range hint 28-43: Verify actor-related mappings in DialogEntity to IntermediateDialogDto.
With the consolidation of actor DTOs, please verify if any actor-related properties in the DialogEntity need to be mapped to the new unified ActorDto structure.
✅ Verification successful
Based on the comprehensive search results, I can now provide a final response:
Actor mappings are properly configured in the DialogEntity to IntermediateDialogDto transformation
The codebase shows that actor-related properties are already correctly mapped in the unified ActorDto structure:
DialogSeenLogSeenByActoris mapped toActorDtoin both EndUser and ServiceOwner contexts- Actor properties (ActorType, ActorName, ActorId) are consistently mapped across DTOs
- The mapping configurations in MappingProfile.cs files handle all actor-related transformations properly
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for actor-related properties in DialogEntity and IntermediateDialogDto
echo "Checking DialogEntity properties:"
ast-grep --pattern 'class DialogEntity {
$$$
Actor $_ { get; set; }
$$$
}'
echo -e "\nChecking IntermediateDialogDto properties:"
ast-grep --pattern 'class IntermediateDialogDto {
$$$
Actor $_ { get; set; }
$$$
}'
Length of output: 482
Script:
#!/bin/bash
# Let's try a different approach to find actor-related mappings
# Search for DialogEntity class definition and its properties
echo "=== DialogEntity Properties ==="
rg "class DialogEntity" -A 20
# Search for IntermediateDialogDto class definition and its properties
echo -e "\n=== IntermediateDialogDto Properties ==="
rg "class IntermediateDialogDto" -A 20
# Search for actor-related mapping configurations
echo -e "\n=== Actor-related Mappings ==="
rg "CreateMap.*Dialog.*Actor" -A 5
Length of output: 13742
Script:
#!/bin/bash
# Let's search for the base DTO class and actor-related properties
echo "=== DialogDtoBase Properties ==="
rg "class DialogDtoBase" -A 20
# Search for any actor-related properties and mappings
echo -e "\n=== Actor Properties and Mappings ==="
rg "Actor.*\{.*get.*set" -A 2
# Search for all mapping configurations in the MappingProfile
echo -e "\n=== All Mapping Configurations in MappingProfile ==="
rg "CreateMap.*Dialog.*Dto" -A 5
Length of output: 65275
src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Dialogs/Queries/Search/MappingProfile.cs (2)
2-3: LGTM! Import changes align with actor DTO consolidation.
The addition of Common and Common.Actors namespaces properly supports the consolidated ActorDto approach.
Line range hint 1-35: Verify the actor mapping configuration.
The removal of actor-specific mappings (DialogSeenLogSeenByActorDto and DialogActivityPerformedByActorDto) aligns with the PR objective. However, we should verify that the consolidated ActorDto mapping is properly configured elsewhere.
Let's verify the actor mapping configuration:
✅ Verification successful
Let me gather more information about the specific actor mapping configurations and their usage:
Let me gather one final piece of information about the ActorDto structure:
Actor mapping configuration is properly consolidated and verified
The actor mapping configuration has been successfully consolidated:
- The
ActorDtostructure matches theActorentity with all necessary properties (ActorType, ActorName, ActorId) - Proper mappings are configured in both EndUser and ServiceOwner contexts:
src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Common/Actors/MappingProfile.cssrc/Digdir.Domain.Dialogporten.Application/Features/V1/EndUser/Common/Actors/MappingProfile.cs
- The old DTOs (
DialogSeenLogSeenByActorDtoandDialogActivityPerformedByActorDto) are being phased out, with proper validators in place for the newActorDto
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that ActorDto mapping is properly configured
# Check for any remaining references to old actor DTOs
echo "Checking for old actor DTO references..."
rg -l "DialogSeenLogSeenByActorDto|DialogActivityPerformedByActorDto"
# Look for new ActorDto mapping configurations
echo "Checking for new ActorDto mapping configurations..."
ast-grep --pattern 'CreateMap<$_, ActorDto>()'
# Verify that all actor-related entities have corresponding mappings
echo "Checking for unmapped actor-related entities..."
rg -l "class.*Actor.*Entity"
Length of output: 1823
Script:
#!/bin/bash
# Check the content of files with old DTO references
echo "Content of files with old DTO references:"
rg -A 5 "DialogSeenLogSeenByActorDto|DialogActivityPerformedByActorDto"
# Check the complete actor mapping configurations
echo -e "\nActor mapping configurations:"
for file in $(fd "MappingProfile.cs"); do
echo -e "\n=== $file ==="
rg "CreateMap.*Actor" "$file" -A 3
done
# Check the Actor entity structure
echo -e "\nActor entity structure:"
cat src/Digdir.Domain.Dialogporten.Domain/Actors/Actor.cs
Length of output: 16269
Script:
#!/bin/bash
# Check ActorDto structure and its usage
echo "Checking ActorDto structure:"
rg -A 10 "class ActorDto"
# Check for any unmapped actor-related DTOs
echo -e "\nChecking for other actor-related DTOs:"
fd -e cs | xargs rg "class.*Actor.*Dto"
Length of output: 2954
src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Dialogs/Queries/Get/MappingProfile.cs (2)
2-2: LGTM: Import changes align with actor DTO consolidation.
The new imports correctly reference the consolidated actor DTOs namespaces.
Also applies to: 4-4
24-24: Verify mapping compatibility and removed mappings.
The consolidation to ActorDto aligns with the PR objective. However, please ensure:
- All properties from
DialogSeenLogSeenByActormap correctly toActorDto - The removed mappings for
DialogActivityPerformedByActorandDialogTransmissionSenderActorare properly handled elsewhere
Let's verify the mapping compatibility and usage:
✅ Verification successful
Mapping compatibility verified successfully
The mapping from DialogSeenLogSeenByActor to ActorDto is valid as:
DialogSeenLogSeenByActorinherits fromActorbase class which contains all required properties (ActorId,ActorName,ActorTypeId)- These properties align perfectly with
ActorDtoproperties (ActorId,ActorName,ActorType) - No remaining references to removed DTOs found in mapping configurations
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check property compatibility between old and new DTOs
# and verify if removed mappings are handled elsewhere
# Check for any remaining references to old actor DTOs
echo "Checking for any remaining references to old DTOs..."
rg -l "DialogSeenLogSeenByActorDto|DialogActivityPerformedByActorDto|DialogTransmissionSenderActorDto"
# Look for ActorDto usage in related contexts
echo "Checking ActorDto usage in related contexts..."
rg -A 5 "class ActorDto"
# Find all AutoMapper profiles that might handle these mappings
echo "Checking other mapping profiles..."
fd -e cs -x grep -l "CreateMap.*Actor.*Dto" {}
Length of output: 2474
Script:
#!/bin/bash
# Let's check the properties of DialogSeenLogSeenByActor and compare with ActorDto
echo "Checking DialogSeenLogSeenByActor properties..."
rg -A 10 "class DialogSeenLogSeenByActor"
echo -e "\nChecking mapping configurations in other profiles..."
for file in $(fd -e cs MappingProfile.cs); do
echo -e "\nFile: $file"
rg "CreateMap.*Actor" "$file" -A 2
done
echo -e "\nChecking for any direct usage of DialogSeenLogSeenByActor..."
rg "DialogSeenLogSeenByActor" -A 2
Length of output: 55780
Script:
#!/bin/bash
# Let's check the Actor base class properties and ActorDto properties for mapping compatibility
echo "Checking Actor base class properties..."
rg -A 10 "class Actor\b"
echo -e "\nChecking both ActorDto implementations..."
rg -A 10 "class ActorDto"
echo -e "\nChecking for any remaining mapping configurations for removed DTOs..."
rg "CreateMap.*DialogActivityPerformedByActor|CreateMap.*DialogTransmissionSenderActor" -A 2
Length of output: 3890
src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/DialogTransmissions/Queries/Search/TransmissionDto.cs (2)
1-1: LGTM: Using directives align with actor DTO consolidation.
The new imports correctly reference the consolidated actor DTOs namespaces.
Also applies to: 4-4
51-51: Verify ActorDto compatibility and update documentation.
The change from SenderActorDto to ActorDto aligns with the DTO consolidation objective. However, please ensure:
- The new
ActorDtoincludes all necessary fields from the oldSenderActorDto - Update the XML documentation to reflect any changes in the available sender properties
Let's verify the compatibility:
✅ Verification successful
Based on the gathered information, I can now provide a final response:
Change from SenderActorDto to ActorDto is compatible and well-implemented
The consolidation of SenderActorDto into ActorDto maintains all necessary fields and functionality:
-
The new
ActorDtocontains all required fields:ActorType(same as in SenderActorDto)ActorName(same as in SenderActorDto)ActorId(same as in SenderActorDto)
-
Proper mapping configurations are in place:
- Both ServiceOwner and EndUser contexts have appropriate mappings
- Validators have been updated to use the new ActorDto
- Identity masking is preserved in EndUser context
The change successfully consolidates actor DTOs while maintaining all functionality and validation rules.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for SenderActorDto usage and properties
echo "Searching for old SenderActorDto properties..."
rg -A 5 "class SenderActorDto"
echo "Checking new ActorDto properties..."
rg -A 10 "class ActorDto"
echo "Checking for potential breaking changes in tests..."
rg "SenderActorDto" -g "*.cs"
Length of output: 1826
Script:
#!/bin/bash
# Let's try to find the old SenderActorDto definition using git history
echo "Searching in git history for SenderActorDto..."
git log -S "SenderActorDto" --patch
# Let's also check for any mapping configurations that might show the relationship
echo -e "\nChecking for mapping configurations..."
rg "CreateMap.*SenderActorDto|CreateMap.*ActorDto" -A 5
# Let's check for any tests or usages that might reveal the properties
echo -e "\nChecking for test implementations..."
rg "new SenderActorDto" -A 3 -g "*.cs"
Length of output: 235192
Script:
#!/bin/bash
# Let's check the ActorDto properties and compare with old SenderActorDto
echo "Checking ActorDto properties..."
rg -A 5 "class ActorDto" -g "*.cs"
# Let's check the mapping configurations for ActorDto
echo -e "\nChecking mapping configurations for ActorDto..."
rg "CreateMap.*ActorDto" -A 2 -g "*.cs"
# Let's check for any validator configurations
echo -e "\nChecking validator configurations..."
rg "Validator.*ActorDto" -A 2 -g "*.cs"
Length of output: 9405
src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/DialogTransmissions/Queries/Get/TransmissionDto.cs (2)
1-1: LGTM! Actor-related namespace imports align with DTO consolidation.
The addition of Common.Actors and ServiceOwner.Common.Actors namespaces properly supports the transition to consolidated actor DTOs.
Also applies to: 4-4
51-51: Verify the impact of ActorDto migration on API consumers.
The change from SenderActorDto to ActorDto looks good and aligns with the PR objective of consolidating actor DTOs. However, since this is a public API change, we should ensure it's properly documented and validated.
Let's verify the API changes and their documentation:
✅ Verification successful
Based on the git history and code analysis, I can now provide a definitive response:
The ActorDto consolidation is properly documented and implemented
The change from SenderActorDto to ActorDto is part of a well-executed consolidation of actor DTOs across the codebase. The changes are:
- Properly documented in the OpenAPI specification
- Consistently implemented across all related components (transmissions, activities, seen logs)
- Maintains the same property structure and validation rules
- Reflected in both service owner and end user APIs
The migration was done through a series of commits that:
- Removed different actor DTOs
- Updated all mapping profiles
- Updated all validators
- Maintained API compatibility by preserving the same property structure
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if this change is reflected in API documentation
fd -e md -e yaml -e json "openapi|swagger" | xargs rg -l "SenderActorDto|ActorDto"
# Look for potential breaking changes in tests
rg -A 5 "SenderActorDto"
Length of output: 109
Script:
#!/bin/bash
# Let's try a broader search to understand the DTO changes and their impact
# Search for files containing ActorDto
rg -l "ActorDto"
# Check the implementation and usage of ActorDto
ast-grep --pattern 'class ActorDto { $$$ }'
# Look for any test files related to transmissions
fd -e cs "Transmission.*Tests?"
# Search for API documentation files
fd "api|swagger|openapi"
Length of output: 4766
Script:
#!/bin/bash
# Let's examine the ActorDto implementation and check for breaking changes
# Check the ActorDto implementation
cat src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Common/Actors/ActorDto.cs
# Look for previous SenderActorDto implementations in git history
git log -p --all -S "SenderActorDto"
# Check the test files for transmission-related tests
cat tests/Digdir.Domain.Dialogporten.Application.Integration.Tests/Features/V1/ServiceOwner/Transmissions/Commands/UpdateTransmissionTests.cs
# Examine the verified swagger documentation
cat docs/schema/V1/swagger.verified.json
Length of output: 495139
src/Digdir.Domain.Dialogporten.Application/Features/V1/EndUser/DialogTransmissions/Queries/Get/TransmissionDto.cs (2)
1-1: LGTM: Using directive changes align with actor DTO consolidation.
The new imports are necessary and properly scoped for the ActorDto changes.
Also applies to: 4-4
57-57: Verify API compatibility for Sender property type change.
The change from SenderActorDto to ActorDto aligns with the PR objective to consolidate actor DTOs. However, this is a breaking change that requires careful consideration.
Let's verify the impact on API consumers:
✅ Verification successful
Based on the search results, I can now generate the final response:
Type change from SenderActorDto to ActorDto appears safe to proceed
The search results show that:
- No direct references to
SenderActorDtowere found in the codebase TransmissionDtois used consistently across both EndUser and ServiceOwner features- The change is part of a broader DTO consolidation effort
- All usages are internal to the application layer and properly mapped
The type change appears to be an internal refactoring with no impact on external API consumers, as the property maintains its contract through proper mapping profiles present in both EndUser and ServiceOwner features.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for potential API consumers that might be affected by the breaking change
# Search for references to the old SenderActorDto
echo "Checking for direct SenderActorDto usage..."
rg -l "SenderActorDto"
# Search for Sender property access patterns
echo "Checking for Sender property access patterns..."
rg -A 3 "\.Sender\."
# Look for API documentation or OpenAPI specs
echo "Checking for API documentation updates..."
fd -e yaml -e json "openapi|swagger" --exec grep -l "SenderActorDto" {} \;
Length of output: 1026
Script:
#!/bin/bash
# Let's perform a more comprehensive check
# Check for any references to TransmissionDto and its usage patterns
echo "Checking TransmissionDto usage..."
rg -l "TransmissionDto"
# Look for Sender property usage in a broader context
echo "Checking Sender property usage with more context..."
rg -B 2 -A 2 "Sender\s*{"
# Check for any API controllers or endpoints that might use TransmissionDto
echo "Checking API controllers..."
rg -l "TransmissionDto.*Controller"
# Check for any test files that might be affected
echo "Checking test files..."
fd -e cs -p "*Test*.cs" --exec rg -l "TransmissionDto" {} \;
# Look for migration files that might contain related changes
echo "Checking migration files..."
fd -e cs -p "*Migration*.cs" --exec rg -l "Sender" {} \;
Length of output: 4805
src/Digdir.Domain.Dialogporten.Application/Features/V1/EndUser/DialogTransmissions/Queries/Search/TransmissionDto.cs (3)
1-1: LGTM: Using directives are properly organized
The new actor-related namespace imports align with the PR objective of consolidating actor DTOs.
Also applies to: 4-4
57-57: Update XML documentation to reflect the new ActorDto type
While the type change from SenderActorDto to ActorDto aligns with the PR objectives, the XML documentation should be updated to reflect this change and any new properties or behaviors introduced by ActorDto.
Let's verify the impact of this change:
Line range hint 1-150: Address missing tests and documentation
While the code changes look good and align with the PR objectives of consolidating actor DTOs, there are two important items from the PR checklist that need attention:
- Tests: Add unit tests to verify the proper mapping and behavior of the new
ActorDtoin the transmission context. - Documentation: Complete the documentation updates to reflect these DTO changes, including:
- XML documentation updates
- API documentation/OpenAPI schema updates
- Migration guide for clients using the old
SenderActorDto
Let's check the current test coverage:
Would you like help with:
- Generating unit tests for the DTO changes?
- Creating a migration guide for clients?
tests/Digdir.Domain.Dialogporten.Application.Integration.Tests/Features/V1/ServiceOwner/Dialogs/Commands/UpdateDialogTests.cs (2)
1-1: LGTM: Import change aligns with DTO consolidation
The new import for Common.Actors reflects the architectural change to use consolidated actor DTOs.
Line range hint 35-39: Consider adding positive test cases for the new ActorDto structure
While the negative test case is good, consider adding tests to verify:
- Valid ActorDto creation with all required properties
- Proper mapping between old and new DTO structures
- Validation of ActorDto properties
Let's check for existing positive test cases:
src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Dialogs/Queries/Search/DialogDtoBase.cs (2)
189-189: LGTM! Verify mapping configuration for PerformedBy property.
The change from DialogActivityPerformedByActorDto to ActorDto aligns with the PR objective of consolidating actor DTOs.
Let's verify the mapping configuration:
✅ Verification successful
Based on the search results, I can now generate a final response. The code shows that:
- The
DialogActivityentity has aPerformedByproperty of typeDialogActivityPerformedByActorwhich inherits fromActor - There are proper mapping configurations for
ActortoActorDtoin both ServiceOwner and EndUser contexts - The mapping configurations handle all necessary properties including
ActorTypeandActorId
The mapping configuration for PerformedBy property is properly handled through the Actor mapping configurations. No issues found.
The change from DialogActivityPerformedByActorDto to ActorDto is correctly supported by the existing mapping profiles, particularly in:
src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Common/Actors/MappingProfile.cssrc/Digdir.Domain.Dialogporten.Application/Features/V1/EndUser/Common/Actors/MappingProfile.cs
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for mapping profiles that handle DialogActivityDto
ast-grep --pattern 'CreateMap<$_, DialogActivityDto>()
$$$
.ForMember(d => d.PerformedBy, $$$)
$$$'
Length of output: 107
Script:
#!/bin/bash
# Let's try a broader search for mapping configurations related to PerformedBy
rg -l "PerformedBy" --type cs
# Then search for any AutoMapper profile configurations
rg "CreateMap.*Dialog.*Activity" --type cs -A 5
Length of output: 16223
Script:
#!/bin/bash
# Let's search for Actor mapping configurations
rg "CreateMap.*Actor" --type cs -A 5
# And also check the DialogActivity entity to understand the PerformedBy property
cat src/Digdir.Domain.Dialogporten.Domain/Dialogs/Entities/Activities/DialogActivity.cs
Length of output: 9593
141-141: LGTM! Verify mapping configuration for SeenBy property.
The change from DialogSeenLogSeenByActorDto to ActorDto aligns with the PR objective of consolidating actor DTOs.
Let's verify the mapping configuration:
✅ Verification successful
Based on the gathered information, I can now generate the final response:
Mapping configuration for SeenBy property is properly handled
The change from DialogSeenLogSeenByActorDto to ActorDto is properly supported by the mapping infrastructure:
- The domain model
DialogSeenLoghas aSeenByproperty of typeDialogSeenLogSeenByActorwhich inherits fromActor - The mapping profile includes
CreateMap<DialogSeenLog, DialogSeenLogDto>() - The base actor mapping
CreateMap<Actor, ActorDto>()is configured in the common actors mapping profile - The inheritance-based mapping ensures
DialogSeenLogSeenByActoris correctly mapped toActorDto
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for mapping profiles that handle DialogSeenLogDto
ast-grep --pattern 'CreateMap<$_, DialogSeenLogDto>()
$$$
.ForMember(d => d.SeenBy, $$$)
$$$'
Length of output: 101
Script:
#!/bin/bash
# Let's try a broader search for mapping configurations related to DialogSeenLogDto
rg -l "DialogSeenLogDto" --type cs
# Then search for any AutoMapper profile classes
ast-grep --pattern 'class $_ : Profile
{
$$$
}'
Length of output: 1271
Script:
#!/bin/bash
# Let's check the mapping configurations in the relevant MappingProfile files
rg -A 10 "CreateMap.*DialogSeenLogDto" src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Dialogs/Queries/Search/MappingProfile.cs
# Also check the source type that's being mapped from
rg -A 2 "class DialogSeenLog" --type cs
Length of output: 9593
Script:
#!/bin/bash
# Let's check the specific mapping configuration in the ServiceOwner Search MappingProfile
cat src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Dialogs/Queries/Search/MappingProfile.cs
# And check the domain model for DialogSeenLog
rg "class DialogSeenLog" -A 10 --type cs
Length of output: 9876
src/Digdir.Domain.Dialogporten.Application/Features/V1/EndUser/Dialogs/Queries/Search/DialogDtoBase.cs (2)
129-129: LGTM! Property types updated consistently
The changes to use ActorDto for both SeenBy and PerformedBy properties align well with the PR objective of consolidating actor DTOs.
Also applies to: 176-176
Line range hint 185-206: Remove unused DialogActivityPerformedByActorDto class
This class appears to be unused after transitioning to the common ActorDto. It should be removed as part of the actor DTO consolidation effort.
✅ Verification successful
Remove unused DialogActivityPerformedByActorDto class
The class can be safely removed. The verification confirms:
- The class is only referenced in an unused import in MappingProfile.cs
- No active usages found in method parameters, return types, inheritance, or instantiations
- The functionality has been replaced by
ActorDtoas seen inDialogActivityDto.PerformedBy
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify if the class is used anywhere in the codebase
rg -l "DialogActivityPerformedByActorDto"
# Check for any remaining references to this type
ast-grep --pattern 'class $_ {
$$$
DialogActivityPerformedByActorDto $_
$$$
}'
Length of output: 303
Script:
#!/bin/bash
# Check how DialogActivityPerformedByActorDto is used in MappingProfile.cs
rg -A 5 -B 5 "DialogActivityPerformedByActorDto" src/Digdir.Domain.Dialogporten.GraphQL/EndUser/Common/MappingProfile.cs
# Check if there are any references to this type in method parameters or return types
ast-grep --pattern '($_) DialogActivityPerformedByActorDto $_ {
$$$
}'
# Check if there are any mappings or conversions using this type
ast-grep --pattern 'CreateMap<DialogActivityPerformedByActorDto, $_>($$$)'
ast-grep --pattern 'CreateMap<$_, DialogActivityPerformedByActorDto>($$$)'
Length of output: 1172
Script:
#!/bin/bash
# Let's check the content of the file to see the actual class definition and any usages
cat src/Digdir.Domain.Dialogporten.Application/Features/V1/EndUser/Dialogs/Queries/Search/DialogDtoBase.cs
# Check for any references in method calls or instantiations
rg -A 2 "new DialogActivityPerformedByActorDto"
rg -A 2 "DialogActivityPerformedByActorDto\."
# Check if it's used in any type definitions or inheritance
ast-grep --pattern 'class $_ : DialogActivityPerformedByActorDto'
ast-grep --pattern 'class $_ : $_ where $_ : DialogActivityPerformedByActorDto'
Length of output: 7567
src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/ServiceOwner/Dialogs/Update/UpdateDialogSwaggerConfig.cs (2)
2-2: LGTM: Using directive additions align with DTO consolidation.
The new imports support the transition to unified actor DTOs, maintaining a clear separation between common and service-owner specific actor types.
Also applies to: 4-4
153-156: Verify consistency of ActorDto examples across API documentation.
The ActorDto example is minimal but appropriate for this context. However, we should ensure consistent representation across all documentation.
✅ Verification successful
Let me gather more information about the ActorDto structure to ensure we have a complete picture.
Let me try a different approach to find the ActorDto definition and usage patterns.
Based on the comprehensive search results, I can now provide a final response:
ActorDto example is consistent with codebase usage patterns
The minimal ActorDto example with ServiceOwner type in the swagger config is appropriate and consistent with the codebase patterns:
- The ActorDto definition shows only three properties: ActorType, ActorName, and ActorId
- ServiceOwner type is correctly used in the context of service owner endpoints
- Other examples in the codebase (tests, fake data generator) follow the same pattern
- Validators enforce the ActorType enum value, making the example valid
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other ActorDto examples in swagger configs to verify consistency
rg -l "new ActorDto" | while read -r file; do
echo "=== $file ==="
rg -A 5 "new ActorDto" "$file"
done
Length of output: 1897
Script:
#!/bin/bash
# Search for ActorDto definition and its properties
ast-grep --pattern 'class ActorDto {
$$$
}'
# Also check for any other swagger configs that might use ActorDto
rg -l "SwaggerConfig.*ActorDto"
Length of output: 81
Script:
#!/bin/bash
# Find all files containing ActorDto to understand its structure and usage
rg -A 5 "ActorDto" --type cs
# Specifically look for any property definitions
rg "public.*ActorDto" --type cs
Length of output: 46582
src/Digdir.Tool.Dialogporten.GenerateFakeData/DialogGenerator.cs (2)
3-3: LGTM: Using directives align with actor DTO consolidation
The new using directives appropriately support the transition to common actor DTOs.
Also applies to: 5-5
270-270: Verify complete migration of actor DTOs
Let's ensure all actor DTOs have been migrated to the new common DTOs across the codebase.
src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Dialogs/Commands/Update/UpdateDialogCommandValidator.cs (2)
205-207: LGTM! Constructor changes align with actor DTO consolidation.
The parameter change from specific actor validator to generic ActorDto validator is consistent with the PR's objective of unifying actor DTOs.
398-398: LGTM! Constructor parameter change maintains consistency.
The change to use ActorDto validator maintains consistency with the unified actor DTO approach.
src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Dialogs/Commands/Update/UpdateDialogDto.cs (3)
1-4: LGTM: Using directive changes align with actor DTO consolidation
The addition of actor-related namespaces properly supports the consolidation of actor DTOs into common types.
149-149: Verify TransmissionDto.Sender property type change impact
The change to use ActorDto aligns with the actor DTO consolidation objective.
Let's verify all TransmissionDto usages have been updated:
✅ Verification successful
Let me gather more information to verify the impact of the Sender property type change.
Based on the verification results, I can now generate the final response:
TransmissionDto.Sender property type change is correctly implemented
The codebase scan shows that all usages of the Sender property have been updated to use the new ActorDto type. The only usage found in tests demonstrates correct initialization with the expected ActorDto structure:
Sender = new() { ActorType = ActorType.Values.ServiceOwner }🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for TransmissionDto usage to verify Sender property type compatibility
rg -l "TransmissionDto" | xargs rg "Sender\s*=" -A 2 -B 2
Length of output: 1718
Script:
#!/bin/bash
# Search for all usages of TransmissionDto to verify Sender property assignments and usage
ast-grep --pattern 'new TransmissionDto { $$$ }'
# Also search for direct property assignments
rg "\.Sender\s*=" --type csharp -B 2 -A 2
Length of output: 2008
256-256: Verify ActivityDto.PerformedBy property type change impact
The change to use ActorDto aligns with the actor DTO consolidation objective.
Let's verify all ActivityDto usages have been updated:
✅ Verification successful
Let me gather more information to verify if there are any other usages of ActivityDto and its PerformedBy property.
Let me try a different approach to find all ActivityDto usages and mappings.
Based on the comprehensive search results, I can now generate the final response:
ActivityDto.PerformedBy property type change is properly implemented
The change to use ActorDto for the PerformedBy property is consistently implemented across the codebase:
- All ActivityDto usages have been updated with proper mappings in place
- Both ServiceOwner and EndUser features use the same ActorDto type
- Validators (CreateDialogDialogActivityDtoValidator and UpdateDialogDialogActivityDtoValidator) properly handle ActorDto validation
- AutoMapper profiles correctly map the PerformedBy property between DialogActivity and ActivityDto
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for ActivityDto usage to verify PerformedBy property type compatibility
rg -l "ActivityDto" | xargs rg "PerformedBy\s*=" -A 2 -B 2
Length of output: 1718
Script:
#!/bin/bash
# Search for ActivityDto class definition and its usages
ast-grep --pattern 'class ActivityDto { $$$ }'
# Search for any assignments or mappings to ActivityDto.PerformedBy
rg "ActivityDto.*PerformedBy" -B 2 -A 2
Length of output: 90
Script:
#!/bin/bash
# Find the ActivityDto definition
fd "ActivityDto.cs"
# Search for any mappings or assignments involving ActivityDto
rg "ActivityDto" -B 3 -A 3
# Search specifically for AutoMapper profiles that might handle ActivityDto
rg "CreateMap.*ActivityDto"
Length of output: 61453
src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Dialogs/Commands/Create/CreateDialogDto.cs (2)
1-4: LGTM! Well-structured namespace organization.
The new actor-related namespace structure properly separates common and service-owner specific actors while maintaining API versioning. This aligns well with the PR objective of consolidating actor DTOs.
309-309: Consider enhancing the XML documentation for the PerformedBy property.
While the change to use the consolidated ActorDto is good, consider enhancing the XML documentation to describe what types of actors can perform activities and any validation rules that apply.
Let's verify the impact of this actor DTO consolidation:
src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Dialogs/Commands/Create/CreateDialogCommandValidator.cs (3)
8-8: LGTM: Import statement aligns with actor DTO consolidation
The added import for common actors namespace supports the PR objective of consolidating actor DTOs.
154-154: LGTM: Actor validator parameter updated correctly
The parameter type change from TransmissionSenderActorDto to ActorDto aligns with the consolidation effort while maintaining the same validation capabilities.
✅ Verification successful
Based on the search results, I can now make a determination. The codebase shows that:
DialogTransmissionSenderActoris a domain entity that inherits fromActorbase classActorDtois used consistently throughout the application for all actor-related validations- The change from
TransmissionSenderActorDtotoActorDtoaligns with the codebase's architecture where all actor validations use the commonActorDtotype
LGTM: Parameter type change is correct and consistent
The change from IValidator<TransmissionSenderActorDto> to IValidator<ActorDto> is correct as it:
- Aligns with the codebase's pattern of using
ActorDtofor all actor validations - Maintains consistency with the domain model where
DialogTransmissionSenderActoris a specializedActor - Follows the consolidation effort while preserving validation capabilities
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify that TransmissionSenderActorDto is not used elsewhere in the codebase
rg "TransmissionSenderActorDto"
Length of output: 31
Script:
#!/bin/bash
# Let's check the entire codebase for any references to TransmissionSenderActor (without Dto suffix)
# and ActorDto to understand the context better
echo "=== Searching for TransmissionSenderActor references ==="
rg "TransmissionSenderActor" -A 2
echo -e "\n=== Searching for ActorDto references ==="
rg "ActorDto" -A 2
# Also check for validator implementations
echo -e "\n=== Searching for validator implementations ==="
ast-grep --pattern 'class $_ : AbstractValidator<$_> {
$$$
}'
Length of output: 64508
413-413: LGTM: Activity actor validator parameter updated correctly
The parameter type change from ActivityPerformedByActorDto to ActorDto aligns with the consolidation effort while maintaining the same validation capabilities.
✅ Verification successful
Parameter change from ActivityPerformedByActorDto to ActorDto is safe
The change aligns with the codebase's consolidation efforts. The DialogActivityPerformedByActorDto class is only used in GraphQL mapping and search queries, while ActorDto is consistently used throughout the application for actor validation. The validator maintains the same validation capabilities through the base ActorDto type.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify that ActivityPerformedByActorDto is not used elsewhere in the codebase
rg "ActivityPerformedByActorDto"
Length of output: 420
Script:
#!/bin/bash
# Check if DialogActivityPerformedByActorDto is still being used in any validation or mapping logic
ast-grep --pattern 'class $_ : IValidator<DialogActivityPerformedByActorDto>'
# Check for any references to this type in method signatures or property declarations
rg "DialogActivityPerformedByActorDto" -A 2
# Check for any usages of ActorDto to ensure proper migration
ast-grep --pattern 'class $_ : IValidator<ActorDto>'
Length of output: 64508
src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Dialogs/Queries/Get/DialogDto.cs (3)
231-231: LGTM: Sender property updated to use consolidated ActorDto
The change aligns with the PR objective of consolidating actor DTOs while maintaining proper null safety and documentation.
259-259: LGTM: SeenBy property updated to use consolidated ActorDto
The change aligns with the PR objective of consolidating actor DTOs while maintaining proper null safety and documentation.
368-368: LGTM: PerformedBy property updated to use consolidated ActorDto
The change aligns with the PR objective of consolidating actor DTOs while maintaining proper null safety and documentation.
docs/schema/V1/swagger.verified.json (2)
343-359: Well-structured common actor schemas!
The new V1EndUserCommonActors_Actor and V1ServiceOwnerCommonActors_Actor schemas are well-defined with:
- Clear and consistent property definitions
- Appropriate nullability settings
- Proper enum reference for actorType
- Prevention of unknown properties
Also applies to: 1790-1806
384-384: Verify comprehensive actor schema reference updates.
The schema references have been systematically updated to use the new common actor schemas. Let's verify all references have been updated correctly.
Also applies to: 442-442, 465-465, 488-488, 742-742, 1002-1002, 1063-1063, 1348-1348, 1395-1395, 1562-1562, 1723-1723, 3045-3045, 3159-3159, 3180-3180, 3450-3450, 3710-3710, 3772-3772, 4079-4079, 4126-4126, 4194-4194, 4351-4351, 4508-4508
✅ Verification successful
Actor schema references have been correctly updated
The verification confirms:
- Both
V1EndUserCommonActors_ActorandV1ServiceOwnerCommonActors_Actorschemas are properly defined - All actor references (performedBy, seenBy, sender) consistently use the new common actor schemas
- No traces of old DTO references were found
- The schema updates maintain proper context separation between end-user and service owner actors
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify all references to actor schemas are using the new common schemas
# Test: Search for any remaining references to old actor DTOs
echo "Searching for potential missed actor DTO references..."
rg --type json \
-e 'PerformedByActorDto' \
-e 'SenderActorDto' \
-e 'SeenByActorDto' \
-e 'LabelAssignmentLogActorDto' \
-e 'ActivityPerformedByActorDto'
# Test: Verify all actor references point to the new common schemas
echo "Verifying actor references use new common schemas..."
rg --type json \
-e '"\\$ref": "#/components/schemas/V1EndUserCommonActors_Actor"' \
-e '"\\$ref": "#/components/schemas/V1ServiceOwnerCommonActors_Actor"'
Length of output: 524
Script:
#!/bin/bash
# Let's try a different approach to verify the actor schema references
# First, let's find all JSON files in the docs/schema directory
echo "=== JSON files in schema directory ==="
fd -e json . docs/schema
# Now let's look for any $ref patterns in the swagger.verified.json
echo -e "\n=== All actor-related $ref patterns ==="
rg -A 1 '"performedBy"|"seenBy"|"sender"' docs/schema/V1/swagger.verified.json
# Let's also check the schema definitions to ensure new actor schemas exist
echo -e "\n=== Actor schema definitions ==="
rg -A 5 '"V1EndUserCommonActors_Actor"|"V1ServiceOwnerCommonActors_Actor"' docs/schema/V1/swagger.verified.json
Length of output: 3611
src/Digdir.Domain.Dialogporten.Application/Features/V1/EndUser/Common/Actors/ActorDto.cs
Outdated
Show resolved
Hide resolved
src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Common/Actors/ActorDto.cs
Show resolved
Hide resolved
src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Common/Actors/ActorDto.cs
Outdated
Show resolved
Hide resolved
...Domain.Dialogporten.Application/Features/V1/EndUser/DialogSeenLogs/Queries/Get/SeenLogDto.cs
Outdated
Show resolved
Hide resolved
...ain.Dialogporten.Application/Features/V1/EndUser/DialogSeenLogs/Queries/Search/SeenLogDto.cs
Show resolved
Hide resolved
src/Digdir.Domain.Dialogporten.GraphQL/EndUser/Common/MappingProfile.cs
Outdated
Show resolved
Hide resolved
....Domain.Dialogporten.Application/Features/V1/EndUser/Dialogs/Queries/Search/DialogDtoBase.cs
Outdated
Show resolved
Hide resolved
src/Digdir.Domain.Dialogporten.Application/Features/V1/EndUser/Dialogs/Queries/Get/DialogDto.cs
Show resolved
Hide resolved
...ir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Dialogs/Queries/Get/DialogDto.cs
Show resolved
Hide resolved
src/Digdir.Domain.Dialogporten.Application/Features/V1/EndUser/Common/Actors/ActorDto.cs
Show resolved
Hide resolved
...ten.Application.Integration.Tests/Features/V1/ServiceOwner/Dialogs/Queries/GetDialogTests.cs
Outdated
Show resolved
Hide resolved
...Application/Features/V1/ServiceOwner/Dialogs/Commands/Create/CreateDialogCommandValidator.cs
Outdated
Show resolved
Hide resolved
...in.Dialogporten.WebApi/Endpoints/V1/ServiceOwner/Dialogs/Update/UpdateDialogSwaggerConfig.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (4)
src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Common/Actors/ActorValidator.cs (2)
8-11: Add XML documentation for the validator class.Consider adding XML documentation to describe the purpose and validation rules of this class, especially since it's implementing important business rules.
+/// <summary> +/// Validates the ActorDto ensuring proper actor identification and type constraints. +/// </summary> public sealed class ActorValidator : AbstractValidator<ActorDto>
13-14: Consider adding a custom error message for ActorType validation.The current validation will use a default message. A custom message would provide clearer guidance to API consumers.
RuleFor(x => x.ActorType) - .IsInEnum(); + .IsInEnum() + .WithMessage("ActorType must be a valid actor type value.");src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Dialogs/Commands/Update/UpdateDialogCommandValidator.cs (2)
377-377: Consider adding activity-specific actor validation rulesWhile the change to use the unified ActorDto validator is correct, consider adding activity-specific validation rules for the PerformedBy actor. For example, certain activity types might require specific actor properties or have restrictions on actor types.
Example validation rule to consider:
RuleFor(x => x.PerformedBy) .NotNull() - .SetValidator(actorValidator); + .SetValidator(actorValidator) + .Must((activity, actor) => + activity.Type != DialogActivityType.Values.Information || + actor.Type == ActorType.Values.ServiceOwner) + .WithMessage("Information activities must be performed by a service owner.");
Based on the search results from multiple attempts, we can see that there are no dedicated validator test files, and the integration tests don't appear to cover the specific validation rules. Let me generate the final response:
Add unit tests for validator classes
The validator changes need dedicated unit tests to verify the validation rules. Please add:
- Unit tests for
UpdateDialogCommandValidatorand its related validator classes- Specific test cases for ActorDto validation in:
UpdateDialogDialogActivityDtoValidator(PerformedBy property)UpdateDialogDialogTransmissionDtoValidator(Sender property)- Test cases for validation rules around:
- Activity type-specific validation
- Transmission sender validation
- GUI/API action validation
🔗 Analysis chain
Line range hint
1-411: Add tests for the updated validator classesThe PR objectives indicate that automated tests haven't been added. Please add tests to verify:
- Validation behavior with the unified ActorDto
- Activity-specific actor validation rules
- Transmission sender validation rules
Let's verify existing test coverage:
Would you like help creating the test cases for these validator changes?
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for existing validator tests and identify gaps # Search for test files related to these validators fd -e cs -e fs "UpdateDialogCommandValidator.*Test" # Search for existing actor validation tests rg -l "ActorDto.*Validator.*Test"Length of output: 89
Script:
#!/bin/bash # Let's try a broader search for test files related to these validators and actors # Search for any test files in the test project that might contain validator tests fd -e cs "Test" tests/ # Search for any validator tests rg -l "Validator.*Test" # Search for any actor-related tests rg -l "Actor.*Test" # Search specifically for dialog-related tests rg -l "Dialog.*Test"Length of output: 8318
Script:
#!/bin/bash # Let's check the content of UpdateDialogTests to see if it includes validator tests # Check UpdateDialogTests for validator testing rg -A 10 "UpdateDialog.*Validator" "tests/Digdir.Domain.Dialogporten.Application.Integration.Tests/Features/V1/ServiceOwner/Dialogs/Commands/UpdateDialogTests.cs" # Check for any validator-specific test files rg -l ".*Validator.*Test" tests/ # Look for actor validation tests in integration tests rg -A 5 "Actor.*Valid" "tests/Digdir.Domain.Dialogporten.Application.Integration.Tests/Features/V1/ServiceOwner/Dialogs/Commands/UpdateDialogTests.cs"Length of output: 348
Script:
#!/bin/bash # Let's try a different approach to find validator tests # Search for test files containing validation-related test methods rg -l "Should.*Valid|Invalid.*Test|Test.*Validation" tests/ # Look for specific test cases related to dialog validation rg -B 2 -A 5 "\[Fact\].*\n.*Valid" "tests/Digdir.Domain.Dialogporten.Application.Integration.Tests/Features/V1/ServiceOwner/Dialogs/Commands/UpdateDialogTests.cs" # Check if there are any test classes for validators rg -l "class.*Validator.*Tests?" tests/Length of output: 547
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (4)
src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Common/Actors/ActorValidator.cs(1 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/Update/UpdateDialogCommandValidator.cs(3 hunks)src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/ServiceOwner/Dialogs/Update/UpdateDialogSwaggerConfig.cs(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Dialogs/Commands/Create/CreateDialogCommandValidator.cs
- src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/ServiceOwner/Dialogs/Update/UpdateDialogSwaggerConfig.cs
🔇 Additional comments (5)
src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Common/Actors/ActorValidator.cs (3)
1-6: LGTM! Imports and namespace are well-organized.
16-20: 🛠️ Refactor suggestion
Improve readability of complex validation logic.
The current validation logic is hard to follow. Consider breaking it down into smaller, well-named methods.
RuleFor(x => x)
- .Must(dto => (dto.ActorId is null || dto.ActorName is null) &&
- ((dto.ActorType == ActorType.Values.ServiceOwner && dto.ActorId is null && dto.ActorName is null) ||
- (dto.ActorType != ActorType.Values.ServiceOwner && (dto.ActorId is not null || dto.ActorName is not null))))
+ .Must(HasValidActorIdentification)
.WithMessage(ActorValidationErrorMessages.ActorIdActorNameExclusiveOr);
+/// <summary>
+/// Validates that actor identification follows business rules:
+/// 1. ActorId and ActorName are mutually exclusive
+/// 2. ServiceOwner type must have neither ActorId nor ActorName
+/// 3. Non-ServiceOwner types must have either ActorId or ActorName
+/// </summary>
+private static bool HasValidActorIdentification(ActorDto dto)
+{
+ var isIdNameMutuallyExclusive = dto.ActorId is null || dto.ActorName is null;
+
+ if (dto.ActorType == ActorType.Values.ServiceOwner)
+ return isIdNameMutuallyExclusive && dto.ActorId is null && dto.ActorName is null;
+
+ return isIdNameMutuallyExclusive && (dto.ActorId is not null || dto.ActorName is not null);
+}Let's verify the error message constant exists:
✅ Verification successful
Let me verify the complete context of the error message constant by checking its class definition.
Validation logic and error message are correctly aligned
The error message constant exists and accurately describes the complex business rules being validated:
- For ServiceOwner: both ActorId and ActorName must be null
- For other types: either ActorId or ActorName must be set (but not both)
The suggested refactoring to improve readability remains valid, as it would make the code more maintainable while preserving the exact same validation logic.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for ActorValidationErrorMessages class and the specific constant
rg -l "class ActorValidationErrorMessages" | xargs rg "ActorIdActorNameExclusiveOr"
Length of output: 221
Script:
#!/bin/bash
# Get the complete context of ActorValidationErrorMessages class
rg -A 5 "class ActorValidationErrorMessages"
Length of output: 902
22-24: Verify the party identifier validation implementation.
The code uses a custom IsValidPartyIdentifier() extension method. Let's ensure it implements all necessary validation rules for party identifiers.
✅ Verification successful
Let's search with broader patterns to locate the validation implementation and related files.
Let's examine the actual validation implementation and the party identifier model to understand the validation rules.
Let's examine the specific implementations to understand the validation rules for Norwegian organization and person identifiers, as these are the only accepted types according to the validation extension.
Party identifier validation implementation is comprehensive and correct
The validation implementation is well-structured and enforces:
- Valid format with prefix and separator (urn:altinn:organization:identifier-no: or urn:altinn:person:identifier-no:)
- For Norwegian organization numbers:
- 9-digit format with Mod11 checksum validation
- For Norwegian person identifiers (F-number/D-number):
- 11-digit format with dual Mod11 checksum validation
- Null values are allowed (as per the When condition in the original validator)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for IsValidPartyIdentifier implementation
ast-grep --pattern 'public static IRuleBuilderOptions<$_, string> IsValidPartyIdentifier($_)'
# Search for any tests related to party identifier validation
rg -l "IsValidPartyIdentifier.*Test"
Length of output: 134
Script:
#!/bin/bash
# Try different pattern to find IsValidPartyIdentifier implementation
rg -A 5 "IsValidPartyIdentifier"
# Search for validator extensions
fd "Validator" --exec rg -l "IRuleBuilder.*Extension"
# Search for party identifier related files
fd -e cs -e fs | rg -i "party|identifier"
Length of output: 5212
Script:
#!/bin/bash
# Let's examine the party identifier validation implementation
cat src/Digdir.Domain.Dialogporten.Application/Common/Extensions/FluentValidation/FluentValidationPartyIdentifierExtensions.cs
# Check the PartyIdentifier base implementation
cat src/Digdir.Domain.Dialogporten.Domain/Parties/Abstractions/PartyIdentifier.cs
Length of output: 6005
Script:
#!/bin/bash
# Let's check the specific implementations for Norwegian identifiers
cat src/Digdir.Domain.Dialogporten.Domain/Parties/NorwegianOrganizationIdentifier.cs
cat src/Digdir.Domain.Dialogporten.Domain/Parties/NorwegianPersonIdentifier.cs
Length of output: 3668
src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Dialogs/Commands/Update/UpdateDialogCommandValidator.cs (2)
7-7: LGTM: Import added for unified actor DTO
The import aligns with the PR objective of consolidating actor DTOs and follows the proper namespace organization.
184-184: LGTM: Updated to use unified actor validator
The change correctly implements the unified ActorDto validator for transmission sender validation, maintaining existing validation rules while reducing DTO complexity.
...dir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Common/Actors/ActorValidator.cs
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (3)
tests/Digdir.Domain.Dialogporten.Application.Unit.Tests/Features/V1/ServiceOwner/Common/ActorValidatorTest.cs (3)
1-10: Add XML documentation for the test class.While the class setup is good, consider adding XML documentation to describe the purpose and scope of these tests, especially given that this is testing a new consolidated actor validation approach.
namespace Digdir.Domain.Dialogporten.Application.Unit.Tests.Features.V1.ServiceOwner.Common; +/// <summary> +/// Tests for validating the consolidated ActorValidator implementation, +/// which handles validation rules for the unified actor DTOs. +/// </summary> public class ActorValidatorTest
11-22: Enhance error validation assertions.The test only verifies that errors exist but doesn't validate the specific error message or count. Consider enhancing the assertions to verify the exact validation failure.
var result = _actorValidator.Validate(actorDto); - Assert.NotEmpty(result.Errors); + Assert.Collection(result.Errors, + error => { + Assert.Equal("ActorId", error.PropertyName); + Assert.Contains("valid party identifier", error.ErrorMessage); + });
38-50: Extract common test setup to reduce duplication.The ActorDto creation is duplicated across tests. Consider extracting a helper method to create test data.
+ private static ActorDto CreateTestActorDto(string? actorId = null, string? actorName = null) + { + return new ActorDto + { + ActorType = ActorType.Values.PartyRepresentative, + ActorName = actorName, + ActorId = actorId ?? DialogGenerator.GenerateRandomParty(forcePerson: true) + }; + } + [Fact] public void GivenActorIdAndActorNameShouldReturnError() { - var actorDto = new ActorDto - { - ActorType = ActorType.Values.PartyRepresentative, - ActorName = "Fredrik Testland", - ActorId = DialogGenerator.GenerateRandomParty(forcePerson: true) - }; + var actorDto = CreateTestActorDto(actorName: "Fredrik Testland");
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
tests/Digdir.Domain.Dialogporten.Application.Unit.Tests/Features/V1/ServiceOwner/Common/ActorValidatorTest.cs(1 hunks)
🔇 Additional comments (1)
tests/Digdir.Domain.Dialogporten.Application.Unit.Tests/Features/V1/ServiceOwner/Common/ActorValidatorTest.cs (1)
24-36: Verify the forcePerson parameter usage.
The test uses forcePerson: true but it's not clear why this is necessary. Consider adding a comment explaining this requirement or verify if it's needed.
...in.Dialogporten.Application.Unit.Tests/Features/V1/ServiceOwner/Common/ActorValidatorTest.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (3)
tests/Digdir.Domain.Dialogporten.Application.Unit.Tests/Features/V1/ServiceOwner/Common/ActorValidatorTest.cs (3)
9-9: Consider using test fixture setup.While initializing the validator in a field is valid, consider using a constructor or test fixture setup method to make the initialization more explicit and maintainable.
- private readonly ActorValidator _actorValidator = new(); + private ActorValidator _actorValidator; + + public ActorValidatorTest() + { + _actorValidator = new ActorValidator(); + }
27-27: Remove unnecessary empty line.Maintain consistent spacing between test method sections.
68-68: Rename test method to follow consistent naming convention.The method name
ActorTypeServiceOwnerRulesdoesn't follow the "Given_When_Then" or "Given_Should" pattern used in other tests.- public void ActorTypeServiceOwnerRules(string? actorName, bool generateActorId) + public void GivenServiceOwnerTypeWithInvalidConfigurationShouldReturnError(string? actorName, bool generateActorId)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
tests/Digdir.Domain.Dialogporten.Application.Unit.Tests/Features/V1/ServiceOwner/Common/ActorValidatorTest.cs(1 hunks)
🔇 Additional comments (1)
tests/Digdir.Domain.Dialogporten.Application.Unit.Tests/Features/V1/ServiceOwner/Common/ActorValidatorTest.cs (1)
7-79: Add test cases for complete validation coverage.
The test suite is missing several important validation scenarios:
- Validation with different ActorType values (Organization, Person)
- Validation with null ActorId when ActorName is provided
- Edge cases for ActorName validation (empty string, whitespace, max length)
The previous review already provided implementation examples for these test cases. Please refer to the earlier comment for the suggested implementations.
...in.Dialogporten.Application.Unit.Tests/Features/V1/ServiceOwner/Common/ActorValidatorTest.cs
Show resolved
Hide resolved
…of WithoutAuditableSideEffects
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
src/Digdir.Domain.Dialogporten.Domain/Dialogs/Entities/DialogEntity.cs (1)
Line range hint
89-115: Consider adding input validation and documentation.The method could benefit from:
- Input validation for
endUserId- XML documentation explaining the method's purpose and parameters
Consider applying these improvements:
+ /// <summary> + /// Updates the seen status for a dialog when viewed by an end user. + /// </summary> + /// <param name="endUserId">The ID of the end user who viewed the dialog</param> + /// <param name="userTypeId">The type of the user viewing the dialog</param> + /// <param name="endUserName">Optional name of the end user</param> + /// <exception cref="ArgumentException">Thrown when endUserId is null or empty</exception> public void UpdateSeenAt(string endUserId, DialogUserType.Values userTypeId, string? endUserName) { + if (string.IsNullOrEmpty(endUserId)) + { + throw new ArgumentException("End user ID cannot be null or empty", nameof(endUserId)); + } + var lastSeenAt = SeenLog
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
src/Digdir.Domain.Dialogporten.Domain/Dialogs/Entities/DialogEntity.cs(2 hunks)
🔇 Additional comments (2)
src/Digdir.Domain.Dialogporten.Domain/Dialogs/Entities/DialogEntity.cs (2)
12-12: LGTM: Using directive is required for ID generation.
The addition of this using directive is appropriate as it provides access to the IdentifiableExtensions used for generating Version 7 UUIDs in the UpdateSeenAt method.
111-112: LGTM: Proper timestamp and ID generation.
Good practices implemented:
- Using UTC timestamp for consistent time recording
- Using Version 7 UUIDs which are optimized for database performance
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (3)
src/Digdir.Domain.Dialogporten.Domain/Dialogs/Entities/DialogEntity.cs (2)
Line range hint
101-113: Consider adding input validation.While the log creation logic is solid, consider adding validation for
endUserIdandendUserNameto ensure data integrity.public DialogSeenLog? UpdateSeenAt(string endUserId, DialogUserType.Values userTypeId, string? endUserName) { + if (string.IsNullOrWhiteSpace(endUserId)) + { + throw new ArgumentException("Actor ID cannot be empty", nameof(endUserId)); + } + var lastSeenAt = SeenLog
114-116: Consider implementing log size management.The
SeenLoglist grows unbounded. Consider implementing a cleanup strategy or size limit to prevent potential memory/storage issues.Options to consider:
- Implement a rolling window (e.g., keep last 30 days)
- Keep only the latest N entries per user
- Archive older entries to a separate storage
Would you like me to propose a specific implementation for any of these approaches?
src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Dialogs/Queries/Get/GetDialogQuery.cs (1)
111-115: Consider adding documentation for the seen status tracking logic.The
UpdateSeenAtmethod's behavior and return value implications aren't immediately clear. Consider adding XML documentation to explain when and why it returns null, and how it integrates with the actor DTO consolidation.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (3)
src/Digdir.Domain.Dialogporten.Application/Features/V1/EndUser/Dialogs/Queries/Get/GetDialogQuery.cs(1 hunks)src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Dialogs/Queries/Get/GetDialogQuery.cs(1 hunks)src/Digdir.Domain.Dialogporten.Domain/Dialogs/Entities/DialogEntity.cs(4 hunks)
🔇 Additional comments (6)
src/Digdir.Domain.Dialogporten.Domain/Dialogs/Entities/DialogEntity.cs (2)
12-12: LGTM: Using directive is correctly placed and necessary.
The added import supports the new ID generation functionality used in the UpdateSeenAt method.
Line range hint 88-98: LGTM: Improved method signature with meaningful return type.
The change from void to DialogSeenLog? enhances the API by allowing callers to track if a new log entry was created. The null check logic is clean and maintains the existing behavior.
src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Dialogs/Queries/Get/GetDialogQuery.cs (2)
116-119: LGTM! Null check before adding the log is a good practice.
The conditional check before adding the log entry prevents unnecessary database operations.
111-120: Verify error handling assumptions.
The use of UnreachableException suggests that domain and concurrency errors are impossible in this context. This is a strong assumption that should be verified.
Let's verify the error handling assumptions by checking the UpdateSeenAt implementation and its callers:
src/Digdir.Domain.Dialogporten.Application/Features/V1/EndUser/Dialogs/Queries/Get/GetDialogQuery.cs (2)
123-131: Consider enhancing error handling and validation.
While the null check is good, there are two areas for improvement:
- The TODO comment above suggests potential name lookup failures that should be handled.
- Consider adding validation of the log object's state before adding to DB.
Let's check if there are similar patterns in the codebase:
Line range hint 134-139: Verify unreachable code paths and enhance error messages.
While using UnreachableException is appropriate for truly unreachable code paths, we should:
- Verify that domain and concurrency errors are indeed impossible during
UpdateSeenAt. - Consider adding more descriptive error messages including the dialog ID and operation context.
Let's verify these assumptions:
✅ Verification successful
Based on the code analysis, I can now generate the final response:
The UnreachableException usage for SeenAt updates is correct and safe.
The code shows that UpdateSeenAt is a simple operation that:
- Only creates a new
DialogSeenLogentry when the dialog hasn't been seen since its last update - Maps
SeenAtdirectly toCreatedAtwithout any domain validation - Uses
WithoutAuditableSideEffects()to indicate this is a side-effect-free operation
Since there are no domain validations or constraints on SeenAt updates, it's impossible to get domain errors. The operation is also concurrency-safe as it doesn't depend on or modify any versioned data.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other UpdateSeenAt implementations to verify error handling
ast-grep --pattern 'class $_ {
$$$
UpdateSeenAt($$$) {
$$$
}
$$$
}'
# Search for similar SaveChangesAsync error handling patterns
rg -A 5 'SaveChangesAsync.*Switch'
Length of output: 118
Script:
#!/bin/bash
# Let's try a broader search for the domain model and error handling patterns
# Search for domain model and validation logic
rg -l "SeenAt"
# Search for SaveChangesAsync usage patterns
rg "SaveChangesAsync" -A 5 -B 5
# Search for WithoutAuditableSideEffects usage
rg "WithoutAuditableSideEffects" -A 5 -B 5
# Look for domain error definitions and handling
rg "DomainError" -A 5
Length of output: 41405
Script:
#!/bin/bash
# Let's examine the DialogSeenLog and SeenAt related code to verify domain validation
# Search for DialogSeenLog entity and related code
ast-grep --pattern 'class DialogSeenLog {
$$$
}'
# Look for any validation rules or constraints on SeenAt
rg -B 5 -A 5 "SeenAt.*=.*"
# Search for any domain validation related to SeenAt updates
rg -B 5 -A 5 "UpdateSeenAt"
Length of output: 18577
...main.Dialogporten.Application/Features/V1/ServiceOwner/Dialogs/Queries/Get/GetDialogQuery.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
src/Digdir.Domain.Dialogporten.Domain/Dialogs/Entities/DialogEntity.cs (1)
Line range hint
101-114: Consider domain model improvements.The current implementation mixes domain concepts with data transfer concerns. Consider these improvements:
- Create a dedicated value object for actor identity in the domain model
- Use domain events to handle the seen status tracking
- Consider making
SeenByimmutableExample domain model improvements:
public sealed class ActorIdentity { public ActorType Type { get; } public string Id { get; } public string? Name { get; } private ActorIdentity(ActorType type, string id, string? name) { Type = type; Id = id; Name = name; } public static ActorIdentity Create(ActorType type, string id, string? name) => new(type, id, name); }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (4)
docs/schema/V1/swagger.verified.json(29 hunks)src/Digdir.Domain.Dialogporten.Application/Features/V1/EndUser/Dialogs/Queries/Search/DialogDtoBase.cs(3 hunks)src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Dialogs/Queries/Search/DialogDtoBase.cs(3 hunks)src/Digdir.Domain.Dialogporten.Domain/Dialogs/Entities/DialogEntity.cs(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/Digdir.Domain.Dialogporten.Application/Features/V1/EndUser/Dialogs/Queries/Search/DialogDtoBase.cs
- src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Dialogs/Queries/Search/DialogDtoBase.cs
🔇 Additional comments (5)
src/Digdir.Domain.Dialogporten.Domain/Dialogs/Entities/DialogEntity.cs (2)
12-12: Verify if this import is needed.
The Identifiable namespace is imported but not used in this file. Consider removing it if it's not needed for future changes.
#!/bin/bash
# Check if IIdentifiableEntity is implemented or used anywhere in this file
rg -l "IIdentifiableEntity" "src/Digdir.Domain.Dialogporten.Domain/Dialogs/Entities/DialogEntity.cs"
# Check for any other uses of types from the Identifiable namespace
rg -l "Identifiable\." "src/Digdir.Domain.Dialogporten.Domain/Dialogs/Entities/DialogEntity.cs"Line range hint 101-114: Consider aligning with the consolidated actor model.
The method creates a DialogSeenLogSeenByActor which appears to be using the old actor model. This seems inconsistent with the PR's objective of consolidating actor DTOs into a common model.
Consider refactoring to use the new consolidated actor model:
SeenLog.Add(new DialogSeenLog
{
EndUserTypeId = userTypeId,
IsViaServiceOwner = userTypeId == DialogUserType.Values.ServiceOwnerOnBehalfOfPerson,
- SeenBy = new DialogSeenLogSeenByActor
+ SeenBy = new CommonActor // Use the new consolidated actor type
{
ActorTypeId = ActorType.Values.PartyRepresentative,
ActorId = endUserId,
ActorName = endUserName
}
});docs/schema/V1/swagger.verified.json (3)
343-368: Well-structured common actor schemas!
The new V1EndUserCommonActors_Actor and V1ServiceOwnerCommonActors_Actor schemas are well-designed with:
- Clear property descriptions explaining mutual exclusivity between actorId/actorName
- Proper nullable flags and type definitions
- Helpful examples for implementation guidance
Also applies to: 1804-1829
Line range hint 1-4600: Successful consolidation of actor schemas.
The following schemas have been properly removed and replaced with the new common actor schemas:
- V1EndUserDialogActivitiesQueriesGet_PerformedByActor
- V1EndUserDialogLabelAssignmentLogQueriesSearch_LabelAssignmentLogActor
- V1EndUserDialogSeenLogsQueriesGet_SeenLogSeenByActor
- V1ServiceOwnerDialogActivitiesQueriesGet_ActivityPerformedByActor
- V1ServiceOwnerDialogTransmissionsQueriesGet_SenderActor
This consolidation improves schema maintainability by reducing duplication.
393-393: Consistent usage of common actor schemas across DTOs.
The new actor schemas are properly referenced across all relevant DTOs using standard OpenAPI $ref syntax. This consistent implementation supports the PR objective of consolidating actor DTOs.
Let's verify no old actor schemas are still referenced:
Also applies to: 451-451, 474-474, 497-497, 751-751, 1011-1011, 1072-1072, 1362-1362, 1409-1409, 1576-1576, 1737-1737, 2012-2012, 2481-2481, 2612-2612, 3068-3068, 3182-3182, 3203-3203, 3473-3473, 3733-3733, 3795-3795, 4107-4107, 4154-4154, 4222-4222, 4379-4379, 4536-4536
✅ Verification successful
Schema consolidation successfully verified
The verification confirms that:
- All actor references have been properly consolidated into two main schemas:
V1EndUserCommonActors_ActorandV1ServiceOwnerCommonActors_Actor - No legacy actor schemas (PerformedByActor, SenderActor, SeenByActor, LabelAssignmentLogActor) are referenced
- The schema references are consistently used across all relevant DTOs with proper context separation between EndUser and ServiceOwner scenarios
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining references to old actor schemas
rg -i "V1.*PerformedByActor|V1.*SenderActor|V1.*SeenByActor|V1.*LabelAssignmentLogActor"
Length of output: 90
Script:
#!/bin/bash
# Let's verify the schema references more thoroughly by:
# 1. Finding all actor-related $ref references
# 2. Ensuring they point to the new consolidated schema
rg -A 1 '"performedBy"|"seenBy"|"sender"|"actor"' docs/schema/V1/swagger.verified.json
Length of output: 2749
Script:
#!/bin/bash
# Let's verify the actual schema definitions to ensure proper consolidation
rg -A 5 'V1EndUserCommonActors_Actor|V1ServiceOwnerCommonActors_Actor' docs/schema/V1/swagger.verified.json
Length of output: 6881
...dir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Common/Actors/ActorValidator.cs
Show resolved
Hide resolved
|
🤖 I have created a release *beep* *boop* --- ## [1.30.0](v1.29.0...v1.30.0) (2024-11-08) ### Features * **performance:** Performance/create serviceowner search ([#1413](#1413)) ([f1096a4](f1096a4)) * **webapi:** Combine actorDtos ([#1374](#1374)) ([ca18a99](ca18a99)) * **webapi:** Limit Content-Length / request body size ([#1416](#1416)) ([44be20a](44be20a)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Description
Replaced all actorDtos with one common enduser actordto, and one common serivceowner actordto
Related Issue(s)
Verification
Documentation
docs-directory, Altinnpedia or a separate linked PR in altinn-studio-docs., if applicable)Summary by CodeRabbit
New Features
ActorDtofor actor representation across various components, replacing multiple specific actor DTOs.ActorDtoto ensure proper data integrity based on actor type.Bug Fixes
Documentation
Tests
ActorValidatorto ensure validation rules are correctly enforced.