Add a HTTP response code area handler

Question. If the Area response is returning a status code, how do you control what the actual response body is? (Most response codes can have a legal body, unless the request was a HEAD request, but that's already handled by Symfony.)

Nitpicks!

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/area/HTTPStatusCode.phpundefined
@@ -0,0 +1,68 @@
+    // Add the http status code, so it's easier for people to find it.

+++ b/core/modules/views/lib/Drupal/views/Tests/Handler/AreaHTTPStatusCodeTest.phpundefined
@@ -0,0 +1,43 @@
+    // Change the http status code to 403.
...
+    // Test that the http response is "I'm a teapot".

s/http/HTTP

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/area/HTTPStatusCode.phpundefined
@@ -0,0 +1,68 @@
+    array_walk($options, function($title, $code) use(&$options) {
+      $title = t($title);
+      $options[$code] = t('@title (@code)', array('@title' => $title, '@code' => $code));
+    });

*whistles* That is nice :)

Except I'm not sure about t($title), can we get away with that?

+++ b/core/modules/views/lib/Drupal/views/Tests/Handler/AreaHTTPStatusCodeTest.phpundefined
@@ -0,0 +1,43 @@
+   * Test tha area handler.

Tests

+++ b/core/modules/views/lib/Drupal/views/ViewExecutable.phpundefined
@@ -320,7 +320,7 @@ class ViewExecutable {
-   * @var Symfony\Component\HttpFoundation\Response
+   * @var \Symfony\Component\HttpFoundation\Response
    */
   protected $response = NULL;
 
@@ -1575,7 +1575,7 @@ public function access($displays = NULL, $account = NULL) {

@@ -1575,7 +1575,7 @@ public function access($displays = NULL, $account = NULL) {
   /**
    * Sets the used response object of the view.
    *
-   * @param Symfony\Component\HttpFoundation\Response $response
+   * @param \Symfony\Component\HttpFoundation\Response $response
    *   The response object which should be set.
    */
   public function setResponse(Response $response) {
@@ -1585,7 +1585,7 @@ public function setResponse(Response $response) {

@@ -1585,7 +1585,7 @@ public function setResponse(Response $response) {
   /**
    * Gets the response object used by the view.
    *
-   * @return Symfony\Component\HttpFoundation\Response
+   * @return \Symfony\Component\HttpFoundation\Response

Out of scope ;)

t($title) is not sufficient as it doesn't make the strings translatable. Check with Gabor or Karen about how we handled timezone labels for Drupal 7. We'll have to do the same.

I think these are standard HTTP names, not sure why would we want to translate them. If we do want to translate them anyway, a list of the code names as t()ed strings somewhere in the code (no need for them to actually be executed ever, just for static code parsing) is sufficient.

Nitpick: It would probably look better with the numbers first, then the string, so that it aligns better. That's also how it is usually expressed in the HTTP response.

Agreed with Crell.

I think that takes care of everything except translation. It looks like currently it would always show the English version of the status text; whether or not that's acceptable I will defer to Gabor.

Cool. Then let's do that.

#26: drupal-1849356-26.patch queued for re-testing.

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/area/HTTPStatusCode.phpundefined
@@ -0,0 +1,67 @@
+    // Get all possible status code defined by symfony.

codes

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/display/Page.phpundefined
@@ -92,7 +92,10 @@ public function execute() {
+    $response = $this->view->getResponse();
+    $response->setContent(drupal_render_page($render));
+

I guess we should be doing this now anyway, nice.

+++ b/core/modules/views/lib/Drupal/views/Tests/Handler/AreaHTTPStatusCodeTest.phpundefined
@@ -0,0 +1,50 @@
+    // Change the HTTP status code to 403.

This comment has the wrong status code, do we want a test for 403? Or should we just change this to 418?

Happy to rtbc this again when they are done..

Looks good!

#33: drupal-1849356-33.patch queued for re-testing.

#37: drupal-1849356-37.patch queued for re-testing.

So this patch doesn't seem to be working. Most likely it broke somewhere along the line between mid-January and now. I set my Views header to output a 500 error and nothing happens on view, neither did it on a 404 or 403. So needs work for that.

However, I'm not sure about this feature in general for core. While I see why it's wanted, it seems like a *very* advanced thing that wouldn't make sense to at least 80% of users. And heck, leave alone 80% of users. I'm a full-fledged geek, and even I don't know what about 90% of these HTTP status codes are and why I would ever use them:

"425 (Reserved for WebDAV advanced collections expired proposal)"? Really?

PS: We shouldn't be double-escaping "It's a teapot"; the test for that is GREAT though. :D

And unfortunately, thanks to the magical power of the alphabet, this exceptionally geeky option is the very first thing that shows up in the list of stuff to add to a header/footer. :\

If we want this feature for core, it seems like it'd be much better to be just a simple 403/404 selector on the empty text plugin. I dunno, thoughts?

Hmm, I can see where you're coming from. I think I would prefer to keep the feature more as-is with the codes. Core does have some advanced features, but I also appreciate that alot of people will not use this anyway :) However, we would then have to re implement another handler that does exactly the same to include other status codes.

How about we put the 'popular' codes at the top of the list?

Let's not forget 200. ;) That should be at the top of the list...

However, I'm thinking having both a text field and a select box would be odd. Maybe just a text field, and do validation against Symfony's allowed values. (We could also use Symfony to provide a label when a status code is set.)

Perhaps a case for http://caniuse.com/#search=datalist

Back to rtbc per #40. I was able to change my front page response code to 500.

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/area/HTTPStatusCode.phpundefined
@@ -0,0 +1,71 @@
+    $options = array('500' => $options['500']) + $options;
+    $options = array('404' => $options['404']) + $options;
+    $options = array('403' => $options['403']) + $options;
+    $options = array('200' => $options['200']) + $options;

Instead of reassigning each time, we could just have one array that contains all 4 of these codes and then + $options. I know there was also talk of this above, but why do we want 200 in here?

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/area/HTTPStatusCode.phpundefined
@@ -0,0 +1,71 @@
+      $response = $this->view->getResponse();
+      $response->setStatusCode($this->options['status_code']);

We could just do this on one line and not bother assigning the variable?

+++ b/core/modules/views/tests/modules/views_test_config/test_views/views.view.test_http_status_code.ymlundefined
@@ -0,0 +1,82 @@
+human_name: test_http_status_code

na ah

+++ b/core/modules/views/views.views.incundefined
@@ -90,6 +90,14 @@ function views_views_data() {
+    'help' => t('Alter the HTTP response status code used by this view, mostly helpful for empty results.'),

This only really makes sense for empty results, no? we could add 'sub_type' => 'empty', to the views area data?

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/area/HTTPStatusCode.phpundefined
@@ -40,10 +40,11 @@ public function buildOptionsForm(&$form, &$form_state) {
     // Move 200/403/404/500 to the top.

Sorry, I guess 200 needs to go from here too.

The rest is looking great now. Just one thing...

The only thing is that I think we should just remove the 200 code form the list. It doesn't make any sense in there, as all responses we return will be 200 anyway. So really it is completely redundant. Removing it from the top of the list is a good start though :) This is not really blocking this patch (it's good).

+++ b/core/modules/views/tests/modules/views_test_config/test_views/views.view.test_http_status_code.yml
@@ -0,0 +1,82 @@
+api_version: '3.0'

+++ b/core/modules/views/tests/modules/views_test_config/test_views/views.view.test_http_status_code.yml
@@ -0,0 +1,82 @@
+disabled: '0'

Let's remove/change these too.

The only thing I'd be missing with this is the ability to set Location header with certain status codes.

Still needs work for double-escaping "It's a teapot"...

About the select list, what about simple grouping like 1xx Informational, 2xx Success, ..?

You are right. Back to rtbc.

#56: vdc-1849356-56.patch queued for re-testing.

I reviewed this an the code is code to go and from my side this should go in. @catch thinks that the use of an area handler is weird but this was discussed for the page title area handler and no better solution was found. Therefore...

Committed 5dba5f6 and pushed to 8.x. Thanks!

Dear folks who added this cool feature, RFC: #2959415: When a view uses the HTTPStatusCode handler to set a 404 or 403, throw the appropriate exception

Thanks,
-Derek