Add a canonical path for contact message

+1 to this. This could have some test love. Just a nitpick.

+++ b/contact_storage.module
@@ -93,12 +118,15 @@ function contact_storage_entity_type_alter(array &$entity_types) {
+  $entity_types['contact_message']->setHandlerClass('route_provider',  array('html' => 'Drupal\contact_storage\Entity\MessageRouteProvider'));

We can use class constant here.

1. +++ b/contact_storage.module
   @@ -80,6 +82,29 @@ function contact_storage_entity_base_field_info(\Drupal\Core\Entity\EntityTypeIn
   + *  Implements hook_entity_base_field_info_alter().
   ...
   + *  Implements hook_entity_operation_alter().
   

   please remove extra spaces after * on next re-roll

2. +++ b/src/Controller/MessageViewController.php
   @@ -0,0 +1,36 @@
   +    return 'View message';
   

   $this->t()

3. +++ b/src/Entity/MessageRouteProvider.php
   @@ -0,0 +1,41 @@
   +    $route = (new Route('/admin/structure/contact/messages/{contact_message}'))
   

   Is that really needed after https://www.drupal.org/node/2593577

1. +++ b/contact_storage.module
   @@ -80,6 +82,33 @@ function contact_storage_entity_base_field_info(\Drupal\Core\Entity\EntityTypeIn
   +  if ($entity->getEntityTypeId() == 'contact_message') {
   +    $operations['view'] = array(
   

   Should we check for view access here?

2. +++ b/src/Controller/MessageViewController.php
   @@ -0,0 +1,36 @@
   +  public function view(EntityInterface $contact_message, $view_mode = 'full') {
   +    $build = parent::view($contact_message, $view_mode);
   +    return $build;
   +  }
   

   This is needed because of the variable matching, right? kinda annoying.

3. +++ b/src/Controller/MessageViewController.php
   @@ -0,0 +1,36 @@
   +  public function title() {
   +    return 'View message';
   

   missing t() ?

4. +++ b/src/Entity/MessageRouteProvider.php
   @@ -0,0 +1,41 @@
   + * @file
   + * Contains \Drupal\contact_storage\Entity\NodeRouteProvider.
   + */
   

   oops.

5. +++ b/src/Entity/MessageRouteProvider.php
   @@ -0,0 +1,41 @@
   +  /**
   +   * {@inheritdoc}
   +   */
   +  public function getRoutes( EntityTypeInterface $entity_type) {
   +    $route_collection = new RouteCollection();
   

   would it be easier to get the other issue in first, so we get that automatically as soon as we have a canonical link template?

1. Yes, the context there is confusing but that's not dreditor's fault, that's how it is in the diff. You understood it correctly I think :)
2. Thinking about this again, why not use _entity_view in the route?
3. Because if it's a static title, then we can also just put it in _title in the route?

attached is a simple reroll based on the latest 8.x-1.x

#13 fixed the error i was getting when trying to view/edit a message using version 8.x-1.0-beta2:

The website encountered an unexpected error. Please try again later.

Drupal\Core\Entity\Exception\UndefinedLinkTemplateException: No link template 'canonical' found for the 'contact_message' entity type in Drupal\Core\Entity\Entity->toUrl() (line 219 of core/lib/Drupal/Core/Entity/Entity.php).
Drupal\Core\Entity\Entity->urlInfo()
content_translation_page_attachments(Array)
Drupal\Core\Render\MainContent\HtmlRenderer->invokePageAttachmentHooks(Array)
Drupal\Core\Render\MainContent\HtmlRenderer->prepare(Array, Object, Object)
Drupal\Core\Render\MainContent\HtmlRenderer->renderResponse(Array, Object, Object)
Drupal\Core\EventSubscriber\MainContentViewSubscriber->onViewRenderArray(Object, 'kernel.view', Object)
Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher->dispatch('kernel.view', Object)
Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object, 1)
Symfony\Component\HttpKernel\HttpKernel->handle(Object, 1, 1)
Drupal\Core\StackMiddleware\Session->handle(Object, 1, 1)
Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object, 1, 1)
Drupal\page_cache\StackMiddleware\PageCache->pass(Object, 1, 1)
Drupal\page_cache\StackMiddleware\PageCache->handle(Object, 1, 1)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object, 1, 1)
Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object, 1, 1)
Stack\StackedHttpKernel->handle(Object, 1, 1)
Drupal\Core\DrupalKernel->handle(Object)

But it doesn't add the new features planned on this issue:

make name, subject & mail base fields configurable on display
add a 'view' operation - I've been thinking whether overriding the list builder would be a better idea or adding a 'link to entity' formatter on the subject, can be debated of course.

Would also help with #2676552: File usage view breaks if an entity uses a file that has no canonical link template

@efraihn: While this would help with that problem, it's message that has to be fixed. Modules must check that an entity has a canonical link template before calling it.

These modifications are needed to make the module really cool. I was shocked to see it provides an "Edit" option but not a "View" one.

Thanks for all the work!

Combined patches -8 and -12 into a big one. Tested with 8.x-1.x-dev.

Ok, lets just do this and clean up routes/controllers in the other issue to let them be generated.

Couple of minor observations.

1. +++ b/src/Controller/MessageViewController.php
   @@ -0,0 +1,36 @@
   +  /**
   +   * {@inheritdoc}
   +   */
   +  public function view(EntityInterface $contact_message, $view_mode = 'full') {
   +    $build = parent::view($contact_message, $view_mode);
   +    return $build;
   +  }
   

   This seems redundant? We can just defer to the parent here yeah?

2. +++ b/src/Entity/MessageRouteProvider.php
   @@ -0,0 +1,41 @@
   + * Contains \Drupal\contact_storage\Entity\NodeRouteProvider.
   

   c/p error

1. The variable name needs to match. But not really sure why we actually need a custom controller, I think we can possibly remove the controller again when adding a default route provider.

@swentel yeah you're right, I just reviewed the latest file listed in the issue summary, sorry.

Have hidden all the irrelevant stuff now.

Credit update

+++ b/contact_storage.routing.yml
@@ -1,6 +1,14 @@
+    _permission: 'administer contact forms'

I think we need to use entity access here instead?

Which already defers to 'administer contact forms'.

But in reality, I think we need a new permissions 'view contact messsages'.

But that should be a new follow-up - 99% of the sites I work on we'd give low-level admins access to submitted messages. But 'administer contact forms' is a protected permission (because field create/edit access).

So pushed this as-is, will create that follow-up.

#2708809: Revisit permissions required to view contact messages is follow up