feat: add request info to verification emails

[jozefrebjak]

It is common practice to include in email verification emails some user request info like IP Address, User Agent Device & Date when a user is asked for some code. This PR adds this information.

I also improved default email templates to improve readability of the code.

@lonnieezell @kenjis @MGatner I don't know if we want to provide more complex Email templates, or if basic HTML is fine for the start of using this library and leave it completely to the developer.

Language files are not completed, some translated with Google Translator.

Please help with translations.

Here is what it looks like now in the Email client.

[kenjis]

some translated with Google Translator.

Can we use translations by Google Translator in this project? Do we have copyright of the translation? And we can certify the following? https://github.com/codeigniter4/CodeIgniter4/blob/develop/contributing/DCO.md

[datamweb]

Hello @jozefrebjak , Thank you for sending PR.

I agree to the following terms with this PR:

IP Address, User Agent Device & Date

Please add Time.

I also improved default email templates to improve readability of the code. I don't know if we want to provide more complex Email templates, or if basic HTML is fine for the start of using this library and leave it completely to the developer.

I am against adding and changing the default templates, the reason is that in a real project each site has its own theme and the email templates are the same as the site theme, so I believe that in the real project the email templates are also The face is designed exclusively and we do not need to add details in the shield.

Please help with translations.

In the final stages of this PR, we can get help from our friends for translation. Therefore, we must first see what the members think.

Refactor In my opinion, this PR should be implemented by View Decorators. I believe it makes us write less code and provides better flexibility. I want to hear from other members here?

[MGatner]

Can we use translations by Google Translator

An intriguing question! I did some digging, and the answer is murky but "yes". Here is an excerpt from WikiMedia's legal team who had to answer this for their services:

Assuming that such a mechanical translation is a protected derivative work ... the resulting copyright owner would be the user of the software, not the software manufacturer. ... Since the Supreme Court defined “author” as “he to whom anything owes its origin,”[7] it follows that person who initiated the translation would be the author.

So two interpretations:

1. The translation is too shallow a change to count as a derivative work so copyright belongs solely to the originating author
2. The translation is derivative but is done via automation so the copyright is the author's but the license for the derivative version belongs to the user of Google Translate

In both cases since @jozefrebjak is the originating author and the one using the translation software he is the sole owner of the content and may submit it based on our license and agreement.

[kenjis]

@MGatner Very interesting opinion. Can you give the URL of the WikiMedia's legal team answer?

[MGatner]

Here ya go: https://meta.m.wikimedia.org/wiki/Wikilegal/Copyright_for_Google_Translations

Specifically "Copyright in Translations"

[kenjis]

Please fix the failed test: 1) Tests\Controllers\ActionsTest::testEmailActivateShow

[kenjis]

Untranslated language files cause test failures. What if you put some marks as follows for the ones that cannot be translated?

--- a/src/Language/fa/Auth.php
+++ b/src/Language/fa/Auth.php
@@ -74,10 +74,10 @@
     'resetTokenExpired'         => 'متاسفانه، توکن بازنشانی شما منقضی شده است.',
 
     // Email Globals
-    'emailInfo'      => 'Some information about the person who asked for the code:',
-    'emailIpAddress' => 'IP Address:',
-    'emailDevice'    => 'Device:',
-    'emailDate'      => 'Date:',
+    'emailInfo'      => '(To be translated) Some information about the person who asked for the code:',
+    'emailIpAddress' => '(To be translated) IP Address:',
+    'emailDevice'    => '(To be translated) Device:',
+    'emailDate'      => '(To be translated) Date:',
 
     // 2FA
     'email2FATitle'       => 'احراز هویت دو عاملی',

[jozefrebjak]

Untranslated language files cause test failures. What if you put some marks as follows for the ones that cannot be translated?

--- a/src/Language/fa/Auth.php
+++ b/src/Language/fa/Auth.php
@@ -74,10 +74,10 @@
     'resetTokenExpired'         => 'متاسفانه، توکن بازنشانی شما منقضی شده است.',
 
     // Email Globals
-    'emailInfo'      => 'Some information about the person who asked for the code:',
-    'emailIpAddress' => 'IP Address:',
-    'emailDevice'    => 'Device:',
-    'emailDate'      => 'Date:',
+    'emailInfo'      => '(To be translated) Some information about the person who asked for the code:',
+    'emailIpAddress' => '(To be translated) IP Address:',
+    'emailDevice'    => '(To be translated) Device:',
+    'emailDate'      => '(To be translated) Date:',
 
     // 2FA
     'email2FATitle'       => 'احراز هویت دو عاملی',

Good idea how to handle missing languages.

[jozefrebjak]

Hello @jozefrebjak , Thank you for sending PR.

I agree to the following terms with this PR:

IP Address, User Agent Device & Date

Please add Time.

I also improved default email templates to improve readability of the code. I don't know if we want to provide more complex Email templates, or if basic HTML is fine for the start of using this library and leave it completely to the developer.

I am against adding and changing the default templates, the reason is that in a real project each site has its own theme and the email templates are the same as the site theme, so I believe that in the real project the email templates are also The face is designed exclusively and we do not need to add details in the shield.

Please help with translations.

In the final stages of this PR, we can get help from our friends for translation. Therefore, we must first see what the members think.

Refactor In my opinion, this PR should be implemented by View Decorators. I believe it makes us write less code and provides better flexibility. I want to hear from other members here?

For me is date enough, but I can change this to return also time.

Time::now()->toLocalizedString('MMM d, yyyy');

I'm fine with html email like they are now. I have added for now only a few basic HTML tags, which I believe we should support in default email templates.

I'd like to learn how to use View Decorators to provide some kind of templating system, but I don't have experience with that yet.

[datamweb]

For me is date enough, but I can change this to return also time.

Please add, maybe this is better. toDateTimeString

I'm fine with html email ...

Okay. Because you have made some changes, the test has an error, please correct the following to fix it. https://github.com/codeigniter4/shield/blob/5c1a9afff601d5e87f57311c7e861715c62d7130/tests/Controllers/ActionsTest.php#L233-L241 more info see : https://github.com/codeigniter4/shield/actions/runs/3061616247/jobs/4941607181

I'd like to learn how to use View Decorators ...

In this regard, since other members did not comment, there is no need to change. But if you want to learn, watch this video.

[datamweb]

@ddevsr thank you for your help. You can register a change request in the review. please see here:

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/reviewing-proposed-changes-in-a-pull-request

[ddevsr]

@ddevsr thank you for your help. You can register a change request in the review. please see here:

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/reviewing-proposed-changes-in-a-pull-request

@datamweb Thankyou for information how to request changes

[datamweb]

Hello @jozefrebjak , can you continue this PR?

[datamweb]

Some of your commits are unsigned, please check GPG-Signing Old Commits, also note other comments.

[jozefrebjak]

Some of your commits are unsigned, please check GPG-Signing Old Commits, also note other comments.

@datamweb I signed the latest commit, but the older ones were not signed. The rebase method is completely new to me, so if you provide me with how to sign them, I will.

[kenjis]

I recommend you set git sign automatically.

As a faster alternative, you can still securely sign commits without the -S option in git commit by setting git config --global commit.gpgsign true and git config --global user.signingkey FA4CD0840816FD28 to all local repositories. Without the --global option, the change is applied to one local repository only. https://github.com/codeigniter4/CodeIgniter4/blob/develop/contributing/workflow.md#gpg-signing-old-commits

Then, just to run git rebase signs all commits.

$ git fetch upstream
$ git rebase upstream/develop

[jozefrebjak]

I recommend you set git sign automatically.

As a faster alternative, you can still securely sign commits without the -S option in git commit by setting git config --global commit.gpgsign true and git config --global user.signingkey FA4CD0840816FD28 to all local repositories. Without the --global option, the change is applied to one local repository only. https://github.com/codeigniter4/CodeIgniter4/blob/develop/contributing/workflow.md#gpg-signing-old-commits

Then, just to run git rebase signs all commits.

$ git fetch upstream
$ git rebase upstream/develop

What I did

git fetch upstream
git rebase upstream/develop
git config pull.rebase false

and run sync from vscode.

is it ok now?

[kenjis]

and run sync from vscode.

Does it run git merge? It seems this branch was broken.

1. there are merge commits.
2. the line is NOT straight.
3. there are some commits having the same commit messages.

[kenjis]

I got it.

The last merge commit broke the branch.

• 2b86f82 - (HEAD -> jozefrebjak/develop) Merge branch 'develop' of github.com:jozefrebjak/shield into develop [2022-10-06 12:09:13 +0200] <Jozef Rebjak>

All you need to do is:

1. reset the last commit

   $ git reset HEAD^
   

2. force push

Resetting the last commit makes the git log graph straight.

* 8e958ae - (HEAD -> jozefrebjak/develop) Fix testEmailActivateShow [2022-10-06 12:08:48 +0200] <Jozef Rebjak>
* 60f14a5 - Update src/Language/fr/Auth.php [2022-10-06 12:08:48 +0200] <Jozef Rebjak>
* 8b4af59 - Update src/Language/id/Auth.php [2022-10-06 12:08:48 +0200] <Jozef Rebjak>
* ee9ec4f - fix fr translate [2022-10-06 12:08:48 +0200] <Jozef Rebjak>
* 0526ada - tests fix [2022-10-06 12:08:48 +0200] <Jozef Rebjak>
* dd635e3 - Update src/Views/Email/email_2fa_email.php [2022-10-06 12:08:48 +0200] <Jozef Rebjak>
* d71564c - Update src/Language/ja/Auth.php [2022-10-06 12:08:48 +0200] <Jozef Rebjak>
* f3baef9 - feat: add request info to emails [2022-10-06 12:08:48 +0200] <Jozef Rebjak>
*   f4cdfb6 - (upstream/develop, test, develop) Merge pull request #464 from datamweb/fix-property-last_used_at [2022-10-05 13:41:44 +0330] <Pooya Parsa Dadashi>

[datamweb]

hello @jozefrebjak, Please address the mentioned items, including Time and magic link. What do you think about them?

[jozefrebjak]

hello @jozefrebjak, Please address the mentioned items, including Time and magic link. What do you think about them?

@datamweb This PR was about verification emails, I would like to make another PR about the magic link email.

[jozefrebjak]

Sorry for the delay, Google translator is limited here. The unit test doesn't fully cover it, however I don't mind.

Also note that we are in the process of releasing a new version, if possible send out the magic link PR as soon as possible. (Honestly, I believe that should have been done in this PR as well.) Thank you.

@datamweb Ok, please leave it open, I'll add support for magic link as well.

[jozefrebjak]

@datamweb, @kenjis

Here is the final emails:

Register

2FA

Magic Link

[kenjis]

All checks have passed. I'm merging.