Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ability to set Sender for mail #375

Closed
wants to merge 3 commits into from
Closed

Add ability to set Sender for mail #375

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4884742
Consolidate target frameworks for libraries.
johnwc Feb 27, 2024
f6df4f4
Add Sender setting
johnwc Feb 27, 2024
2f91329
Have From() override default global From
johnwc Feb 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions Src/Coravel.Cache.Database/Coravel.Cache.Database.Core/Coravel.Cache.Database.Core.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<Project Sdk="Microsoft.NET.Sdk">

<PropertyGroup>
<TargetFramework>netstandard2.0</TargetFramework>
<TargetFramework>netstandard2.1</TargetFramework>
<PackageId>Coravel.Cache.Database.Core</PackageId>
<Version>2.0.2</Version>
<Authors>James Hickey</Authors>
Expand All @@ -19,9 +19,12 @@
</PropertyGroup>

<ItemGroup>
<PackageReference Include="Coravel" Version="3.0.0" />
<PackageReference Include="Dapper" Version="1.60.6" />
<PackageReference Include="Dapper" Version="2.1.28" />
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the reasoning behind changing the version here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not, it is a local build depending on the and not a runtime dependency. It is not going to have dependency issue for the consumer. Would we not want to use the latest and greatest updates from a package like Dapper, so to utilize performance and security updates?

<PackageReference Include="Newtonsoft.Json" Version="12.0.2" />
</ItemGroup>

<ItemGroup>
<ProjectReference Include="..\..\Coravel\Coravel.csproj" />
</ItemGroup>

Comment on lines +26 to +29
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because Coravel.Cache.Database.Core is a nuget package, we can't reference another project this way. It's unfortunate, but nuget doesn't support bringing in project references into the final published binary.

So this would actually break at either publish or runtime since the Coravel package would be missing.

We'd have to revert this back to the PackageReference.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We reference our packages like this, and all of our packages show that they have dependencies to the library that is Project Referenced. I don't think I have ever not seen that.

Looking at my local nupkg of this library, so does yours. Am I not understanding what you are referring to?

<?xml version="1.0" encoding="utf-8"?>
<package xmlns="http://schemas.microsoft.com/packaging/2013/05/nuspec.xsd">
  <metadata>
    <id>Coravel.Cache.Database.Core</id>
    <version>2.0.2</version>
    <title>Coravel Database Cache Driver Shared Library</title>
    <authors>James Hickey</authors>
    <license type="expression">MIT</license>
    <licenseUrl>https://licenses.nuget.org/MIT</licenseUrl>
    <projectUrl>https://github.com/jamesmh/coravel</projectUrl>
    <description>Core tools for Coravel database cache drivers</description>
    <tags>netcore coravel caching database</tags>
    <repository type="git" url="https://github.com/jamesmh/coravel" commit="0ec0e2d0ed24f99c5ab4b24a8fd575114b4f3b7b" />
    <dependencies>
      <group targetFramework=".NETStandard2.1">
        <dependency id="Coravel" version="5.0.2" exclude="Build,Analyzers" />
        <dependency id="Dapper" version="2.1.28" exclude="Build,Analyzers" />
        <dependency id="Newtonsoft.Json" version="12.0.2" exclude="Build,Analyzers" />
      </group>
    </dependencies>
  </metadata>
</package>

</Project>
1 change: 0 additions & 1 deletion Src/Coravel.Cache.Database/Coravel.Cache.Database.Core/Extensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
using System;
using System.Data.Common;
using System.Data.SqlClient;
using System.Threading.Tasks;

namespace Coravel.Cache.Database.Core
Expand Down
1 change: 0 additions & 1 deletion Src/Coravel.Cache.Database/Coravel.Cache.Database.Core/IDriver.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
using System.Data.Common;
using System.Data.SqlClient;

namespace Coravel.Cache.Database.Core
{
Expand Down
7 changes: 5 additions & 2 deletions Src/Coravel.Cache.Database/Coravel.Cache.PostgreSQL/Coravel.Cache.PostgreSQL.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,14 @@

<ItemGroup>
<PackageReference Include="npgsql" Version="4.0.7" />
<PackageReference Include="Coravel.Cache.Database.Core" version="2.0.0" />
</ItemGroup>

<ItemGroup>
<ProjectReference Include="..\Coravel.Cache.Database.Core\Coravel.Cache.Database.Core.csproj" />
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, you can't use project references in nuget like this. It will break. We'll have to revert this change.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, need clarification on what you are referring to, as my understanding is that it works OoB as correct dependency in built package.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just coming back to this to explain briefly. If we published this as a nuget package, everything would build fine and publish fine. But, the consumer would get an error when consuming the nuget package.

It's been a debate for years and is currently ongoing: NuGet/Home#3891

There have been some "hacks" - some I've tried when I started this project years ago. But a hack is a hack - and it's not intuitive, ideal, implicit knowledge, etc. and I didn't want to add risk by using a hack.

Copy link
Contributor Author

@johnwc johnwc Sep 20, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you see my other comment prior, you will notice that when the packages get built and packaged, the other project reference packages get added as nuget package dependencies.

The issue you shared is something different, they want the DLL from the referenced project to be included in the parent project's nuget package and not as a nuget dependency. You do not want this; other libraries need to stay as other nuget packages. And if an outside project references the parent nuget, then the nuget restore will pull down all the dependent libraries which would include your other DLLs. If you added the DLL as another file to the nuget, then you would have issues with other packages that also have it included in thier nuget, but have other version etc... It would not allow nuget to do what it does best and maintain nuget version compatibility between packages.

Reference from the issue

Expected

projectB.dll should be in the .nupkg along with projectA.dll

Actual

projectB is still a package reference, not a DLL included in the package.

</ItemGroup>

<PropertyGroup>
<TargetFramework>netstandard2.0</TargetFramework>
<TargetFramework>netstandard2.1</TargetFramework>
<PackageId>Coravel.Cache.PostgreSQL</PackageId>
<Version>2.0.2</Version>
<Authors>James Hickey</Authors>
Expand Down
14 changes: 9 additions & 5 deletions Src/Coravel.Cache.Database/Coravel.Cache.SqlServer/Coravel.Cache.SqlServer.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,7 @@
<Project Sdk="Microsoft.NET.Sdk">

<ItemGroup>
<PackageReference Include="Coravel.Cache.Database.Core" version="2.0.0" />
</ItemGroup>

<PropertyGroup>
<TargetFramework>netstandard2.0</TargetFramework>
<TargetFramework>netstandard2.1</TargetFramework>
<PackageId>Coravel.Cache.SQLServer</PackageId>
<Version>2.0.2</Version>
<Authors>James Hickey</Authors>
Expand All @@ -22,4 +18,12 @@
<NoWarn>$(NoWarn);1591</NoWarn>
</PropertyGroup>

<ItemGroup>
<PackageReference Include="System.Data.SqlClient" Version="4.8.6" />
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need this new package? I don't see a change to warrant this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need a reference to this specific to use the version that does not have security vulnerabilities. It is already a dependency of the package, just need to push it to latest to avoid security issues.

</ItemGroup>

<ItemGroup>
<ProjectReference Include="..\Coravel.Cache.Database.Core\Coravel.Cache.Database.Core.csproj" />
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can't use project references, we have to stick to package references.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same response as above.

</ItemGroup>

</Project>
2 changes: 1 addition & 1 deletion Src/Coravel.Mailer/Mail/Interfaces/IMailer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,6 @@ public interface IMailer
{
Task<string> RenderAsync<T>(Mailable<T> mailable);
Task SendAsync<T>(Mailable<T> mailable);
Task SendAsync(string message, string subject, IEnumerable<MailRecipient> to, MailRecipient from, MailRecipient replyTo, IEnumerable<MailRecipient> cc, IEnumerable<MailRecipient> bcc, IEnumerable<Attachment> attachments = null);
Task SendAsync(string message, string subject, IEnumerable<MailRecipient> to, MailRecipient from, MailRecipient replyTo, MailRecipient sender, IEnumerable<MailRecipient> cc, IEnumerable<MailRecipient> bcc, IEnumerable<Attachment> attachments = null);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this breaks a public API. Anyone using this piece of Coravel will have their code broken if this gets published... So we'd need to think about the impact, potential alternatives, etc.

At the moment I'm not sure... first thought is if we put this at the end of the params then we can make it an optional param and not break existing usages.

Or, we could make a new method with an updated params here, depreciate the "old" one, and come up with some kind of versioning/sunsetting strategy here.

Open to suggestions.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would go with the new/old with soft [Obsolete] attribute on old. So, it gives them a warning with description to use the new.

}
}
20 changes: 19 additions & 1 deletion Src/Coravel.Mailer/Mail/Mailable.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,12 @@ public class Mailable<T>
/// <summary>
/// Who the email is from.
/// </summary>
private MailRecipient _from;
private MailRecipient _from;

/// <summary>
/// Who is sending the email on the from address's behalf.
/// </summary>
private MailRecipient? _sender;

/// <summary>
/// Recipients of the message.
Expand Down Expand Up @@ -64,6 +69,18 @@ public class Mailable<T>
/// </summary>
private T _viewModel;

public Mailable<T> Sender(MailRecipient recipient)
{
this._sender = recipient;
return this;
}

public Mailable<T> Sender(string email) =>
this.Sender(new MailRecipient(email));

public Mailable<T> Sender(string email, string name) =>
this.Sender(new MailRecipient(email, name));

public Mailable<T> From(MailRecipient recipient)
{
this._from = recipient;
Expand Down Expand Up @@ -173,6 +190,7 @@ await mailer.SendAsync(
this._to,
this._from,
this._replyTo,
this._sender,
this._cc,
this._bcc,
this._attachments
Expand Down
4 changes: 3 additions & 1 deletion Src/Coravel.Mailer/Mail/Mailers/AssertMailer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ public class Data
public IEnumerable<MailRecipient> to { get; set; }
public MailRecipient from { get; set; }
public MailRecipient replyTo { get; set; }
public MailRecipient sender { get; set; }
public IEnumerable<MailRecipient> cc { get; set; }
public IEnumerable<MailRecipient> bcc { get; set; }
public IEnumerable<Attachment> attachments { get; set; }
Expand All @@ -26,7 +27,7 @@ public AssertMailer(Action<Data> assertAction)
this._assertAction = assertAction;
}

public Task SendAsync(string message, string subject, IEnumerable<MailRecipient> to, MailRecipient from, MailRecipient replyTo, IEnumerable<MailRecipient> cc, IEnumerable<MailRecipient> bcc, IEnumerable<Attachment> attachments)
public Task SendAsync(string message, string subject, IEnumerable<MailRecipient> to, MailRecipient from, MailRecipient replyTo, MailRecipient sender, IEnumerable<MailRecipient> cc, IEnumerable<MailRecipient> bcc, IEnumerable<Attachment> attachments)
{
this._assertAction(new Data
{
Expand All @@ -35,6 +36,7 @@ public Task SendAsync(string message, string subject, IEnumerable<MailRecipient>
to = to,
from = from,
replyTo = replyTo,
sender = sender,
cc = cc,
bcc = bcc,
attachments = attachments
Expand Down
6 changes: 3 additions & 3 deletions Src/Coravel.Mailer/Mail/Mailers/CustomMailer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ namespace Coravel.Mailer.Mail.Mailers
{
public class CustomMailer : IMailer
{
public delegate Task SendAsyncFunc(string message, string subject, IEnumerable<MailRecipient> to, MailRecipient from, MailRecipient replyTo, IEnumerable<MailRecipient> cc, IEnumerable<MailRecipient> bcc, IEnumerable<Attachment> attachments = null);
public delegate Task SendAsyncFunc(string message, string subject, IEnumerable<MailRecipient> to, MailRecipient from, MailRecipient replyTo, MailRecipient sender, IEnumerable<MailRecipient> cc, IEnumerable<MailRecipient> bcc, IEnumerable<Attachment> attachments = null);
private RazorRenderer _renderer;
private SendAsyncFunc _sendAsyncFunc;
private MailRecipient _globalFrom;
Expand All @@ -25,10 +25,10 @@ public Task<string> RenderAsync<T>(Mailable<T> mailable) =>
public async Task SendAsync<T>(Mailable<T> mailable) =>
await mailable.SendAsync(this._renderer, this);

public async Task SendAsync(string message, string subject, IEnumerable<MailRecipient> to, MailRecipient from, MailRecipient replyTo, IEnumerable<MailRecipient> cc, IEnumerable<MailRecipient> bcc, IEnumerable<Attachment> attachments)
public async Task SendAsync(string message, string subject, IEnumerable<MailRecipient> to, MailRecipient from, MailRecipient replyTo, MailRecipient sender, IEnumerable<MailRecipient> cc, IEnumerable<MailRecipient> bcc, IEnumerable<Attachment> attachments)
{
await this._sendAsyncFunc(
message, subject, to, this._globalFrom ?? from, replyTo, cc, bcc, attachments
message, subject, to, this._globalFrom ?? from, replyTo, sender, cc, bcc, attachments
);
}
}
Expand Down
3 changes: 2 additions & 1 deletion Src/Coravel.Mailer/Mail/Mailers/FileLogMailer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ public async Task<string> RenderAsync<T>(Mailable<T> mailable)
return await mailable.RenderAsync(this._renderer, this);
}

public async Task SendAsync(string message, string subject, IEnumerable<MailRecipient> to, MailRecipient from, MailRecipient replyTo, IEnumerable<MailRecipient> cc, IEnumerable<MailRecipient> bcc, IEnumerable<Attachment> attachments = null)
public async Task SendAsync(string message, string subject, IEnumerable<MailRecipient> to, MailRecipient from, MailRecipient replyTo, MailRecipient sender, IEnumerable<MailRecipient> cc, IEnumerable<MailRecipient> bcc, IEnumerable<Attachment> attachments = null)
{
from = this._globalFrom ?? from;

Expand All @@ -39,6 +39,7 @@ await writer.WriteAsync($@"
To: {CommaSeparated(to)}
From: {DisplayAddress(from)}
ReplyTo: {DisplayAddress(replyTo)}
Sender: {DisplayAddress(sender)}
Cc: {CommaSeparated(cc)}
Bcc: {CommaSeparated(bcc)}
Attachment: { (attachments is null ? "N/A" : string.Join(";", attachments.Select(a => a.Name))) }
Expand Down
14 changes: 12 additions & 2 deletions Src/Coravel.Mailer/Mail/Mailers/SmtpMailer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ public async Task SendAsync<T>(Mailable<T> mailable)
await mailable.SendAsync(this._renderer, this);
}

public async Task SendAsync(string message, string subject, IEnumerable<MailRecipient> to, MailRecipient from, MailRecipient replyTo, IEnumerable<MailRecipient> cc, IEnumerable<MailRecipient> bcc, IEnumerable<Attachment> attachments = null)
public async Task SendAsync(string message, string subject, IEnumerable<MailRecipient> to, MailRecipient from, MailRecipient replyTo, MailRecipient sender, IEnumerable<MailRecipient> cc, IEnumerable<MailRecipient> bcc, IEnumerable<Attachment> attachments = null)
{
var mail = new MimeMessage();

Expand All @@ -66,6 +66,11 @@ public async Task SendAsync(string message, string subject, IEnumerable<MailReci
SetReplyTo(replyTo, mail);
}

if (sender != null)
{
SetSender(sender, mail);
}

using (var client = new SmtpClient())
{
client.ServerCertificateValidationCallback = this._certCallback;
Expand Down Expand Up @@ -121,14 +126,19 @@ private static void SetRecipients(IEnumerable<MailRecipient> to, MimeMessage mai

private void SetFrom(MailRecipient @from, MimeMessage mail)
{
mail.From.Add(AsMailboxAddress(this._globalFrom ?? @from));
mail.From.Add(AsMailboxAddress(@from ?? this._globalFrom));
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting change. I would leave out of this MR since it would require, I think, a different discussion around what the expect behavior should be, etc.

Right now, this is the expected behavior and many users could rely on this usage.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was a change from one of your previous issues that you ok'ed and requested them to create a PR for this work. They did not, so I added it to this one. As we as well were very confused on why when we manually set the from address in a single mail, it would always use the global default. I mentioned this in the PR notes. Have From() override default global From. #321

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would also debate that you cannot use this feature without this change, because if you apply the global from, you can never set the from in the mail.

}

private static void SetReplyTo(MailRecipient replyTo, MimeMessage mail)
{
mail.ReplyTo.Add(AsMailboxAddress(replyTo));
}

private static void SetSender(MailRecipient sender, MimeMessage mail)
{
mail.Sender = AsMailboxAddress(sender);
}

private static MailboxAddress AsMailboxAddress(MailRecipient recipient) =>
new MailboxAddress(recipient.Name, recipient.Email);

Expand Down
2 changes: 1 addition & 1 deletion Src/Coravel/Coravel.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<TargetFramework>.net6.0</TargetFramework>
<TargetFramework>netstandard2.1</TargetFramework>
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't run this, but pretty sure that this would break when running tests or at runtime. We need to target .net6 due to features that exist in .net6 and not in .netstandard2.1.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The 2 needed code style changes below were made to be able to target the netstandard.

<PackageId>Coravel</PackageId>
<Version>5.0.2</Version>
<Authors>James Hickey</Authors>
Expand Down
84 changes: 43 additions & 41 deletions Src/Coravel/Scheduling/Schedule/EnsureContinuousSecondTicks.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,44 +3,46 @@
using System.Linq;
using Coravel.Scheduling.Schedule.Helpers;

namespace Coravel.Scheduling.Schedule;

public class EnsureContinuousSecondTicks
{
private DateTime previousTick;

public EnsureContinuousSecondTicks(DateTime firstTick)
{
previousTick = firstTick;
}

/// <summary>
/// Give this method when the next tick occurs and it will return any intermediary ticks that should
/// have existed been the stored previous tick and the next one.
/// </summary>
/// <param name="nextTick"></param>
/// <returns></returns>
public IEnumerable<DateTime> GetTicksBetweenPreviousAndNext(DateTime nextTick)
{
// Starting at previousTick, we move ahead one second a time and record the next time until we get to the "nextTick".
// Then we check if there are any missed ticks between the two.
List<DateTime> missingTicks = null; // We don't want to commit any memory until we know for sure there's at least 1 missed tick.
DateTime nextTickToTest = previousTick.PreciseUpToSecond().AddSeconds(1);
while (nextTickToTest < nextTick.PreciseUpToSecond())
{
if (missingTicks is null)
{
missingTicks = new List<DateTime>();
}
missingTicks.Add(nextTickToTest);
nextTickToTest = nextTickToTest.PreciseUpToSecond().AddSeconds(1);
}

return missingTicks ?? Enumerable.Empty<DateTime>();
}

public void SetNextTick(DateTime nextTick)
{
previousTick = nextTick;
}
}
namespace Coravel.Scheduling.Schedule
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we undo this file's styling changes? Don't think it's necessary and I think would be best in a dedicated "styling fix" PR vs. combining logic changes, styling changes, bug fix, etc. in one PR 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was causing it to need to point to .net6, with this style revert, it now can target .netstandard

{

public class EnsureContinuousSecondTicks
{
private DateTime previousTick;

public EnsureContinuousSecondTicks(DateTime firstTick)
{
previousTick = firstTick;
}

/// <summary>
/// Give this method when the next tick occurs and it will return any intermediary ticks that should
/// have existed been the stored previous tick and the next one.
/// </summary>
/// <param name="nextTick"></param>
/// <returns></returns>
public IEnumerable<DateTime> GetTicksBetweenPreviousAndNext(DateTime nextTick)
{
// Starting at previousTick, we move ahead one second a time and record the next time until we get to the "nextTick".
// Then we check if there are any missed ticks between the two.
List<DateTime> missingTicks = null; // We don't want to commit any memory until we know for sure there's at least 1 missed tick.
DateTime nextTickToTest = previousTick.PreciseUpToSecond().AddSeconds(1);
while (nextTickToTest < nextTick.PreciseUpToSecond())
{
if (missingTicks is null)
{
missingTicks = new List<DateTime>();
}
missingTicks.Add(nextTickToTest);
nextTickToTest = nextTickToTest.PreciseUpToSecond().AddSeconds(1);
}

return missingTicks ?? Enumerable.Empty<DateTime>();
}

public void SetNextTick(DateTime nextTick)
{
previousTick = nextTick;
}
}
}
2 changes: 1 addition & 1 deletion Src/Coravel/Scheduling/Schedule/Event/ScheduledEvent.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ public async Task InvokeScheduledEvent(CancellationToken cancellationToken)
}
else
{
await using AsyncServiceScope scope = new(this._scopeFactory.CreateAsyncScope());
await using AsyncServiceScope scope = new AsyncServiceScope(this._scopeFactory.CreateAsyncScope());
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure why this was formatted this way? Could we revert this one please 🙂.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was causing it to need to point to .net6, with this style revert, it now can target .netstandard

if (GetInvocable(scope.ServiceProvider) is IInvocable invocable)
{
if (invocable is ICancellableInvocable cancellableInvokable)
Expand Down
8 changes: 4 additions & 4 deletions Src/UnitTests/MailerUnitTests/Mail/CustomMailerTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ public class CustomMailerTests
[Fact]
public async Task CustomMailerSucessful()
{
async Task SendMailCustom(string message, string subject, IEnumerable<MailRecipient> to, MailRecipient from, MailRecipient replyTo, IEnumerable<MailRecipient> cc, IEnumerable<MailRecipient> bcc, IEnumerable<Attachment> attachments)
async Task SendMailCustom(string message, string subject, IEnumerable<MailRecipient> to, MailRecipient from, MailRecipient replyTo, MailRecipient sender, IEnumerable<MailRecipient> cc, IEnumerable<MailRecipient> bcc, IEnumerable<Attachment> attachments)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we also add some tests against the subject in GeneralMailTests and SmtpMailerTests?

Copy link
Contributor Author

@johnwc johnwc Mar 11, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't think I changed the subject, is this just an ad hoc request to add this while I am doing the other test? 🙂

{
Assert.Equal("test", subject);
Assert.Equal("from@test.com", from.Email);
Expand All @@ -41,7 +41,7 @@ await mailer.SendAsync(
[Fact]
public async Task CustomMailer_GlobalFrom()
{
async Task SendMailCustom(string message, string subject, IEnumerable<MailRecipient> to, MailRecipient from, MailRecipient replyTo, IEnumerable<MailRecipient> cc, IEnumerable<MailRecipient> bcc, IEnumerable<Attachment> attachments)
async Task SendMailCustom(string message, string subject, IEnumerable<MailRecipient> to, MailRecipient from, MailRecipient replyTo, MailRecipient sender, IEnumerable<MailRecipient> cc, IEnumerable<MailRecipient> bcc, IEnumerable<Attachment> attachments)
{
Assert.Equal("global@test.com", from.Email);
Assert.Equal("Global", from.Name);
Expand All @@ -66,7 +66,7 @@ await mailer.SendAsync(
[Fact]
public async Task CustomMailer_Render()
{
async Task SendMailCustom(string message, string subject, IEnumerable<MailRecipient> to, MailRecipient from, MailRecipient replyTo, IEnumerable<MailRecipient> cc, IEnumerable<MailRecipient> bcc, IEnumerable<Attachment> attachments)
async Task SendMailCustom(string message, string subject, IEnumerable<MailRecipient> to, MailRecipient from, MailRecipient replyTo, MailRecipient sender, IEnumerable<MailRecipient> cc, IEnumerable<MailRecipient> bcc, IEnumerable<Attachment> attachments)
{
await Task.CompletedTask;
};
Expand All @@ -92,7 +92,7 @@ async Task SendMailCustom(string message, string subject, IEnumerable<MailRecipi
[Fact]
public async Task CustomMailerHasAttachments()
{
async Task SendMailCustom(string message, string subject, IEnumerable<MailRecipient> to, MailRecipient from, MailRecipient replyTo, IEnumerable<MailRecipient> cc, IEnumerable<MailRecipient> bcc, IEnumerable<Attachment> attachments)
async Task SendMailCustom(string message, string subject, IEnumerable<MailRecipient> to, MailRecipient from, MailRecipient replyTo, MailRecipient sender, IEnumerable<MailRecipient> cc, IEnumerable<MailRecipient> bcc, IEnumerable<Attachment> attachments)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just like this test, could we add a new dedicated test to ensure that the sender was dealt with successfully?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will see what I can do to add a test for sender based on this current test.

{
Assert.Equal(2, attachments.Count());
Assert.Equal("Attachment 2", attachments.Skip(1).Single().Name);
Expand Down