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.

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

feat: console command to send test email #3212

Merged
merged 25 commits into from
May 2, 2020
Merged

feat: console command to send test email #3212

merged 25 commits into from
May 2, 2020

Conversation

mechanarchy
Copy link

@mechanarchy mechanarchy commented Nov 14, 2019

This PR will close #3128, it will also close #3890. It adds the monica:test-email command, to test whether email delivery is working. Prior to this, I had to repeatedly attempt to send password reset emails for testing (as did user @theexplosivegroup).

The PR contains hard-coded strings: personally, I'm not sure there is value in translating them because they are strictly for testing purposes, and the content is largely irrelevant (you only care if you get the email).

Use as follows:

php artisan monica:test-email --email=my_email@provider.com

If no email is provided as an argument, the command will prompt you for one, but it does not verify that the email belongs to any registered user.

@sonarqubecloud
Copy link

SonarCloud Quality Gate failed.

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 0 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@asbiin
Copy link
Member

asbiin commented Apr 24, 2020

@mechanarchy is it possible you write a test for the command?

Copy link
Member

@asbiin asbiin left a comment

Choose a reason for hiding this comment

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

Hi!
I think that's a great idea.

It just missed some unit tests.

@mechanarchy
Copy link
Author

Sorry it took me a while - I haven't done a lot of PHP. But there are some basic tests now! Thanks for the approval :)

@asbiin
Copy link
Member

asbiin commented Apr 26, 2020

Really great job @mechanarchy ! Thanks a lot

Copy link
Member

@asbiin asbiin left a comment

Choose a reason for hiding this comment

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

See some improvements

app/Console/Commands/SendTestEmail.php Outdated Show resolved Hide resolved
app/Console/Commands/SendTestEmail.php Outdated Show resolved Hide resolved
@mechanarchy
Copy link
Author

Hi asbiin, I tried converting the string-variable concatenation into an interpolation like you suggested, but Psalm tests fails due to PossiblyInvalidCast non-empty-array<array-key, mixed> cannot be cast to string (see https://psalm.dev/190).

I looked at another command with a similar signature, and it is using string concatenation as well:

$email = $this->option('email');
$password = $this->option('password');
if (! empty($email) && ! empty($password)) {
Account::createDefault('John', 'Doe', $email, $password);
$this->info('| You can now sign in to your account:');
$this->line('| username: '.$email);

I couldn't find a way around this after some searching and trial and error, so I think it may have to stay as concatenation unless you know a solution.

Copy link
Member

@asbiin asbiin left a comment

Choose a reason for hiding this comment

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

That's a great job, thank you @mechanarchy

@asbiin asbiin merged commit c5c6946 into monicahq:master May 2, 2020
@mechanarchy mechanarchy deleted the 3128-send-email-from-terminal branch May 3, 2020 12:44
@github-actions
Copy link

github-actions bot commented May 4, 2021

This pull request has been automatically locked since there
has not been any recent activity after it was closed.
Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 4, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

command to test emails Add a console command which sends a test email
2 participants