From 62a85aed23f0b3a896ed4160ce0fc1ff90bd3e5f Mon Sep 17 00:00:00 2001 From: Sebastian Stehle Date: Tue, 23 Apr 2019 20:31:24 +0200 Subject: [PATCH] Bugfixes for plans. --- .../Apps/AppGrain.cs | 17 ++- .../Apps/Guards/GuardAppContributors.cs | 2 +- .../InvitationEmailEventConsumer.cs | 24 +++- .../Apps/AppContributorAssigned.cs | 2 + src/Squidex/appsettings.json | 6 +- .../Apps/AppGrainTests.cs | 22 +++- .../Apps/Guards/GuardAppContributorsTests.cs | 20 +-- .../InvitationEmailEventConsumerTests.cs | 120 +++++++++++------- 8 files changed, 148 insertions(+), 65 deletions(-) diff --git a/src/Squidex.Domain.Apps.Entities/Apps/AppGrain.cs b/src/Squidex.Domain.Apps.Entities/Apps/AppGrain.cs index d91a5ff4f..6a5e5ba2a 100644 --- a/src/Squidex.Domain.Apps.Entities/Apps/AppGrain.cs +++ b/src/Squidex.Domain.Apps.Entities/Apps/AppGrain.cs @@ -67,12 +67,12 @@ namespace Squidex.Domain.Apps.Entities.Apps Create(c); }); - case AssignContributor assigneContributor: - return UpdateReturnAsync(assigneContributor, async c => + case AssignContributor assignContributor: + return UpdateReturnAsync(assignContributor, async c => { - await GuardAppContributors.CanAssign(Snapshot.Contributors, c, userResolver, appPlansProvider.GetPlan(Snapshot.Plan?.PlanId), Snapshot.Roles); + await GuardAppContributors.CanAssign(Snapshot.Contributors, Snapshot.Roles, c, userResolver, GetPlan()); - AssignContributor(c); + AssignContributor(c, !Snapshot.Contributors.ContainsKey(assignContributor.ContributorId)); return EntityCreatedResult.Create(c.ContributorId, Version); }); @@ -218,6 +218,11 @@ namespace Squidex.Domain.Apps.Entities.Apps } } + private IAppLimitsPlan GetPlan() + { + return appPlansProvider.GetPlan(Snapshot.Plan?.PlanId); + } + public void Create(CreateApp command) { var appId = NamedId.Of(command.AppId, command.Name); @@ -261,9 +266,9 @@ namespace Squidex.Domain.Apps.Entities.Apps RaiseEvent(SimpleMapper.Map(command, new AppLanguageUpdated())); } - public void AssignContributor(AssignContributor command) + public void AssignContributor(AssignContributor command, bool isNew) { - RaiseEvent(SimpleMapper.Map(command, new AppContributorAssigned())); + RaiseEvent(SimpleMapper.Map(command, new AppContributorAssigned { IsNew = isNew })); } public void RemoveContributor(RemoveContributor command) diff --git a/src/Squidex.Domain.Apps.Entities/Apps/Guards/GuardAppContributors.cs b/src/Squidex.Domain.Apps.Entities/Apps/Guards/GuardAppContributors.cs index 17d1702d7..21b1b0a51 100644 --- a/src/Squidex.Domain.Apps.Entities/Apps/Guards/GuardAppContributors.cs +++ b/src/Squidex.Domain.Apps.Entities/Apps/Guards/GuardAppContributors.cs @@ -18,7 +18,7 @@ namespace Squidex.Domain.Apps.Entities.Apps.Guards { public static class GuardAppContributors { - public static Task CanAssign(AppContributors contributors, AssignContributor command, IUserResolver users, IAppLimitsPlan plan, Roles roles) + public static Task CanAssign(AppContributors contributors, Roles roles, AssignContributor command, IUserResolver users, IAppLimitsPlan plan) { Guard.NotNull(command, nameof(command)); diff --git a/src/Squidex.Domain.Apps.Entities/Apps/Invitation/InvitationEmailEventConsumer.cs b/src/Squidex.Domain.Apps.Entities/Apps/Invitation/InvitationEmailEventConsumer.cs index 01211fcd2..6c6b31b31 100644 --- a/src/Squidex.Domain.Apps.Entities/Apps/Invitation/InvitationEmailEventConsumer.cs +++ b/src/Squidex.Domain.Apps.Entities/Apps/Invitation/InvitationEmailEventConsumer.cs @@ -5,7 +5,9 @@ // All rights reserved. Licensed under the MIT license. // ========================================================================== +using System; using System.Threading.Tasks; +using NodaTime; using Squidex.Domain.Apps.Events.Apps; using Squidex.Infrastructure; using Squidex.Infrastructure.EventSourcing; @@ -17,6 +19,7 @@ namespace Squidex.Domain.Apps.Entities.Apps.Invitation { public sealed class InvitationEmailEventConsumer : IEventConsumer { + private static readonly Duration MaxAge = Duration.FromDays(2); private readonly IInvitationEmailSender emailSender; private readonly IUserResolver userResolver; private readonly ISemanticLog log; @@ -60,8 +63,27 @@ namespace Squidex.Domain.Apps.Entities.Apps.Invitation return; } - if (@event.Payload is AppContributorAssigned appContributorAssigned && appContributorAssigned.Actor.IsSubject) + if (@event.Headers.EventStreamNumber() <= 1) { + return; + } + + var now = SystemClock.Instance.GetCurrentInstant(); + + var timestamp = @event.Headers.Timestamp(); + + if (now - timestamp > MaxAge) + { + return; + } + + if (@event.Payload is AppContributorAssigned appContributorAssigned) + { + if (!appContributorAssigned.Actor.IsSubject || !appContributorAssigned.IsNew) + { + return; + } + var assignerId = appContributorAssigned.Actor.Identifier; var assigneeId = appContributorAssigned.ContributorId; diff --git a/src/Squidex.Domain.Apps.Events/Apps/AppContributorAssigned.cs b/src/Squidex.Domain.Apps.Events/Apps/AppContributorAssigned.cs index 64e36ed6b..30f6bb480 100644 --- a/src/Squidex.Domain.Apps.Events/Apps/AppContributorAssigned.cs +++ b/src/Squidex.Domain.Apps.Events/Apps/AppContributorAssigned.cs @@ -17,5 +17,7 @@ namespace Squidex.Domain.Apps.Events.Apps public string Role { get; set; } public bool IsCreated { get; set; } + + public bool IsNew { get; set; } } } diff --git a/src/Squidex/appsettings.json b/src/Squidex/appsettings.json index c244a8b2b..932603d27 100644 --- a/src/Squidex/appsettings.json +++ b/src/Squidex/appsettings.json @@ -104,15 +104,15 @@ /* * The email body when a new user is added as contributor. */ - "newUserBody": "Welcome to Squidex\r\nDear User,\r\n\r\n{{var:assigner_name}} ($ASSIGNER_EMAIL) has invited you to join Project (also called an App) {{var:app_name}} at Squidex Headless CMS. Login with your Github, Google or Microsoft credentials to create a new user account and start editing content now.\r\n\r\nThank you very much,\r\nThe Squidex Team\r\n\r\n<> [https://cloud.squidex.io]", + "newUserBody": "Welcome to Squidex\r\nDear User,\r\n\r\n{{$ASSIGNER_NAME}} ($ASSIGNER_EMAIL) has invited you to join Project (also called an App) {{var:app_name}} at Squidex Headless CMS. Login with your Github, Google or Microsoft credentials to create a new user account and start editing content now.\r\n\r\nThank you very much,\r\nThe Squidex Team\r\n\r\n<> [$UI_URL]", /* * The email subject when an existing user is added as contributor. */ - "existingUserSubject": "[Squidex CMS] You have been invited to App $APP_NAME", + "existingUserSubject": "[Squidex CMS] You have been invited to join App $APP_NAME", /* * The email body when an existing user is added as contributor. */ - "existingUserBody": "You have been invited to join an App at Squidex CMS\n\nWelcome to Squidex\n$ASSIGNER_NAME ($ASSIGNER_EMAIL) has invited you to join app $APP_NAME at Squidex Headless CMS.\nLogin or reload the Management UI to see the app.\n\nThank you very much,\nThe Squidex Team\n<> [$UI_URL]" + "existingUserBody": "Dear User,\r\n\r\n{{$ASSIGNER_NAME}} ($ASSIGNER_EMAIL) has invited you to join App {{var:app_name}} at Squidex Headless CMS.\r\n\r\nLogin or reload the Management UI to see the App\r\n\r\nThank you very much,\r\nThe Squidex Team\r\n\r\n<> [$UI_URL]" } }, diff --git a/tests/Squidex.Domain.Apps.Entities.Tests/Apps/AppGrainTests.cs b/tests/Squidex.Domain.Apps.Entities.Tests/Apps/AppGrainTests.cs index 924cfc999..dd42c1a43 100644 --- a/tests/Squidex.Domain.Apps.Entities.Tests/Apps/AppGrainTests.cs +++ b/tests/Squidex.Domain.Apps.Entities.Tests/Apps/AppGrainTests.cs @@ -171,7 +171,27 @@ namespace Squidex.Domain.Apps.Entities.Apps LastEvents .ShouldHaveSameEvents( - CreateEvent(new AppContributorAssigned { ContributorId = contributorId, Role = Role.Editor }) + CreateEvent(new AppContributorAssigned { ContributorId = contributorId, Role = Role.Editor, IsNew = true }) + ); + } + + [Fact] + public async Task AssignContributor_should_create_update_events_and_update_state() + { + var command = new AssignContributor { ContributorId = contributorId, Role = Role.Owner }; + + await ExecuteCreateAsync(); + await ExecuteAssignContributorAsync(); + + var result = await sut.ExecuteAsync(CreateCommand(command)); + + result.ShouldBeEquivalent(EntityCreatedResult.Create(contributorId, 6)); + + Assert.Equal(Role.Owner, sut.Snapshot.Contributors[contributorId]); + + LastEvents + .ShouldHaveSameEvents( + CreateEvent(new AppContributorAssigned { ContributorId = contributorId, Role = Role.Owner }) ); } diff --git a/tests/Squidex.Domain.Apps.Entities.Tests/Apps/Guards/GuardAppContributorsTests.cs b/tests/Squidex.Domain.Apps.Entities.Tests/Apps/Guards/GuardAppContributorsTests.cs index fee3c9092..71f9bb239 100644 --- a/tests/Squidex.Domain.Apps.Entities.Tests/Apps/Guards/GuardAppContributorsTests.cs +++ b/tests/Squidex.Domain.Apps.Entities.Tests/Apps/Guards/GuardAppContributorsTests.cs @@ -55,7 +55,7 @@ namespace Squidex.Domain.Apps.Entities.Apps.Guards { var command = new AssignContributor(); - return ValidationAssert.ThrowsAsync(() => GuardAppContributors.CanAssign(contributors_0, command, users, appPlan, roles), + return ValidationAssert.ThrowsAsync(() => GuardAppContributors.CanAssign(contributors_0, roles, command, users, appPlan), new ValidationError("Contributor id is required.", "ContributorId")); } @@ -64,7 +64,7 @@ namespace Squidex.Domain.Apps.Entities.Apps.Guards { var command = new AssignContributor { ContributorId = "1", Role = "Invalid" }; - return ValidationAssert.ThrowsAsync(() => GuardAppContributors.CanAssign(contributors_0, command, users, appPlan, roles), + return ValidationAssert.ThrowsAsync(() => GuardAppContributors.CanAssign(contributors_0, roles, command, users, appPlan), new ValidationError("Role is not a valid value.", "Role")); } @@ -75,7 +75,7 @@ namespace Squidex.Domain.Apps.Entities.Apps.Guards var contributors_1 = contributors_0.Assign("1", Role.Owner); - return ValidationAssert.ThrowsAsync(() => GuardAppContributors.CanAssign(contributors_1, command, users, appPlan, roles), + return ValidationAssert.ThrowsAsync(() => GuardAppContributors.CanAssign(contributors_1, roles, command, users, appPlan), new ValidationError("Contributor has already this role.", "Role")); } @@ -84,7 +84,7 @@ namespace Squidex.Domain.Apps.Entities.Apps.Guards { var command = new AssignContributor { ContributorId = "notfound", Role = Role.Owner }; - return Assert.ThrowsAsync(() => GuardAppContributors.CanAssign(contributors_0, command, users, appPlan, roles)); + return Assert.ThrowsAsync(() => GuardAppContributors.CanAssign(contributors_0, roles, command, users, appPlan)); } [Fact] @@ -92,7 +92,7 @@ namespace Squidex.Domain.Apps.Entities.Apps.Guards { var command = new AssignContributor { ContributorId = "3", Role = Role.Editor, Actor = new RefToken("user", "3") }; - return Assert.ThrowsAsync(() => GuardAppContributors.CanAssign(contributors_0, command, users, appPlan, roles)); + return Assert.ThrowsAsync(() => GuardAppContributors.CanAssign(contributors_0, roles, command, users, appPlan)); } [Fact] @@ -106,7 +106,7 @@ namespace Squidex.Domain.Apps.Entities.Apps.Guards var contributors_1 = contributors_0.Assign("1", Role.Owner); var contributors_2 = contributors_1.Assign("2", Role.Editor); - return ValidationAssert.ThrowsAsync(() => GuardAppContributors.CanAssign(contributors_2, command, users, appPlan, roles), + return ValidationAssert.ThrowsAsync(() => GuardAppContributors.CanAssign(contributors_2, roles, command, users, appPlan), new ValidationError("You have reached the maximum number of contributors for your plan.")); } @@ -115,7 +115,7 @@ namespace Squidex.Domain.Apps.Entities.Apps.Guards { var command = new AssignContributor { ContributorId = "1@email.com" }; - await GuardAppContributors.CanAssign(contributors_0, command, users, appPlan, roles); + await GuardAppContributors.CanAssign(contributors_0, roles, command, users, appPlan); Assert.Equal("1", command.ContributorId); } @@ -125,7 +125,7 @@ namespace Squidex.Domain.Apps.Entities.Apps.Guards { var command = new AssignContributor { ContributorId = "1" }; - return GuardAppContributors.CanAssign(contributors_0, command, users, appPlan, roles); + return GuardAppContributors.CanAssign(contributors_0, roles, command, users, appPlan); } [Fact] @@ -135,7 +135,7 @@ namespace Squidex.Domain.Apps.Entities.Apps.Guards var contributors_1 = contributors_0.Assign("1", Role.Editor); - return GuardAppContributors.CanAssign(contributors_1, command, users, appPlan, roles); + return GuardAppContributors.CanAssign(contributors_1, roles, command, users, appPlan); } [Fact] @@ -149,7 +149,7 @@ namespace Squidex.Domain.Apps.Entities.Apps.Guards var contributors_1 = contributors_0.Assign("1", Role.Editor); var contributors_2 = contributors_1.Assign("2", Role.Editor); - return GuardAppContributors.CanAssign(contributors_2, command, users, appPlan, roles); + return GuardAppContributors.CanAssign(contributors_2, roles, command, users, appPlan); } [Fact] diff --git a/tests/Squidex.Domain.Apps.Entities.Tests/Apps/Invitation/InvitationEmailEventConsumerTests.cs b/tests/Squidex.Domain.Apps.Entities.Tests/Apps/Invitation/InvitationEmailEventConsumerTests.cs index a5e94c8f7..f5accf77e 100644 --- a/tests/Squidex.Domain.Apps.Entities.Tests/Apps/Invitation/InvitationEmailEventConsumerTests.cs +++ b/tests/Squidex.Domain.Apps.Entities.Tests/Apps/Invitation/InvitationEmailEventConsumerTests.cs @@ -8,6 +8,7 @@ using System; using System.Threading.Tasks; using FakeItEasy; +using NodaTime; using Squidex.Domain.Apps.Events.Apps; using Squidex.Infrastructure; using Squidex.Infrastructure.EventSourcing; @@ -34,88 +35,104 @@ namespace Squidex.Domain.Apps.Entities.Apps.Invitation A.CallTo(() => emailSender.IsActive) .Returns(true); + A.CallTo(() => userResolver.FindByIdOrEmailAsync(assignerId)) + .Returns(assigner); + + A.CallTo(() => userResolver.FindByIdOrEmailAsync(assigneeId)) + .Returns(assignee); + sut = new InvitationEmailEventConsumer(emailSender, userResolver, log); } [Fact] - public async Task Should_ignore_contributors_assigned_by_clients() + public async Task Should_not_send_email_if_contributors_assigned_by_clients() { - var @event = Envelope.Create(CreateEvent(RefTokenType.Client, true)); + var @event = CreateEvent(RefTokenType.Client, true); await sut.On(@event); - A.CallTo(() => userResolver.FindByIdOrEmailAsync(A.Ignored)) - .MustNotHaveHappened(); + MustNotResolveUser(); + MustNotSendEmail(); + } - A.CallTo(() => emailSender.SendNewUserEmailAsync(A.Ignored, A.Ignored, appName)) - .MustNotHaveHappened(); + [Fact] + public async Task Should_not_send_email_for_initial_owner() + { + var @event = CreateEvent(RefTokenType.Subject, false, streamNumber: 1); + + await sut.On(@event); + + MustNotSendEmail(); + } + + [Fact] + public async Task Should_not_send_email_for_old_events() + { + var @event = CreateEvent(RefTokenType.Subject, true, instant: SystemClock.Instance.GetCurrentInstant().Minus(Duration.FromHours(50))); + + await sut.On(@event); + + MustNotResolveUser(); + MustNotSendEmail(); + } + + [Fact] + public async Task Should_not_send_email_for_old_contributor() + { + var @event = CreateEvent(RefTokenType.Subject, true, isNewContributor: false); + + await sut.On(@event); + + MustNotResolveUser(); + MustNotSendEmail(); } [Fact] public async Task Should_not_send_email_if_sender_not_active() { - var @event = Envelope.Create(CreateEvent(RefTokenType.Subject, true)); + var @event = CreateEvent(RefTokenType.Subject, true); A.CallTo(() => emailSender.IsActive) .Returns(false); - A.CallTo(() => userResolver.FindByIdOrEmailAsync(assignerId)) - .Returns(Task.FromResult(null)); - await sut.On(@event); - A.CallTo(() => userResolver.FindByIdOrEmailAsync(A.Ignored)) - .MustNotHaveHappened(); - - A.CallTo(() => emailSender.SendNewUserEmailAsync(A.Ignored, A.Ignored, appName)) - .MustNotHaveHappened(); + MustNotResolveUser(); + MustNotSendEmail(); } [Fact] public async Task Should_not_send_email_if_assigner_not_found() { - var @event = Envelope.Create(CreateEvent(RefTokenType.Subject, true)); + var @event = CreateEvent(RefTokenType.Subject, true); A.CallTo(() => userResolver.FindByIdOrEmailAsync(assignerId)) .Returns(Task.FromResult(null)); await sut.On(@event); - A.CallTo(() => emailSender.SendNewUserEmailAsync(A.Ignored, A.Ignored, appName)) - .MustNotHaveHappened(); - + MustNotSendEmail(); MustLogWarning(); } [Fact] public async Task Should_not_send_email_if_assignee_not_found() { - var @event = Envelope.Create(CreateEvent(RefTokenType.Subject, true)); - - A.CallTo(() => userResolver.FindByIdOrEmailAsync(assignerId)) - .Returns(assigner); + var @event = CreateEvent(RefTokenType.Subject, true); A.CallTo(() => userResolver.FindByIdOrEmailAsync(assigneeId)) .Returns(Task.FromResult(null)); await sut.On(@event); - A.CallTo(() => emailSender.SendNewUserEmailAsync(A.Ignored, A.Ignored, appName)) - .MustNotHaveHappened(); - + MustNotSendEmail(); MustLogWarning(); } [Fact] public async Task Should_send_email_for_new_user() { - var @event = Envelope.Create(CreateEvent(RefTokenType.Subject, true)); - - A.CallTo(() => userResolver.FindByIdOrEmailAsync(assignerId)) - .Returns(assigner); - - A.CallTo(() => userResolver.FindByIdOrEmailAsync(assigneeId)) - .Returns(assignee); + var @event = CreateEvent(RefTokenType.Subject, true); await sut.On(@event); @@ -126,13 +143,7 @@ namespace Squidex.Domain.Apps.Entities.Apps.Invitation [Fact] public async Task Should_send_email_for_existing_user() { - var @event = Envelope.Create(CreateEvent(RefTokenType.Subject, false)); - - A.CallTo(() => userResolver.FindByIdOrEmailAsync(assignerId)) - .Returns(assigner); - - A.CallTo(() => userResolver.FindByIdOrEmailAsync(assigneeId)) - .Returns(assignee); + var @event = CreateEvent(RefTokenType.Subject, false); await sut.On(@event); @@ -146,15 +157,38 @@ namespace Squidex.Domain.Apps.Entities.Apps.Invitation .MustHaveHappened(); } - private IEvent CreateEvent(string assignerType, bool isNew) + private void MustNotResolveUser() { - return new AppContributorAssigned + A.CallTo(() => userResolver.FindByIdOrEmailAsync(A.Ignored)) + .MustNotHaveHappened(); + } + + private void MustNotSendEmail() + { + A.CallTo(() => emailSender.SendNewUserEmailAsync(A.Ignored, A.Ignored, A.Ignored)) + .MustNotHaveHappened(); + + A.CallTo(() => emailSender.SendExistingUserEmailAsync(A.Ignored, A.Ignored, A.Ignored)) + .MustNotHaveHappened(); + } + + private Envelope CreateEvent(string assignerType, bool isNewUser, bool isNewContributor = true, Instant? instant = null, int streamNumber = 2) + { + var @event = new AppContributorAssigned { Actor = new RefToken(assignerType, assignerId), AppId = new NamedId(Guid.NewGuid(), appName), ContributorId = assigneeId, - IsCreated = isNew + IsCreated = isNewUser, + IsNew = isNewContributor, }; + + var envelope = Envelope.Create(@event); + + envelope.SetTimestamp(instant ?? SystemClock.Instance.GetCurrentInstant()); + envelope.SetEventStreamNumber(streamNumber); + + return envelope; } } }