Email translation

At the moment they can't be, but I don't think it would be too difficult to make each email a translatable entity. I will add this to my to-do list (may be a few weeks) but would be happy if anyone is willing to contribute a patch to help.

This is important functionality for us on a site I'm building. I'm not positive when I'll have time to get to this, but I'll create a patch in the near future if there is not one already by then.

Thanks! I have given this a start, I will try to get my progress more complete and upload it here as a patch and outline the remaining tasks.

Here is my progress thus far. What is completed:

• Create the entity
• Migrate the data from the database table to the entity table

What is left to do:

• Update the controller to use the contact email entity
• Update the forms to use the contact email entity
• Update the contact email class of helper functions to use the entity
• Update the emailer service to send from the entity data
• Uncomment the database delete line in the hook install

And finally:

• Test that it can actually then be translated.

Awesome! Migrating the existing data is the part I have the least experience with, so I could definitely help with at least some of those other remaining tasks very soon if they're still open. Thanks for posting your progress on that so far!

Just to let you know, I'm almost finished with this.

The changes are pretty far-reaching, definitely affecting more than just adding translations, however I think they are all positive.

There may be a couple of regressions given that things work a bit differently in some cases. For example, I don't know the best way to have the message at the top of the contact email listing page indicating whether or not the default email settings are still in effect.

I need to finish a little bit more testing and will post a full patch for further review / discussion tomorrow.

Sounds good, thanks for the update and your efforts!

Here's my first try. I began with your patch, and ended up modifying just about every file in the module in some pretty large ways to try and do things the "entity" way. The end result is that the two services are much lighter, and some functionality has been moved out of those into other entity-related classes.

I'm hoping I haven't removed any functionality that existed before, other than the aforementioned message that I haven't figured out how to make work the same using a list builder class, though I'm sure there's an easy way to do it. After abstracting things out to the entity classes, it turned out the controller wasn't even needed anymore--though it could be added back if I've missed anything there.

I've tested fully using the new entity system, I think. There are a couple of quirks with the UX that can be enhanced, such as making sure to redirect to the proper list page (currently if you add a contact email from the main list, it'll redirect you to that contact form's list afterward).

I have not yet tested the update hook. I made a couple minor changes to it as I modified the "disabled" field to be a "status" field. I also changed the "contact_form" field to be an entity reference instead of a plain string, but I don't think that requires any changes to the update hook. I'm going to attempt some testing of the update process tonight yet.

I'll update this if I find any further issues. I'd appreciate if you could check it over and provide any feedback, as well. Thanks!

BTW, accessing the lists is slightly different with this patch as well. I figure I should clarify how to access the various pages.

1. Full list at Structure > Contact forms > Manage emails
2. Individual lists when you're editing any contact form using the "Manage emails" tab
3. Through the action button on the Structure > Contact forms listing page, like before

Sorry if I overstepped at all with those updates. I started with different routes entirely using the entity system so they didn't collide with the existing ones, but ended up liking where they are and leaving them. Feel free to move these things around as you see fit!

Just noticed an issue, currently when deleting it redirects to the homepage. Maybe it's necessary to override the entity delete form to specify a redirect, or perhaps there's a cleaner way to do it while still using the default entity delete form, I'm not positive yet.

I tested the update hook, and the good news is it works! Here's a new patch with a couple minor changes:

1. Added a "Contact form" column to the list page which solves a couple of issues. First, on the main list page it differentiates the rows so you know which form is for which email. Second, on the specific list pages, it makes it abundantly clear which form you're editing the emails for.

2. Added a line to the update hook to call entity updates first. Previously, you had to be sure to run drush entity-updates before the update hook or the new entity type schema wouldn't have been installed. I think that should solve that issue.

I should also mention, I've been so caught up just getting the whole entity system fully baked, I neglected to test that translations actually work properly.

I'm noticing that there currently isn't a Translate tab visible when editing a contact email, which I would guess to be a requirement?

I'm working on a new patch version now that completes the translation aspect of this patch. Running into a couple issues, but should have a new patch shortly, or in the morning.

Sounds good, looks like you've put a lot of work into it. Just uploading an interdiff to help me see what's changed and review easier. In general the approach of using the built in entity handlers should be fine; I'll review the UX and flow from core contact email to contact emails provided by this module in the process. Also makes sense to move the getBody, etc to the entity class as you've done. Will try to review in more detail once you feel it's ready for review (sounds like you are still working on it).

Sounds good, looks like you've put a lot of work into it.

Probably the biggest understatement :)

Thanks for the initial feedback!

I've resolved an issue with one route title, and finished making the ContactEmail entity type translatable. New patch attached.

Testing needs to be done to see how well the translations work now that they're in place, and if the proper email gets selected depending on the user's preference when submitting a contact form, etc.

I think this is ready for further review / discussion. Thanks again for taking a look!

It'd be nice if there were a "Translate" tab at the top of the page when editing a contact email. Normally I think that happens automatically when enabling translations, however it seems like there should be other actions showing up there as well (at least Edit). Perhaps there's something more that needs to be done to make local actions show up as tabs at the top?

From the hook_update_N manual page:

Never call \Drupal::entityDefinitionUpdateManager()::applyUpdates() in an update function, as it will apply updates for any module not only yours, which will lead to unpredictable results.

This should do the trick to instead just install it for the contact_email entity:

$entity_manager = \Drupal::entityTypeManager();
$update_manager = \Drupal::entityDefinitionUpdateManager();
foreach ($entity_manager->getDefinitions() as $entity_type) {
  if ($entity_type->id() == 'contact_email') {
    $update_manager->installEntityType($entity_type);
  }
}

I'm happy to of course carry on from where you leave off and implement stuff like that, not necessary to take the whole rework on your shoulders.

Great thanks for the new patch! Shall I take it from here for a bit? I don't want to start on it while you're in progress.

Thanks for the tip about that update hook issue! Also, that'd be great if you'd like to take it from here for now. I'd be happy to come back later to put in some additional effort on changes or fixes, but now's probably a good time for me to step back for a bit while you go over things and make some changes or provide some additional feedback about what else should be completed.

I appreciate your timely comments, and willingness to take on this beast of a patch!

I ran DB updates with Drush again to test this, but this time I didn't have as good of results.

Drupal\Core\Database\DatabaseExceptionWrapper: SQLSTATE[HY000]:      [error]
General error: 1364 Field 'status' doesn't have a default value:
INSERT INTO {contact_email} (uuid, langcode) VALUES
(:db_insert_placeholder_0, :db_insert_placeholder_1); Array
(
    [:db_insert_placeholder_0] =>
b917a69d-2fa6-4092-80e9-c2fb7b3a789f
    [:db_insert_placeholder_1] => en
)
 in
Drupal\Core\Entity\Sql\SqlContentEntityStorage->doSaveFieldItems()
(line 843 of
/code/core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorage.php).
SQLSTATE[HY000]: General error: 1364 Field 'status' doesn't have a   [error]
default value: INSERT INTO {contact_email} (uuid, langcode) VALUES
(:db_insert_placeholder_0, :db_insert_placeholder_1); Array
(
    [:db_insert_placeholder_0] =>
b917a69d-2fa6-4092-80e9-c2fb7b3a789f
    [:db_insert_placeholder_1] => en
)

Last time I tested, the upgrades were successful. It's possible there was some unrelated issue with my data that was being upgraded which caused the other fields to be missing. But, figured I would report this in case you or anyone else sees this as well.

Thanks for the info. It will take me a bit to get through this but I am making some progress (just don't it in my free time so time is a bit sparse) and I am trying to get tests in to ensure such a major change is non-breaking.

I haven't been getting through this as quick as hoped, but anyways uploading my progress here. Will keep assigned to me as I've only gone through a portion of the work and would like to build in more tests before actually moving ahead with this (but an upgrade test is there at least now).

Just an update on this. I am almost there with it - I still need to add test coverage for the reply-to and recipient's to ensure those continue to work, but otherwise I would consider this quite usable already. You can find the progress on the 8.x-1.x-translatable-entities branch found here.

Cool, thanks a lot for your continued efforts on this!

I'm a bit scared to switch over from my patch that something unexpected might break, but I'll give it a try in the near future.

Even if I need to uninstall it and re-create my emails afterward it wouldn't be a big deal, but I suppose I should wait on THAT aspect until it's in the official dev branch at least.

Happy to say this is now on the dev branch with a fair amount of test coverage including for the upgrade path. Thanks again for all the effort on your end as well getting this end - definitely a useful step!

Woohoo! Thanks for getting this functionality in! I've been using your other branch for a couple of days successfully now, so I anticipate it'll be a smooth transition to come back to the dev branch now.

Yep, should be smooth - only a few php warning/notice bug fixes - primarily just adding test coverage over the past few days. Glad to hear you've been using it successfully already!