CVS edit link for Matthew Davidson

I am currently creating a website to facilitate a local "virtual library". As part of this I want to be able to pull book and author metadata from http://openlibrary.org. For this purpose I have written a set of simple wrapper functions to make it easy to use the Open Library Restful API (http://openlibrary.org/dev/docs/restful_api) in Drupal. Simple though it is, it's probably the most generally useful code I've ever produced for Drupal, so it seemed worthwhile making the effort to make it a public project.

At the moment the API module enables developers to use all of the Open Library RESTful API, with the exception of the Save and Login calls (http://openlibrary.org/dev/docs/restful_api#save). These will be implemented in future, but as Save is currently regarded as an internal API, only functional from the Open Library localhost, and Login has no particular purpose except in the context of save, this is a low priority. I understand the Open Library developers are considering extending the API to securely allow remote saving.

The module caches retrieved data and cover images to be kind to the Open Library. At the moment the cache duration is set to one week, but in future versions this will be configurable.

In addition to the API, I've provided a set of theme functions, which will probably be sufficient for many developers to achieve what they want without having to use the lower level API functions or learn how Open Library RESTful API requests work in detail. The "block" display modes for Open Library book and author metadata have template files for ease of overriding.

I've also included another two modules which serve as examples for other developers, and are also intrinsically somewhat useful. The most substantial of these is a CCK display formatter for field types provided by the ISBN Field module (http://drupal.org/project/isbn). The project page for ISBN Field encourages development of such modules, so I'm confident I'm not treading on anybody's toes there.

The other "bonus" module is an input filter to insert book metadata into text fields. By itself this is a duplication of the Book Post module (http://drupal.org/project/bookpost), but the primary motivation for writing it was to show that project how to integrate their module with the Open Library API module to reduce the amount of code they need to maintain. It's a throwaway module which I'm quite happy to throw away should anybody object to it.

I've run all the code through the coder module, and have endeavoured to Doxygen comment at least all the functions intended for use by other developers. You can download the code at http://library.ourcoffs.org.au/code/openlibrary and for (ugly unstyled) examples of the output check out http://library.ourcoffs.org.au/node/2 and http://library.ourcoffs.org.au/code/examples/openlibrary-input-filter

CommentFileSizeAuthor
#8 openlibrary-6.x-0.3.tar_.gz7.31 KBMatthew Davidson
#6 openlibrary-6.x-0.2.tar_.gz7.3 KBMatthew Davidson
#1 openlibrary-6.x-0.1.tar_.gz13.89 KBMatthew Davidson

Comments

Matthew Davidson’s picture

Matthew Davidson commented
Status: Postponed (maintainer needs more info) » Needs review
StatusFileSize
new openlibrary-6.x-0.1.tar_.gz13.89 KB

Here's the code.

dawehner’s picture

dawehner
Primary language German
commented

Just one note.

  $img = theme_image($url, $variables['edition']->title .' cover', '', $attributes, FALSE);

Should be

  $img = theme('image', $url, $variables['edition']->title .' cover', '', $attributes, FALSE);

So you can override theme_image in your template.php.

But the rest looks quite good.

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Status: Needs review » Needs work

@dereine: If the OP is supposed to change the code, you should change the status of the issue.

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented 
  1.       if (strlen($id) == 13) {
        $args['isbn_13'] = $id;
      }
      elseif (strlen($id) == 10) {
        $args['isbn_10'] = $id;
      }

    Use the Drupal string functions, when available. 

  2.   t('ISBN numbers must have either 13 or 10 digits. Number provided: ') . $id

    In such cases, use a placeholder. 

  3.   theme_openlibrary_edition($element['#item']['value'], 'isbn', $style = 'title');

      theme('openlibrary_edition', $element['#item']['value'], 'isbn', $style = 'title');

  4. core = "6.x"
version = "6.x-0.1"
project = "openlibrary"
datestamp = "1247293507"

    That information is added by the packaging script; remove it from the .info file. 

  5.   require_once('openlibrary.theme.inc');

    The instruction will never find the file you are trying to include.
    It is better to put hook_theme() into the module, and remove the instruction to unconditionally include the other file; there is a way to instruct Drupal about which file the theme functions are.

  6. There is no match between the parameters for the theme functions as declared in hook_theme(), and the actual parameters used by some theme functions; the match must be complete, not only for the parameter names.
  7. The license file must be removed.
Matthew Davidson’s picture

Matthew Davidson commented

D'oh! What an embarrassing one. Fixed (posted below). Thanks.

Matthew Davidson’s picture

Matthew Davidson commented
Status: Needs work » Needs review
StatusFileSize
new openlibrary-6.x-0.2.tar_.gz7.3 KB

@KiamLaLuno Wow, thanks for the attention to detail. I had no idea how many obvious gaffs I'd made in a relatively small amount of code, and you've taught me a few things I didn't know. All these issues have been addressed now.

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Status: Needs review » Needs work

There is something to correct, and then we are ready.

  if (!$edition) {
    return t('Edition not found: ') . $id_type .':'. $id;
  }

Use the function placeholders.

Matthew Davidson’s picture

Matthew Davidson commented
Status: Needs work » Needs review
StatusFileSize
new openlibrary-6.x-0.3.tar_.gz7.31 KB

Done. Thanks again.

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Component: Miscellaneous » new project application
Assigned: Unassigned » avpaderno
Issue summary: View changes
Status: Closed (fixed) » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.