Port Homebox to Drupal 7

Hi rolfmeijer,

Well actually I don't know if I'm going to release a 7 release because of the core Dashboard module. I haven't look at it for now and don't know if it solves what homebox has to offer.

If you have more on this please let me know. And more than this I really am full of work now, and it's are find some 'coding free time'.

But I do know that porting Homebox to D7 could give some really nice features, and as I think of it, I do have good ideas about the big rewrite. And last point, what makes me doubt about a D7 release is discussions that were made about homebox when it was released. Anyway, the short answer is: don't know yet.

Jérémy

D7 core dashboards adds blocks, but doesn't support multiple dashboards or per user dashboards.

Updating this to 2.x..

At the moment, I don't have plans for a D7 port (yet). That doesn't mean that I'm going to do it - I just can't say when. This module is still being actively developed and is (most likely) going to be used for the drupal.org redesign. That work must come first.

Hi Mike -- I got a look at the Drupal.org redesign and I think once everyone sees Homebox in action, they are going to want it for their sites. Have you given any more thoughts about a port to D7? Is the task seriously daunting or just daunting?

It will most likely get done once all the API changes are in place for the 6.x branch.

i'm more a fan of Panels for a complex setup, or D7 core Dashboard / D6 Managesite for a simple setup, when building a admin dashboard.

when building a personal dashboard, i would rely on Panels Dashboard or Homebox when they mature enough.

i've written a big review of dashboard modules here:
http://groups.drupal.org/node/95039

feel free to provide input so i can update it. homebox was reviewed about 2 months ago.

Why'd you call Homebox buggy?

the last version (2 months ago) i used made all sorts of "special effects" when trying to drag boxes around, even when using the default theme, garland.

also at that time, i think couldn't add new boxes successfully

edit: i updated the wiki further to state that homebox might at this moment be fully functional and that the review for it is outdated.

i do not have the time at this point to review it and other modules in that comparison again.

That was probably before a 2.0 release was made.

I am building a Drupal 7 version of homebox right now for a site that upgraded to Drupal 7 - but needed homebox and can't revert just for that one function. I would be glad to help in this process, my version is thus far highly hacked - though I'm learning quite a bit.

subscribe

Subscribing.

Subscribing

subscribing

Subscribe

Subscribing

Subscribing

Subscribing

Since this module is running on Drupal.org, Drupal.org can't upgrade to Drupal 7 until this module does.

Since the d.o. redesign team jumped onto Homebox, I've basically removed myself from it. I was responsible for the jump to the 2.x branch of homebox, but I really don't like what I see was done in the dev branch. So this, and I suppose the remaining issues, will have to be taken care of by one of them.

I'm actually in the process of porting this module to D7. I'd say I'm about halfway through, but I was wondering whether I should stick as closely as possible to the module's code, or if I should go ahead and make the homeboxes into entities? If so, would you accept entity api module as a dependency, or would you rather have custom CRUD functions?

Hm! that's a fine question. I'm afraid I don't really know the answer. I guess I would say that for an initial D7 port, probably makes the most sense to do as straight a port as possible, and then make a 7.x-2.x branch that's "awesome entities craziness".

I'll ping Neil and ask him to look at this, too.

Thanks so much for helping with the port!!

Here's what i've gotten so far. Everything works fine except for the drag/drop of blocks in the homebox page (using the dropdown works fine), and there's an error from _form_set_class() if i remember correctly (caused by the dropdowns). I spent a while trying to fix those, but given that they don't affect the main functionality, I'll leave it to you guys. Oh, and i forgot to port homebox_og... maybe i'll get to that later.

Sorry, forgot to change status.

NICE!!!! Thanks a ton, blup!

thank u very much blup!

one question, why we need dependence to jquery ui module if it's already included in d7?

Fatal error: Call to undefined function jquery_ui_get_version() in /home/u3220/domains/u******/sites/all/modules/homebox/homebox.module on line 1463

and

Notice: Undefined variable: output in homebox_help() (line 196 of /home/u3220/domains/u******/sites/all/modules/homebox/homebox.module).

I believe hook_help() should only return something if there is something to return.

Current code:

<?php
function homebox_help($path, $arg) {
  switch ($path) {
    case 'admin/structure/homebox':
      $output = '<p>' . theme('advanced_help_topic', array('module' => 'homebox', 'topic' => 'introduction')) . '&nbsp;';
      $output .= t("Homebox pages are listed below. Each page is accessible from a single url on your site that you specify during page creation. You can create as 
many pages as you need. Be sure to review all available layout options and settings.</p>");
      break;
    case 'admin/structure/homebox/layout/%':
      $output = '<p>' . theme('advanced_help_topic', array('module' => 'homebox', 'topic' => 'default-layout')) . '&nbsp;';
      $output .= t("This page behave the same way as Drupal block administration page. Drag blocks to whatever column you want to enable it for your users. Note that you can change the number of columns in the <a href='!settings_url'>settings page</a>", array('!settings_url' => url('admin/structure/homebox/settings/' . arg(4)))) . '.</p>'; 
      break;
    case 'admin/structure/homebox/settings/%':
      $output = '<p>' . theme('advanced_help_topic', array('module' => 'homebox', 'topic' => 'settings')) . '&nbsp;';
      $output .= t('Homebox configuration page.');
      break;
  }
  return $output ? $output : NULL;
?>

Should something more like:

<?php
function homebox_help($path, $arg) {
  switch ($path) {
    case 'admin/structure/homebox':
      $output = '<p>' . theme('advanced_help_topic', array('module' => 'homebox', 'topic' => 'introduction')) . '&nbsp;';
      $output .= t("Homebox pages are listed below. Each page is accessible from a single url on your site that you specify during page creation. You can create as 
many pages as you need. Be sure to review all available layout options and settings.</p>");
      return $output;
    case 'admin/structure/homebox/layout/%':
      $output = '<p>' . theme('advanced_help_topic', array('module' => 'homebox', 'topic' => 'default-layout')) . '&nbsp;';
      $output .= t("This page behave the same way as Drupal block administration page. Drag blocks to whatever column you want to enable it for your users. Note that you can change the number of columns in the <a href='!settings_url'>settings page</a>", array('!settings_url' => url('admin/structure/homebox/settings/' . arg(4)))) . '.</p>'; 
      return $output;
    case 'admin/structure/homebox/settings/%':
      $output = '<p>' . theme('advanced_help_topic', array('module' => 'homebox', 'topic' => 'settings')) . '&nbsp;';
      $output .= t('Homebox configuration page.');
      return $output;
  }
}
?>

This will solve the warning/error message from #27 about the homebox_help().

Also, the og support produces this warning:
Warning: Invalid argument supplied for foreach() in homebox_og_group_settings_page() (line 133 of sites/all/modules/homebox/homebox_og/homebox_og.module).

Update:
Drupal 7 has jquery version: Query 1.4.2 and jQuery UI 1.8
As explained here: http://drupal.org/node/287304

The code that produces the error mentioned in #27 about "Call to undefined function jquery_ui_get_version() .." happens to be doing a dependency check for version 1.6.

For homebox to be supported in drupal 7, this must support version jquery ui 1.8 and so this segment of code should be removed.

I will make a patch once I resolve a bunch of other problems that have appeared.

Here is a patch against the code supplied from #24.

There were a number of problems solved in addition to those I mentioned in #28.

Numerous db_query calls were pulling data from the database with the wrong function.
Many cases didn't include the ->fetchAll() or ->fetchObject() and some others used ->fetchField() when ->fetchObject() should be true.

A few menu hook entries were incomplete.

The homebox_load() was being passed a string and not an array.
I rewrote the homebox_load() function to handle both a single string and an array of strings.
The homebox_load() was not even returning its results, so it was doing nothing.

I am using a patch so that you can more easily review changes and point out any mistakes or better solutions.

I still get a few error messages on the layout settings page:

Warning: Illegal offset type in theme() (line 811 of includes/theme.inc).
Warning: Illegal offset type in theme() (line 820 of includes/theme.inc).
Warning: Invalid argument supplied for foreach() in element_children() (line 6046 of includes/common.inc).
Warning: Illegal offset type in isset or empty in contextual_preprocess() (line 105 of modules/contextual/contextual.module).

I am subscribing to this great module. thanks!

Hello all,

I'm new to drupal,actually introduced to it this week. I was looking for the igoogle type interface and came across this homebox. My unfortunate dilemma is that being a new user I went straight to D7. From reading the above it seems that user thekevinday has somewhat created a patch to update the homebox for D6?

Hi Ranjitsingh,

First of all welcome. Some advice. Set up your site with D7 like you want it to be, for now without the homebox functionality. By the time you're ready, Homebox will probably be available for D7 (it has an active issue cue). Choosing your modules now (especially now) for D7, you actually pick those that are actively maintained, so stick with D7. You jumped into Drupal at the right moment. Get to know Drupal and some mainstream modules (look at the Reported installs), like Views and Panels. Choose a solid but flexible theme like Fusion or one of it's sub-themes. There are lot's of articles available to get started.

Sorry to be a bit Off Topic. Good luck and be persistent. It takes a bit of time but it definitely pays off.

Thanks a lot for your help thekevinday, i guess i missed some details. Here's an updated version with the patch in #29. I'll try and take a look at homebox_og this week.

Sorry, I had made a mistake with the previous module files (older version). Here's the more recent version revised with patch from #29.

And once more (small discrepancy between patch and revised version).

subscribing.

#35 works much better, thanks.

However, another problem turned up.
There are a few cases where you have something like:
<?php $block->closable = ($block_settings['closable'] === 0) ? FALSE : TRUE; ?>

It seems that this generates a warning on the screen.
To prevent the warning, such cases need to be wrapped in an isset() or similar check.

Example, the above code should look like:
<?php $block->closable = (isset($block_settings['closable']) && $block_settings['closable'] === 0) ? FALSE : TRUE; ?>

I have another patch attached that fixes the cases like the one above where I noticed them.

I seem to have come across a few more problems.
This new patch includes & replaces the patch from #37.

There were a number of problems where warnings were printed to the screen because of elements of an array being directly access without confirming that the variable is an array.

There are a few cases were the CSS class attribute is being altered and passed to the theme engine as a string.
It seems that the theme engine expects the #attributes class element to be an array of strings and not a string itself.

In particular, on line #641 of homebox.admin.inc, there seems to be a custom block_listing row_class variable.
I am assuming/guessing that this needed to be converted to an array, given that one of its options directly accesses the #attributes class element. Someone who knows this project better than me needs to review this change to ensure that it is a correct change.

There is also a database problem with the OG integration module.
I do not have a solution for this and do not know if the problem is with homebox, if the problem is with the views module or if the problem is with og itself.
When viewing a home box page with the og_members: block on it I get the following message:

Debug:

'Exception: SQLSTATE[42601]: Syntax error: 7 ERROR:  syntax error at or near "."
LINE 4: WHERE (( (users.status <> \'0\') AND (.group_audience_state IN...
                                            ^'

in views_plugin_query_default->execute() (line 1314 of views/plugins/views_plugin_query_default.inc).

Thats all I have for now.

Subscribing.

This is great effort which will benefit a lot of pages, I'm sure. Just wondering why the code is posted as attachments instead of being posted to the CVS?

To answer #40:
The submitters of the attachments are the community and not the developers.
We cannot access the CVS for whatever reason be it no write access or it is against the drupal code of conduct to alter a project in which you are not in the projects developer list.
Therefore the attachments have to be made available via this method.

Thanks lolandese.

I'm working on my thesis. It's actually a portal to test usability. I wanted to have about three portals on the main page available so the users could easily navigate to the one they like the most. Not sure if anyone can advise me as to how I can achieve this at the moment as homebox isn't just yet ready.

kind regards,
Ranjitsingh.

Subscribing

Subscribing

Subscribing

I suggest you post a a single patch from 6.x to 7.x, it's impossible to keep track of all these archives and incremental patches :)

Or alternatively, what I'm usually doing for bigger projects, clone the git repository (git clone git://git.drupalcode.org/project/homebox.git), then you can make local commits to a separate branch push your code to github.com or a similiar project. github for example then automatically provides a download link where users interested in testing can download a single archive but you can continue to do incremental patches. Another advantage is that you can more easily merge changes from the official repository.

I think a release here at drupal.org would be great.

Subscribe

The "incremental patch" is a single patch by itself. Do not attempt to apply -1, -2, and -3, just use the most recent
That number represents a version number, not a patching order.

That aside, I am glad drupal git is progressing.
I will at some point bring up one of my own servers and provide sub-branches.
(But only as soon as I learn how to setup a git server)

Back to this project, it looks like I have come across another bug:
Notice: Undefined offset: 1 in _homebox_check_views_block_access() (line 1015 of homebox/homebox.module).
I am not sure what is happening here to cause this.

Yes, but it is a patch that is applied to a uploaded archive if I understood it correctly :)

A single 6.x -> 7.x patch shows the actual differences and can be reviewed with tools like dreditor.

I made a repository with the changes so far. Feel free to branch it with any other patches. https://github.com/blup/homebox

after install last github version:

    Notice: Undefined index: content in homebox_load_blocks_in_regions() (line 361 of /home/u3220/domains/*********/sites/all/modules/homebox/homebox.module).
    Notice: Undefined index: color in _homebox_get_css_classes_for_block() (line 1211 of /home/u3220/domains/*********/sites/all/modules/homebox/homebox.module).
    Notice: Undefined variable: can_access_view in homebox_load_blocks_in_regions() (line 413 of /home/u3220/domains/*********/sites/all/modules/homebox/homebox.module).
    Notice: Undefined index: content in homebox_load_blocks_in_regions() (line 361 of /home/u3220/domains/*********/sites/all/modules/homebox/homebox.module).
    Notice: Undefined index: color in _homebox_get_css_classes_for_block() (line 1211 of /home/u3220/domains/*********/sites/all/modules/homebox/homebox.module).
    Notice: Undefined variable: can_access_view in homebox_load_blocks_in_regions() (line 413 of /home/u3220/domains/*********/sites/all/modules/homebox/homebox.module).
    Notice: Undefined index: content in homebox_load_blocks_in_regions() (line 361 of /home/u3220/domains/*********/sites/all/modules/homebox/homebox.module).
    Notice: Undefined index: color in _homebox_get_css_classes_for_block() (line 1211 of /home/u3220/domains/*********/sites/all/modules/homebox/homebox.module).
    Notice: Undefined variable: can_access_view in homebox_load_blocks_in_regions() (line 413 of /home/u3220/domains/*********/sites/all/modules/homebox/homebox.module).
    Notice: Undefined index: content in homebox_load_blocks_in_regions() (line 361 of /home/u3220/domains/*********/sites/all/modules/homebox/homebox.module).
    Notice: Undefined index: color in _homebox_get_css_classes_for_block() (line 1211 of /home/u3220/domains/*********/sites/all/modules/homebox/homebox.module).
    Notice: Undefined variable: can_access_view in homebox_load_blocks_in_regions() (line 413 of /home/u3220/domains/*********/sites/all/modules/homebox/homebox.module).
    Notice: Undefined index: content in homebox_load_blocks_in_regions() (line 361 of /home/u3220/domains/*********/sites/all/modules/homebox/homebox.module).
    Notice: Undefined index: color in _homebox_get_css_classes_for_block() (line 1211 of /home/u3220/domains/*********/sites/all/modules/homebox/homebox.module).
    Notice: Undefined variable: can_access_view in homebox_load_blocks_in_regions() (line 413 of /home/u3220/domains/*********/sites/all/modules/homebox/homebox.module).

Berdir:

Please find a patch attached between 6.x-2.x and the D7 port in blup's github.

Probably this is where the review can start in earnest!

Edit: It looks like this port started with a copy of the Dev tarball as of Aug. 11 / 2010. I suspect that it likely reverts several month's worth of patches.

I think I will need to diff blup's D7 repo against an Aug. 11 version of the repository to just get the d7 changes, then see if I can work them forward to be against the present 6.x-2.x branch...

Ok, I forward-ported the previous patch against current 6.x-2.x. It should now apply cleanly. However, it will still need work as obviously, it only touches code that existed in August.

That's quite a patch. :) Even though it's 100kb less then the other one.

Since I promised it, I'll give it a look.

+++ b/homebox.admin.inc
@@ -96,15 +93,15 @@ function homebox_admin_page() {
-  $result = db_query('SELECT rid, name FROM {role} ORDER BY name');
+  $result = db_query('SELECT rid, name FROM {role} ORDER BY name')->fetchAll();
   $role_options = array();
-  while ($role = db_fetch_object($result)) {
+  foreach ($result as $role) {

- The fetchAll() is not necessary, foreach() can directly loop over a result set.

- I'm wondering if this could use user_roles() instead? A possible advantage would be that that function adds the translatable tag which could eventually display translated role names then.

+++ b/homebox.admin.inc
@@ -150,67 +147,74 @@ function homebox_admin_page() {
-  if (!$form_state['values']['import']) {
+  if (!isset($form_state['values']['import'])) {

Not sure if this shouldn't be !empty() instead of isset(). Aka, could this be an an empty string, FALSE or something like that?

+++ b/homebox.admin.inc
@@ -349,7 +361,7 @@ function homebox_layout($page) {
-  require_once(drupal_get_path('module', 'block') .'/block.admin.inc');
+  require_once DRUPAL_ROOT . '/' . drupal_get_path('module', 'block') . '/block.admin.inc';

This could use module_load_include, I guess?

+++ b/homebox.admin.inc
@@ -652,14 +681,24 @@ function template_preprocess_homebox_admin_display_form(&$variables) {
-    '#options' => array('1' => 1, '2' => 2, '3' => 3, '4' => 4, '5' => 5, '6' => 6, '7' => 7, '8' => 8, '9' => 9),
+    '#options' => array(
+      '1' => 1,
+      '2' => 2,
+      '3' => 3,
+      '4' => 4,
+      '5' => 5,
+      '6' => 6,
+      '7' => 7,
+      '8' => 8,
+      '9' => 9,
+    ),

There is a nifty little function called drupal_map_assoc() which can convert from array(1,2,3,4) to array(1 => 1, 2 => 2, and so on.

+++ b/homebox.info
@@ -2,12 +2,26 @@
 name = Homebox
 description = Provides user with a page like iGoogle, Netvibes or bbc.co.uk front page
 dependencies[] = block
-dependencies[] = jquery_ui
 package = Homebox
-core = 6.x
+core = 7.x
 
 ; Information added by drupal.org packaging script on 2010-05-22
 version = "6.x-2.x-dev"
-core = "6.x"
+core = 7.x
 project = "homebox"
 datestamp = "1274530155"
+
+; Information added by drupal.org packaging script on 2010-08-11
+version = "6.x-2.1"
+core = 7.x
+project = "homebox"
+datestamp = "1281486392"
+
+
+files[] = homebox-admin-display-form.tpl.php
+files[] = homebox-block.tpl.php
+files[] = homebox.admin.inc
+files[] = homebox.features.inc
+files[] = homebox.install
+files[] = homebox.module
+files[] = homebox.tpl.php

- The whole information added ... stuff can be completely removed, no idea where this is coming from, probably started porting on a downloaded release archive?

- Also, files[] declarations are only necessary for files which contain classes.

+++ b/homebox.install
@@ -7,48 +7,73 @@
+        'size' => 'big',
+        'not null' => TRUE,
+        'serialize' => TRUE,
+        'object default' => array(),
+      ),
+    ),  ¶

Since all of the others are removed, here is a new trailing space added by this patch :)

+++ b/homebox.module
@@ -191,55 +203,64 @@ function homebox_forms($form_id, $args) {
+      'title' => t('administer homebox'),
+      'description' => t('TODO Add a description for \'administer homebox\''),

title should be improved (at least Administer..). description could probably simply be removed, this is kinda obvious :)

Better nothing than a TODO string imho.

+++ b/homebox.module
@@ -191,55 +203,64 @@ function homebox_forms($form_id, $args) {
+      // TODO
+      /*
+       *'variables' => array('regions' => $regions, 'available_blocks' => $available_blocks, 'column_count' => $column_count, 'page' => $page),
+       */
+      'variables' => array('regions' => NULL, 'available_blocks' => NULL, 'column_count' => NULL, 'page' => NULL),

No idea what that commented out part is about?

+++ b/homebox.module
@@ -191,55 +203,64 @@ function homebox_forms($form_id, $args) {
-      'arguments' => array('form' => NULL),
+      'render element' => array('form' => NULL),
     ),
     'homebox_admin_new_page' => array(
-      'arguments' => array('form' => NULL),
+      'render element' => 'form',

render element is not an element, the second one is correct.

+++ b/homebox.module
@@ -255,16 +276,21 @@ function homebox_theme($blocks) {
+  drupal_add_css($path = drupal_get_path('module', 'homebox') . '/includes/tipsy/tipsy.css', array('type' => $type = 'module', 'media' => $media = 'all', 'preprocess' => $preprocess = TRUE));
+  drupal_add_css($path = drupal_get_path('module', 'jquery_ui') . '/jquery.ui/themes/default/ui.all.css', array('type' => $type = 'module', 'media' => $media = 'all', 'preprocess' => $preprocess = TRUE));
+ ¶

Another new trailing whitespace.

+++ b/homebox.module
@@ -747,27 +774,27 @@ function homebox_homebox_block_edit_form($block) {
+  $allowed_roles = db_query("SELECT rid FROM {block_role} WHERE module = :module AND delta = :delta", array(':module' => $block->module, ':delta' => $block->delta))->fetchAll();

Same here, fetchAll() isn't necessary.

+++ b/homebox.module
@@ -874,15 +902,17 @@ function homebox_get_page($name) {
+    db_delete('homebox_pages')
+    ->condition('name', $page->name)
+    ->execute();

The chained calls should be indented by 2 spaces.

+++ b/homebox.module
@@ -905,17 +935,21 @@ function homebox_save_page($page, $check = FALSE) {
-  if (db_query("DELETE FROM {homebox_pages} WHERE name = '%s'", $name) &&
-        db_query("DELETE FROM {homebox_users} WHERE name = '%s'", $name)) {
+  if (db_delete('homebox_pages')
+    ->condition('name', $name)
+    ->execute() &&
+        db_delete('homebox_users')
+    ->condition('name', $name)
+    ->execute()) {
     return TRUE;
   }

Kinda weird syntax, never seen anything in core actually checking the return value of delete queries like this.

If this exists for a reason (perfectly possible), then those calls should imho be done outside of the if to make it easier to read. This would actually make the if unecessary, it could simply be "return $deleted_pages && $deleted_users;".

+++ b/homebox.module
@@ -974,8 +1008,8 @@ function homebox_check_path($path, $name = NULL, $element = NULL) {
+  $pages = db_query("SELECT * FROM {homebox_pages}")->fetchAll();
+  foreach ($pages as $page) {

Again, fetchAll()... :)

+++ b/homebox.module
@@ -1339,8 +1373,10 @@ function _homebox_get_css_classes_for_block($block) {
-  if (db_query("DELETE FROM {homebox_users} WHERE name = '%s'", $page->name)) {
-    return TRUE; 
+  if (db_delete('homebox_users')
+    ->condition('name', $page->name)
+    ->execute()) {
+    return TRUE;
   }

same as above. Or something like "return (bool) db_delete)(..."

And thinking about it, I don't think this code made any sense... this will always return a resource which equals TRUE, even if nothing has been deleted. So it can probably just be deleted :)

+++ b/homebox.module
@@ -1605,19 +1639,19 @@ function _homebox_check_php() {
   $version = explode('.', phpversion());
   if ($version[0] >= 5) {
     if ($version[0] == 5 && $version[1] < 2) {
-      return FALSE; 
+      return FALSE;

This seems to check if we are using at least PHP 5.2. Since this is a core requirement now, this can be removed. Maybe the whole hook_requirements part could be removed since the jquery ui check is removed too..?

+++ b/homebox_example/homebox_example.info
@@ -3,4 +3,13 @@ name = Homebox example
+; Information added by drupal.org packaging script on 2010-08-11
+version = "6.x-2.1"
+core = 7.x
+project = "homebox"
+datestamp = "1281486392"
+
+
+files[] = homebox_example.module

Same as above, this can be removed now.

+++ b/homebox_og/homebox_og.info
@@ -4,4 +4,14 @@ description = Provides integration with Organic Groups and Homebox
+; Information added by drupal.org packaging script on 2010-08-11
+version = "6.x-2.1"
+core = 7.x
+project = "homebox"
+datestamp = "1281486392"
+
+
+files[] = homebox_og.install
+files[] = homebox_og.module

Another one.

+++ b/homebox_og/homebox_og.install
+++ b/homebox_og/homebox_og.install
@@ -2,7 +2,13 @@

@@ -2,7 +2,13 @@
 // $Id: homebox_og.install,v 1.1.2.2 2010/05/26 21:31:13 mikestefff Exp $
 
 /**
- * Implementation of hook_uninstall().
+ * @file
+ * Install, update and uninstall functions for the homebox_og module.
+ *
+ */
+
+/**
+ * Implements hook_uninstall().
  */
 function homebox_og_uninstall() {

Kinda weird that it stops at this point, I guess the port of this module isn't finished.

I'm also quite sure that there were api changes in og.module and this hasn't been tested yet :)

Powered by Dreditor.

There are actually a significant number of problems.

1)
in all cases, the require_once DRUPAL_ROOT . '/' ... code should be removed as it is handled already by files[] = ... in the .info files.

2)
in template_preprocess_homebox_admin_display_form() from homebox.admin.inc:
- All cases of ['#attributes']['class'] should be an array of strings and not a string itself.

3)
in template_preprocess_homebox_admin_display_form() from homebox.admin.inc:
- drupal_render_children() should be used instead of drupal_render():

-  $variables['form_submit'] = drupal_render($variables['form']);
+  $variables['form_submit'] = drupal_render_children($variables['form']);

- I am not entirely clear as to why this is, but without this change things don't work.

4)
in homebox_configure_form() from homebox.admin.inc:
- All cases of $page->settings['colors'][$i] ? $page->settings['colors'][$i] ... should be wrapped in isset() like:

-        '#default_value' => $page->settings['colors'][$i] ? $page->settings['colors'][$i] : '#E4F0F8',
+        '#default_value' => isset($page->settings['colors'][$i]) ? $page->settings['colors'][$i] : '#E4F0F8',

5)
in homebox_named_columns() from homebox.admin.inc:
- the produced columns array should be returned and not some custom array, thus:

-  /*
-   *return $columns;
-   */
-   return array('1' => 'Header', '2' => 'Help', '3' => 'Featured');
+  return $columns;

6)
in the template file homebox/homebox-block.tpl.php:
- should print $block->content; be wrapped by drupal_render() ?

7)
in homebox_theme() from homebox.module:
- A correction from comment #55:

-      'render element' => array('form' => NULL),
+      'render element' => 'form',

8)
in template_preprocess_homebox() from homebox.module:
- I don't understand the logic for doing something like the following:
drupal_add_js($data = drupal_get_path('module', 'homebox') ...
- why is $data being assigned inline with the drupal_get_path()?
- $data does not even seem to be used, so removing this makes sense
- also, for performance reasons, only make one call to drupal_get_path() and store it in a variable

9)
in homebox_build() from homebox.module:
- There is a comment referencing how to properly implement parts of this function, so why not just implement it?
- This includes adding an implementation of hook_page_alter()

10)
in homebox_load_blocks_in_regions() from homebox.module:
- I believe it is better to properly check the existence of an array key before acting on it, example:

-    $block->subject = $block_settings['title'];
+    $block->subject = isset($block_settings['title']) ? $block_settings['title'] : NULL;

11)
in homebox_load_blocks_in_regions() from homebox.module:
- for some reason the invoking of the block was changed in recent patches, this change does not work in any of my cases.
- I had to do the following:

-        $array = module_invoke($block->module, 'block', 'view', $block->delta);
+        $array = module_invoke($block->module, 'block_view', $block->delta);

- why is this? I need to confirm how these functions work and how to make it work properly in all cases.

12)
in homebox_load_blocks_in_regions() from homebox.module:
- The variable $can_access_view does not exists and yet it is being called

13)
in homebox_load_blocks_in_regions() from homebox.module:
- the db_query() takes $block->bid, it is safer to check that $block->bid exists before doing this call

14)
in _homebox_get_css_classes_for_blockfrom homebox.module:
- Again, checking the existence of array keys is safer than just directly accessing

There are more problems than just these, but this is as far as I have gotten thus far.
A few of these problems I am uncertain on whether my solution is a proper fix or not, but it does appear to solve the problem.

I have a version from my patched version and a version from the latest patches.
My patched version works (but also includes some of the above 14 issues) whereas the more recent patches do not work without the above changes and mostly works with the above 14 changes.

I have attached a patch that contains all of my changes mentioned here so you can see for yourself.
They are expected to be applied after the patch from #54 because it is meant to fix problems from the #54
patch.

KevinDay - thanks for the improvements.

I had done a chunk of work on the module since the version I posted in #54 on on the 28th. I've merged your improvements with mine in the below patch, which also deals with all of Berdir's suggestions on the
#54. The biggest departure from what you wrote in #55 is the first point - I removed all the files[] lines from the .info files (since they are intended for class autoloading), and replaces the require_once call with a module_load_include().

I think the attached accurately represents the combined state of your progress and mine.

subscribe

Can someone tell me how to use this? I'm trying to set it up but it wants to Import data....Sorry I posted this here but couldn't find any info on setup.

if u don't know how to use it, my advice, wait for oficial release.

I've tried loading the patch against a download of version 6.x-2.x, and when I view the modules page, it shows it's for 6.x-2.x-dev. Is there anything else I need to do to get it to work?

Module isn't usable yet on a production site, so don't bother trying. Needs a lot of fixing before it will work in any fashion.

The patch is improving, but there are still more problems to fix.

1)
In homebox.tpl.php, change
<?php print theme('homebox_block', $block, $page); ?>
to
<?php print theme('homebox_block', array('block' => $block, 'page' => $page)); ?>

The theme function requires the second parameter to be an array of parameters.
Otherwise there is a whitescreen PHP error about treating an object as an array.

2)
In homebox.module, the function homebox_pre_build_user() has the $available_blocks variable, but I don't see one defined. Perhaps it should be changed to the $allowed_blocks variable?

3)
In homebox.module, the function homebox_prepare_block() around line #621:
<?php if (isset($block->content) && trim($block->content)) { ?>
I am getting an array for $block->content and so trim() is generating warnings.
It seems that I overlooked this issue as I had to already perform a drupal_render($block->content) in the theme template.
Further study suggests that I might be able to move the drupal_render() from the homebox-block.tpl.php to this area of code such that I have:

<?php
  if (is_array($block->content)){
    $block->content = drupal_render($block->content);
  }

  // We don't continue to assign this block since Drupal didn't returned any
  // content which could be permissions rules applied by any module.
  if (!isset($block->content) || trim($block->content) == "") {
    return NULL;
  }
?>

I obviously need a better understanding on how $block->content is being populated before making a decision on how to handle this. (please advise)

4)
The following
Notice: Undefined index: auto_save in template_preprocess_homebox() (line 305 of ...
can be silenced with a change in homebox.tpl.php of:
<?php print $save_form; ?>
to
<?php if (isset($save_form)) print $save_form; ?>

But the real question is why is it not defined?

5)
In homebox.module, the function homebox_save_form():
$page->name does not seem to exist.
I can find the node name at: $page['build_info']['args']['0'];
but I doubt that is the correct thing to do.

6)
The blocks are now appearing again, but javascript drag and drop is not working at this time.
None of the homebox block buttons seem to do anything as well.
Perhaps one of the changes I did broke it?

EDIT:
7)
There also seem to be a large number of places where check_plain(), check_markup(), or some function that calls those needs to be called (or not).

It seems I have added it to a spot where it is unnecessary:
I get the following block: Who's new
because of:
<?php $variables['block_listing'][$region][$i]->block_title = check_plain($block['info']['#value']);?>
Removing the check_plain() in this case produces Who's new.
In this case should t() be used instead?
drupal_render() does not work in this case.

Hi Kevin:

I've caught and addressed most of those (apart form JS not working on the drag and drop) in the following patch, along with a pile more fixes.

It's too bad you aren't on IRC. I am working on this throughout the day, and I think you are putting lots of time catching bugs I have already fixed...

Brian

Attached please find my most recent (for now) update to this patch, along with a tarball copy of the module.

At this point, everything is pretty much working with two exceptions:

1. Autosaving of the block positions when changed - JS issue somewhere.
2. Regular saving of block positions is temperamental. I would say it doesn't work at all except in rare cases when it does.

I think #1 is in homebox.js; I haven't had much luck tracking it down. #2 should be something easier; as far as I can tell, it's in this block of code:

  // Remove any old settings
  db_delete('homebox_users')
    ->condition('uid', $user->uid)
    ->condition('name', $page->name)
    ->execute();

  // Update settings
  $data->uid = $user->uid;
  $data->name = $page->name;
  $data->settings = $blocks;
  drupal_write_record('homebox_users', $data);

The drupal_write_record() call at the end throws a SQL error telling me that I am trying to insert a duplicate row for my users UID. In short, it looks like the db_delete() above isn't actually working for some reason.

I am going to leave this as is for now. Hopefully someone else can step in and fix the above two issues.

What is the exact error message?

The code looks fine for me.

Thank you!!

I'm not sure if this is related to what you mentioned, but I did notice that if I'm logged in as an authenticated user and I try to add a block to the page, it doesn't actually get added. Clicking on 'add a block' shows the blocks I've made available. But, when I click on any of them, it just refreshes the page and doesn't actually get added. No errors or anything, the blocks just don't appear anywhere.

I found user_dashboard as a alternative module but it doesn't work (install failed with sql error). Could somebody show the pros and cons between homebox and user_dashboard?

@georgedamonkey:

As I said above, saving blocks from the user-facing portion is not working in the above tarball.

@JohnnyX:

Please open a support ticket for that. Your question is off-topic for this issue.

@Berdir:

I have the manual block position saving working now in my local code. It's just autosave left. I don't have much to follow up on that one. The save form button doesn't hide like it should, and it's not triggered when blocks are moved.

New version of the patch, more bugs fixed.

I've contacted drumm to see about getting a 7.x development branch opened and / or CVS access on this project. I get the impression it's somewhat abandoned as mikesteff has not been active in it since it was taken over for the drupal.org redesign, and the redesign people have accomplished what they want with it...

Thank you so much for taking the time with this.

I seem to have issues with patch... Could you please attach a tar of the patched module for us?

Thanks again!

@georgedamonkey:

I was just granted CVS access by jchatard, the original maintainer of this module. I'll be putting the module into CVS shortly. Once that is done, you'll be able to grab the module from CVS.

However, first I think I might commit a whitespace fix against 6.x-2.x. This will shrink the size of the current patch above by a very large margin, in turn shrinking the initial 7.x-2.x changes.

Once that is done, I will likely close this issue, so that specific issues can be created against a 7.x branch to address specific problems.

ok, attached is the final patch of the 6.x-2.x branch which I will use to create the D7 branch. As mentioned above, I committed a bunch of whitespace fixed to 6.x-2.x, then re-diffed my local 7.x branch. This results in a much, much more focused patch which would be in turn much, much easier to work with.

A 7.x-2.x branch has been created in CVS, and a dev release node has been created (although it won't appear until -dev tarballs are updated overnight).

I am setting this issue as 'fixed' for now; please file any further bugs / changes required in their own issues against version 7.x-2.x-dev.

Wow, great work on this folks!! :D

Don't be too quick with the accolades - there's some major bugs still in this port. But I a really hoping that having a -dev release out, and code easily available through CVS will make it a little easier for others to help grind through the remaining problems. Or at least find new ones I haven't encountered yet.

I find it a bit unfair that I worked on this port for several days, if not a week, and I was refused CVS access to open a dev branch. brianV then posted a couple of patches and he gets to make the commits...

blup:

If I've stepped on toes, it wasn't intentional. My only goal was to try to get a working D7 homebox port going, since the company I work for (PINGV Creative) needs it for a client project we are working on. As it is, the D7 version committed still has some major warts / critical issues.

I'd love your help polishing the D7 branch into a product that can be released.