Is there any plan to release a Drupal 7 version of this module?

It looks like there is a 7.x-1.x branch in the repository, with some good progress being made, but there isn't a dev release for it available yet. How stable is it?

I may need to use this module in an upcoming D7 project, and I would be happy to help development of the 7.x branch. Is there a roadmap, or a TODO list for it?

Comments

Bevan’s picture

The only D7 plans are to get it working on D7.

Since it depends on implementations in order to be able to test it, the D7 branch has not been tested much. Toggl.com API and Redmine API module are the primary implementations with comprehensive test suites which test REST API Query API as well as each respective module. Porting at least one of these to D7 would allow REST API Query API to be tested quite thoroughly and a release to get out.

If you need REST API Query API on D7 now, try the dev branch from git. Much of it works, but it probably has bugs.

jcieslak’s picture

Hi

as I said earlier to Bevan - I've prepared a test 7.x branch independently from Bevan's 7.x implementation. You could have a look it works well with ChiliProject API.

jcieslak’s picture

And here's the patch.

Bevan’s picture

Status: Active » Needs work

jcieslak;

Thank you so much for your contribution! :)

I reviewed about half the patch but then stopped because I was beginning to repeat myself. Please clean up your code changes, check the modified code still matches the code style and review the patch file yourself to check it is clean, easy to review and only includes your changes.

Review

+++ b/includes/rest_api_query.class.incundefined
@@ -41,15 +41,14 @@ abstract class rest_api_query {
-    'Content-type' => 'application/json',
+  /* modified by jcieslak */
+//    'Content-Type' => 'application/x-www-form-urlencoded',
+    'Content-Type' => 'application/json',

Don't insert new lines that are commented out unless they include useful documentation.

Is there a difference between "Content-type" and Content-Type"?

+++ b/includes/rest_api_query.class.incundefined
@@ -41,15 +41,14 @@ abstract class rest_api_query {
-  // The timeout length for the request.  Large API responses can take a while to
-  // format.

Why is this removed?

+++ b/includes/rest_api_query.class.incundefined
@@ -81,7 +80,7 @@ abstract class rest_api_query {
-  public $cache_validity = 3600;
+  public $cache_validity = 0;

Why is the cache disabled? This does not seem valid for the D7 update.

+++ b/includes/rest_api_query.class.incundefined
@@ -137,7 +136,8 @@ abstract class rest_api_query {
-      if ($key = set_active_rest_api_key($this->rest_api_schema)) {
+      $key = set_active_rest_api_key($this->rest_api_schema);
+      if (isset($key)) {

This is an acceptable style change but is not related to D7 update.

+++ b/includes/rest_api_query.class.incundefined
@@ -149,14 +149,25 @@ abstract class rest_api_query {
-    if (!isset($this->{$name}) && method_exists($this, $method)) {
-      $this->{$name} = $this->$method();
+    if (!isset($this->name) && method_exists($this, $method)) {
+      $this->$name = $this->$method();

This changes the behaviour significantly and will break.

+++ b/includes/rest_api_query.class.incundefined
@@ -149,14 +149,25 @@ abstract class rest_api_query {
-    if (isset($this->{$name})) {
-      return $this->{$name};
-    }
+    /* Modified by jcieslak
+     * before: return $this->$name;
+     * after:
+     * ¶
+     * if($name != 'uses_api_key')
+	 *   return $this->$name;
+	 * else
+	 *   return NULL;
+     */
+    ¶
+    if($name != 'uses_api_key')
+	    return $this->$name;
+	else
+		return NULL;

This is bad style and does not make sense.

+++ b/includes/rest_api_query.class.incundefined
@@ -188,24 +199,30 @@ abstract class rest_api_query {
+      /* Modified jcieslak */
+      /* before:	$this->$property = $url[$key]; { */
+      /* after:
+       * if(!empty($url[$key])) {
+       * $this->$property = $url[$key];
+       * }
+       * */       ¶

Comments like this clutter the code and make it harder to understand. remove them.

+++ b/includes/rest_api_query.class.incundefined
@@ -188,24 +199,30 @@ abstract class rest_api_query {
+      if(!empty($url[$key])) {
+      	$this->$property = $url[$key];
       }

Standard code indentation in drupal is two space characters.

+++ b/includes/rest_api_query.class.incundefined
@@ -222,7 +239,8 @@ abstract class rest_api_query {
-    if (!$this->initialized || empty($this->scheme) || empty($this->host)) {
+
+  	if (!$this->initialized || empty($this->scheme) || empty($this->host)) {

Remove this change set it does nothing but break the conventional indentation.

+++ b/includes/rest_api_query.class.incundefined
@@ -279,8 +297,10 @@ abstract class rest_api_query {
+    ¶
     $url = implode('/', $url) . '.' . $this->format;
-
+    ¶
+    ¶

As above

jcieslak’s picture

OK I'll clean it up today and post the patch once again.