Skip to content

Commit

Permalink
fix: Allow new activities to reference old activities (#935)
Browse files Browse the repository at this point in the history
<!--- Provide a general summary of your changes in the Title above -->

## Description

It is currently not possible to create a new DialogActivity that refers
to an old DialogActivity via `relatedActivityId`
This check in the validator requires the referred Id to be present in
the DTO, which we don't allow.
```cshap
        RuleForEach(x => x.Activities)
            .IsIn(x => x.Activities,
                dependentKeySelector: activity => activity.RelatedActivityId,
                principalKeySelector: activity => activity.Id)
            .SetValidator(activityValidator);
```

This suggestion is to move this check from the validator to the handler
and load all activities to verify the relation ID.

## Related Issue(s)
- #931 

## Verification

- [x] **Your** code builds clean without any errors or warnings
- [x] Manual testing done (required)
- [x] Relevant automated test added (if you find this hard, leave it and
we'll help out)

## Documentation

- [ ] Documentation is updated (either in `docs`-directory, Altinnpedia
or a separate linked PR in
[altinn-studio-docs.](https://github.com/Altinn/altinn-studio-docs), if
applicable)
  • Loading branch information
oskogstad authored Jul 29, 2024
1 parent b91a1a8 commit bbc443e
Show file tree
Hide file tree
Showing 4 changed files with 93 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ public async Task<UpdateDialogResult> Handle(UpdateDialogCommand request, Cancel
var resourceIds = await _userResourceRegistry.GetCurrentUserResourceIds(cancellationToken);

var dialog = await _db.Dialogs
.Include(x => x.Activities)
.Include(x => x.Content)
.ThenInclude(x => x.Value.Localizations)
.Include(x => x.SearchTags)
Expand Down Expand Up @@ -106,7 +107,8 @@ public async Task<UpdateDialogResult> Handle(UpdateDialogCommand request, Cancel
// Update primitive properties
_mapper.Map(request.Dto, dialog);
ValidateTimeFields(dialog);
await AppendActivity(dialog, request.Dto, cancellationToken);
AppendActivity(dialog, request.Dto);
VerifyActivityRelations(dialog);

dialog.SearchTags
.Merge(request.Dto.SearchTags,
Expand Down Expand Up @@ -196,16 +198,19 @@ private void ValidateTimeFields(DialogEntity dialog)
}
}

private async Task AppendActivity(DialogEntity dialog, UpdateDialogDto dto, CancellationToken cancellationToken)
private void AppendActivity(DialogEntity dialog, UpdateDialogDto dto)
{
var newDialogActivities = _mapper.Map<List<DialogActivity>>(dto.Activities);

var existingIds = await _db.GetExistingIds(newDialogActivities, cancellationToken);
var existingIds = dialog.Activities.Select(x => x.Id).ToList();

existingIds = existingIds.Intersect(newDialogActivities.Select(x => x.Id)).ToList();
if (existingIds.Count != 0)
{
_domainContext.AddError(
nameof(UpdateDialogDto.Activities),
$"Entity '{nameof(DialogActivity)}' with the following key(s) already exists: ({string.Join(", ", existingIds)}).");
return;
}

dialog.Activities.AddRange(newDialogActivities);
Expand All @@ -214,6 +219,33 @@ private async Task AppendActivity(DialogEntity dialog, UpdateDialogDto dto, Canc
_db.DialogActivities.AddRange(newDialogActivities);
}

private void VerifyActivityRelations(DialogEntity dialog)
{
var relatedActivityIds = dialog.Activities
.Where(x => x.RelatedActivityId is not null)
.Select(x => x.RelatedActivityId)
.ToList();

if (relatedActivityIds.Count == 0)
{
return;
}

var activityIds = dialog.Activities.Select(x => x.Id).ToList();

var invalidRelatedActivityIds = relatedActivityIds
.Where(id => !activityIds.Contains(id!.Value))
.ToList();

if (invalidRelatedActivityIds.Count != 0)
{
_domainContext.AddError(
nameof(UpdateDialogDto.Activities),
$"Invalid '{nameof(DialogActivity.RelatedActivityId)}, entity '{nameof(DialogActivity)}'" +
$" with the following key(s) does not exist: ({string.Join(", ", invalidRelatedActivityIds)}) in '{nameof(dialog.Activities)}'");
}
}

private IEnumerable<DialogApiAction> CreateApiActions(IEnumerable<UpdateDialogDialogApiActionDto> creatables)
{
return creatables.Select(x =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,9 +95,6 @@ public UpdateDialogDtoValidator(
RuleFor(x => x.Activities)
.UniqueBy(x => x.Id);
RuleForEach(x => x.Activities)
.IsIn(x => x.Activities,
dependentKeySelector: activity => activity.RelatedActivityId,
principalKeySelector: activity => activity.Id)
.SetValidator(activityValidator);
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
using System.Collections.ObjectModel;
using System.Reflection;
using System.Text.Json;
using AutoMapper;
using Digdir.Domain.Dialogporten.Application.Common;
using Digdir.Domain.Dialogporten.Application.Externals;
using Digdir.Domain.Dialogporten.Application.Externals.AltinnAuthorization;
Expand Down Expand Up @@ -29,6 +31,7 @@ namespace Digdir.Domain.Dialogporten.Application.Integration.Tests.Common;

public class DialogApplication : IAsyncLifetime
{
private IMapper? _mapper;
private Respawner _respawner = null!;
private ServiceProvider _rootProvider = null!;
private readonly PostgreSqlContainer _dbContainer = new PostgreSqlBuilder()
Expand All @@ -37,6 +40,12 @@ public class DialogApplication : IAsyncLifetime

public async Task InitializeAsync()
{
var config = new MapperConfiguration(cfg =>
{
cfg.AddMaps(Assembly.GetAssembly(typeof(ApplicationSettings)));
});
_mapper = config.CreateMapper();

AssertionOptions.AssertEquivalencyUsing(options =>
{
//options.ExcludingMissingMembers();
Expand Down Expand Up @@ -144,6 +153,8 @@ private static IServiceOwnerNameRegistry CreateServiceOwnerNameRegistrySubstitut
return organizationRegistrySubstitute;
}

public IMapper GetMapper() => _mapper!;

public async Task DisposeAsync()
{
await _rootProvider.DisposeAsync();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,55 @@
using Digdir.Domain.Dialogporten.Application.Integration.Tests.Common;
using Digdir.Domain.Dialogporten.Application.Features.V1.ServiceOwner.Dialogs.Queries.Get;
using Digdir.Domain.Dialogporten.Application.Features.V1.ServiceOwner.Dialogs.Commands.Create;
using Digdir.Domain.Dialogporten.Application.Features.V1.ServiceOwner.Dialogs.Commands.Update;
using Digdir.Domain.Dialogporten.Application.Integration.Tests.Common;
using Digdir.Domain.Dialogporten.Domain.Dialogs.Entities.Activities;
using Digdir.Domain.Dialogporten.Domain.Dialogs.Entities.Actors;
using Digdir.Tool.Dialogporten.GenerateFakeData;
using FluentAssertions;

namespace Digdir.Domain.Dialogporten.Application.Integration.Tests.Features.V1.ServiceOwner.Dialogs.Commands;

[Collection(nameof(DialogCqrsCollectionFixture))]
public class UpdateDialogTests : ApplicationCollectionFixture
public class UpdateDialogTests(DialogApplication application) : ApplicationCollectionFixture(application)
{
public UpdateDialogTests(DialogApplication application) : base(application)
[Fact]
public async Task New_Activity_Should_Be_Able_To_Refer_To_Old_Activity()
{
// Arrange
var (_, createCommandResponse) = await GenerateDialogWithActivity();
var getDialogQuery = new GetDialogQuery { DialogId = createCommandResponse.AsT0.Value };
var getDialogDto = await Application.Send(getDialogQuery);

var mapper = Application.GetMapper();
var updateDialogDto = mapper.Map<UpdateDialogDto>(getDialogDto.AsT0);

// New activity refering to old activity
updateDialogDto.Activities.Add(new UpdateDialogDialogActivityDto
{
Type = DialogActivityType.Values.DialogClosed,
RelatedActivityId = getDialogDto.AsT0.Activities.First().Id,
PerformedBy = new UpdateDialogDialogActivityActorDto
{
ActorType = DialogActorType.Values.ServiceOwner
}
});

// Act
var updateResponse = await Application.Send(new UpdateDialogCommand { Id = createCommandResponse.AsT0.Value, Dto = updateDialogDto });

// Assert
updateResponse.TryPickT0(out var result, out _).Should().BeTrue();
result.Should().NotBeNull();
}
// TODO: Add tests
}

private async Task<(CreateDialogCommand, CreateDialogResult)> GenerateDialogWithActivity()
{
var createDialogCommand = DialogGenerator.GenerateSimpleFakeDialog();
var activity = DialogGenerator.GenerateFakeDialogActivity(type: DialogActivityType.Values.Information);
activity.PerformedBy.ActorId = DialogGenerator.GenerateRandomParty(forcePerson: true);
activity.PerformedBy.ActorName = null;
createDialogCommand.Activities.Add(activity);
var createCommandResponse = await Application.Send(createDialogCommand);
return (createDialogCommand, createCommandResponse);
}
}

0 comments on commit bbc443e

Please sign in to comment.