Port ZipCart to Drupal 7

http://drupal.org/project/zipcart/git-instructions has instructions to create a patch (clone this repo, drop your D7 code in, then do "Creating a patch").

If you're not familiar with Git, you can just post your D7 code here as a tarball too. I won't mind much. But give patching a try first if you can :D

Thanks!

This looks really good, guys. Thanks - and my apologies, I thought I'd already done this.

I'm going to test a re-roll with the shadowbox elements pulled out and commit that to 7.x as I prefer not to tie this module to a particular theme or design implementation; IMO that should be done in the theme layer (and ideally this module should be flexible enough to support that happening).

Should have a release for you in an hour or so :)

Sorry ... there's a bit of work to do here yet. I want some of the other ideas introduced here to come in via separate issues.

+++ b/README.txt
@@ -9,16 +9,11 @@ want to give users the capacity to select several files for download, then downl
-DEMO
-
-Take a look at http://zipcart.demo.giantrobot.co.nz/ to get an idea of how this works.
-

I reinstated this at the bottom of the README. I think a demo is useful for people. Added a note that the current demo is D6.

+++ b/zipcart.admin.inc
@@ -14,30 +16,127 @@ function zipcart_settings_form() {
+  $form['zipcart_file_dirs'] = array(
+    '#title' => t('Valid File Directories'),
+    '#type' => 'textarea',
+    '#cols' => 30,
+    '#rows' => 3,
+    '#default_value' => variable_get('zipcart_file_dirs', 'zipcart'),
+    '#description' => t('Valid directories to place library files, relative to %dir. Enter each directory on a separate line. Do not include leading or trailing slash.', array('%dir' => variable_get('file_public_path', 'sites/default/files'))),

Need to add a directory prepare so this dir actually exists.

+++ b/zipcart.admin.inc
@@ -14,30 +16,127 @@ function zipcart_settings_form() {
   $items['zip_builtin'] = array( 
     '#type' => 'fieldset',
     '#title' => 'PHP Zip settings',
-  ) ;
+  );
 
   $items['zip_pecl'] = array(
     '#type' => 'fieldset',
     '#title' => 'PECL Zip settings',
-  ) ;
+  );
 
   $items['zip_external'] = array(
     '#type' => 'fieldset',
     '#title' => 'External Zip binary settings',
-  ) ;

Huh. I was going to ask what these were doing, but I see they were in there already :P

+++ b/zipcart.info
@@ -1,6 +1,10 @@
-; dependencies[] = "jquery_update"
+dependencies[] = "jquery_update"
+dependencies[] = "shadowbox"

As per previous comment, I would rather this came in as a separate issue (and I prefer to keep ZipCart theme and design agnostic).

+++ b/zipcart.js
@@ -1,7 +1,31 @@
+if (Drupal.jsEnabled) {
+  (function($) {
+    $(document).ready(function(){
+/*
+      $("a.fancybox").mouseover(function(){
+        alert('stuff');
+      });
+*/
+      // Display preview links in fancybox
+
+      $("a.fancybox").fancybox({
+        'titleShow'     : false,
+        'transitionIn'  : 'none',
+        'transitionOut' : 'none',
+        'frameWidth'    : '960',
+        'frameHeight'   : '960',
+      });
+    });
+  })(jQuery);
+}

As above. If there are technical limitations to the current method, let's fix them. You should still be able to do fancypants JS as I've done on other sites.

+++ b/zipcart.module
@@ -11,126 +15,168 @@
+        'title' => variable_get('zipcart_block_title', 'My Downloads'),

I think "title" is confusing here, as there's already a block title (and that isn't changed when this value is changed).

"width" doesn't belong here. That goes to theme layer - some sites might display ZipCart block in multiple themes.

OK guys - after reviewing the changes here I'm going to ask for a re-roll which introduces minimal (or no) new functionality. The patch in #3 may be a working port, but it adds a bunch of new items as well. These need to come in via their own issues, and not be bundled with D7 port.

The following either belong in separate issues or not in this module -

• shadowbox/fancybox - design-specific functionality goes to theme layer
• block width setting - as for previous
• references to "file-library" path - feels like functionality specific to a certain site?
• new zip limitations (number of files, size etc) - potentially good ideas but separate issue(s) please
• empty cart functionality - #1110264: User should be able to remove files (either "empty cart" or remove specific files)
• changes to _zipcart_get_destination_alias() - is this a bugfix? if so separate issue so it can go into D6 branch

So - we're back to "needs work" I'm afraid.

Ed (uberhacker), please don't think this undermines your good work - your efforts are appreciated. I'm open to discussing any or all of this. Everyone else - I know, maintainers suck :) ... Consider offering Ed a bounty for re-rolling a bare D7 port.

OK, here's a re-roll with minimal feature addition.

There are a couple of several things in this which will go back to 6.x as well.

-Drupal.zipcart = {
-
-  init: function() {
-    $('a.zipcart').click(Drupal.zipcart.addToCart);
-  },
+(function ($) {
+  Drupal.behaviors.zipcart = {

-; dependencies[] = "jquery_update"

-        alert(Drupal.t('Unable to add the file to your ZipCart.');
+            alert(Drupal.t('Unable to add the file to your ZipCart.'));

-  $items['zipcart/get'] = array(
+  $items[ZIPCART_PATH_GET] = array(

 /**
- * Implementation of hook_init().
- */
-function zipcart_init() {
-  drupal_add_js(drupal_get_path('module', 'zipcart') . '/zipcart.js');
-  drupal_add_js(array('zipcart' => array('path_add' => ZIPCART_PATH_ADD, 'path_add_ajax' => ZIPCART_PATH_ADD .'/AJAX')), 'setting');
-}
+function zipcart_preprocess_zipcart_block_downloads(&$variables) {
+  drupal_add_js(drupal_get_path('module', 'zipcart') . '/zipcart.js');
+  drupal_add_js(array('zipcart' => array('path_add' => ZIPCART_PATH_ADD, 'path_add_ajax' => ZIPCART_PATH_ADD .'/AJAX')), 'setting');
+  $variables['count'] = (!empty($_SESSION['zipcart']['files'])) ? sizeof($_SESSION['zipcart']['files']) : 0;
+  $variables['files'] = (!empty($_SESSION['zipcart']['files'])) ? $_SESSION['zipcart']['files'] : NULL;
+}

but that's not all

-  $form['zipcart_zip_dir'] = array(
-    '#title' => t('Zip Cache Directory'),
-    '#type' => 'textfield',
-    '#default_value' => variable_get('zipcart_cache', 'zipcart'),
-    '#description' => t('Directory to build files in, relative to !dir.', array('!dir' => file_directory_path())),
-  );
-
+/*
   $items['zip_builtin'] = array(
     '#type' => 'fieldset',
     '#title' => 'PHP Zip settings',
@@ -42,6 +36,7 @@ function zipcart_settings_form() {
     '#type' => 'fieldset',
     '#title' => 'External Zip binary settings',
   );
+*/

-    'access arguments' => array('access content'),
+    'access arguments' => array('access zipcart downloads'),

+  // we build the file in $filename, but file_transfer will
+  // only deliver it via a registered Drupal stream ,,,
+  $filename = drupal_tempnam(file_directory_temp(), 'zipcart');
+  $transfer_filename = 'temporary://' . basename($filename);

If we build the Zip in temporary:// then it doesn't seem to get written to disk, and when we try and serve it there's no file to serve. But file_transfer() won't play if we're not serving from a registered stream. So we give '/tmp/' to ZipArchive to write our temp file, then we serve the file from temporary:// ... this is uberhacker's workaround from #3.

This restores something like what D6 had in zipcart_filterzip() to prevent download of files that would otherwise be inaccessible to the user.

Added a 7.x-1.x-dev release for this to facilitate testing.

* #15 - Should rewrite the streamwrapper workaround(s) so they do things "properly". Writing the zip to temporary:// didn't leave us with a file to server; writing the zip to /tmp made file_transfer() angry.
* #16 - The assumption that we will only zip files found in the default stream (eg public://) seems a bit random, but that's what we have in place of 6.x's file_check_location() for now.

http://drupal.org/node/1234538

-

@davidlerin, thanks for nudging. If you want to contribute then review and testing is the best thing you can do, unless you want to look at the two items in comment #17 above.

Currently I'm focussed on other projects, but happy to engage any progress here or discuss via IRC.

Thanks! Committed, some minor changes - commented out console.log stmts.

More people are now using 7.x-1.x-dev than 6.x-1.4 so I think we're good to go here :) Will post a release shortly.