Remove @todo in \Drupal\rest\Plugin\views\display\RestExport [#2541490]

Problem/Motivation

There is a leftover @todo in the \Drupal\rest\Plugin\views\display\RestExport class. From what I understand content negotiation is is set for the moment.

Proposed resolution

Remove the todo and clean up any related code.

Remaining tasks

Remove the todo and clean up any related code.

User interface changes

none

API changes

none

Data model changes

none

Comment	File	Size	Author
#1	remove_todo_in-2541490-1.patch	772 bytes	isholgueras
#1	8.0.x: PHP 5.5 & MySQL 5.5 13,976 pass

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

isholgueras CreditAttribution: isholgueras commented 24 September 2015 at 08:54

Status:

Needs work

» Needs review

File	Size
remove_todo_in-2541490-1.patch	772 bytes
8.0.x: PHP 5.5 & MySQL 5.5 13,976 pass

I've tested almost every functionality in content negotiation for REST Export. I've checked the code that I think is a related code and everything work well. So I removed only the @todo line.

Correct me if I'm wrong.

Comment #2

Novitsh CreditAttribution: Novitsh as a volunteer commented 24 September 2015 at 09:42

If indeed the TODO is not relevant anymore, than the patch provided in #1 is good to go.

Comment #3

Jaesin CreditAttribution: Jaesin at Chapter Three commented 24 September 2015 at 13:40

+++ b/core/modules/rest/src/Plugin/views/display/RestExport.php
@@ -126,7 +126,6 @@ public function initDisplay(ViewExecutable $view, array &$display, array &$optio
     $request_content_type = $this->view->getRequest()->getRequestFormat();
...
-    // @todo Remove the need for this when we have better content negotiation.
     if ($request_content_type != 'html') {
...
     }

I think the comment is saying to remove the if statement when content negotiation has improved to the point where $this->setContentType($this->view->getRequest()->getRequestFormat()) works without it. Meaning that a rest request should not return "html" in the first place unless there was a "html" serializer :(.