Change contact message entity to behave more like a normal entity

Hi corvus_ch, any progress on this?
If not willing to pick it up

No, not really. Thought I will be able to do it last weekend but…

So go ahead.

There's some tricks with message formatting in related issue.
+1 to add language field

looking now

Works, to quote @berdir

so we just need a views fairy to write the views data generator and then we can provide a view of sent contact messages

but that's another issue

So this makes it possible to provide contact message storage, with a test implementation and tests to boot

Addresses #7

yeah, ContentEntitySchemaHandler::processIdentifierSchema() converts int to SERIAL which is an alias for BIGINT UNSIGNED NOT NULL AUTO_INCREMENT UNIQUE, so we're good there.

Fixes #10

Nice work :)

1. +++ b/core/modules/contact/tests/modules/contact_storage_test/contact_storage_test.install
   @@ -0,0 +1,22 @@
   +function contact_storage_test_install() {
   

   I don't think this is necessary and if it is we have a bug :)

   Entity schema should already be installed/uninstalled in the module handler.

2. +++ b/core/modules/contact/tests/modules/contact_storage_test/contact_storage_test.module
   @@ -0,0 +1,43 @@
   +      // Explicitly set this to 'contact' so that
   +      // ContentEntityDatabaseStorage::usesDedicatedTable() doesn't attempt to
   +      // put the ID in a dedicated table.
   +      ->setProvider('contact')
   

   This is weird, a base field should never end up in a dedicated table. This is surely taken care in #1498720: [meta] Make the entity storage system handle changes in the entity and field schema definitions, so if this necessary now, let's add a todo to remove this hack, also because the provider property is supposed to be set only by the Entity Manager.

1. Yep it is needed because like berdir said, provide mismatch
2. Same, provider mismatch forced id into a dedicated table

So not sure if there is actually anything to do here - so bumping back to needs review

Adds todo referencing #1498720: [meta] Make the entity storage system handle changes in the entity and field schema definitions

@berdir:

Spoke with @larowlan in IRC: both point make sense, but they should be fixed by #1498720: [meta] Make the entity storage system handle changes in the entity and field schema definitions, so let's add a todo about removing the related "hacks".

The problem is how we define if it's a base field or not

I'd say a base field is also any field defined through hook_entity_base_field_info() :)
-> Any field returned by EntityManagerInterface::getBaseFieldDefinitions().

Added second @todo

Looks RTBC to me, thanks.

1. +++ b/core/modules/contact/src/MessageForm.php
   @@ -74,6 +76,15 @@ public function form(array $form, array &$form_state) {
   +    $language_configuration = \Drupal::moduleHandler()->invoke('language', 'get_default_configuration', array('contact_message', $message->getCategory()->id()));
   

   Can we inject?

2. +++ b/core/modules/contact/src/MessageForm.php
   @@ -254,5 +265,26 @@ public function save(array $form, array &$form_state) {
   +      $language_content = \Drupal::languageManager()->getCurrentLanguage(LanguageInterface::TYPE_CONTENT);
   

   Can we inject?

Fixes #20 and a few other injection issues in MessageForm
there is no OO equiv of drupal_mail, drupal_set_message and user_cookie_save().
user_cookie_save() call can go when #749748: Contact, register and comment forms do not prefill with user info from browser is ready (its close) so added a todo and a note over there.

Looks good

The module handler is already available through setter injection in the EntityForm class...

Fixes #23
We also have currentUser() from FormBase

Looks good, thanks

Just a nitpick

+++ b/core/modules/contact/src/MessageForm.php
@@ -171,18 +192,19 @@ public function preview(array $form, array &$form_state) {
+    $user = $this->currentUser();

@@ -248,11 +270,32 @@ public function save(array $form, array &$form_state) {
+    if ($message->isPersonal() && $this->currentUser()->hasPermission('access user profiles')) {

second line could re-use $user too

fixes 26

git ac https://www.drupal.org/files/issues/contact-storage-2289063.27.patch
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100 14903  100 14903    0     0  14743      0  0:00:01  0:00:01 --:--:-- 15507
error: patch failed: core/modules/contact/src/Entity/Message.php:154
error: core/modules/contact/src/Entity/Message.php: patch does not apply

straight re-roll

@Berdir Do you mean that hunk?

+++ b/core/modules/contact/src/MessageForm.php
@@ -71,9 +83,18 @@ public function form(array $form, array &$form_state) {
+    $language_configuration = $this->moduleHandler->invoke('language', 'get_default_configuration', array('contact_message', $message->getCategory()->id()));
+    $form['langcode'] = array(
...
+      '#default_value' => $message->getUntranslated()->language()->id,

@@ -186,7 +208,7 @@ public function save(array $form, array &$form_state) {
       // Send to the user in the user's preferred language.

Suppose we need @plach here

Also there's related issue #1938178: Contact module categories has not support language select
So probably we can drop that here

Committed 1793df6 and pushed to 8.x. Thanks!