6.x-1.x-dev

* Deciding whether to use HEAD for further development (http://drupal.org/node/17570#HEAD)

* If you look at http://cvs.drupal.org/viewvc.py/drupal/contributions/modules/workspace/ you see that there is no version 6 branch yet, all 6 releases are tags on the main branch as of yet.

* I would suggest creating a stable branch from the 6.x-1.3 version (as of 2008-Jul-24) which will only get bug fixes in future, and applying patches to HEAD which should be the development snapshot.

I agree that HEAD should be where patches go. Although it's a bit late a branch DRUPAL-6--1 for the next stable release(s) would IMO be allright

I did it... I created a 6-1 branch and a 1.4-RC1 release. My plan is to apply the patches to that branch so we will keep HEAD as backup. When version 1.4 is stable we can merge it back to HEAD for further development of 2.x.

However, I would prefer keeping 1.3 as "Recommended for Drupal 6" and 1.4-RC1 as "Also available" in the list of releases but don't know how to achieve this. Any help appreciated!

Frank

I applied the attached cumulative patch by hutch, cleaned up the code with Coder, updated the .pot file and the German translation and committed everything to HEAD.

@hutch
It would be nice if you could provide the issue numbers your patch addresses for possible follow-ups. TIA

Frank

Attachment	Size
workspace.module-hutchdev.patch	6.56 KB

It's been awhile, but I will attempt it ;-)

The issues that caused me to have a good look at workspace.module code were I think the following:

#470646: Authenticated user workspace access when not allowed.
#124912: Permission for who gets access to My workspace

However when I got stuck in I found a number of other issues that were causing errors, especially on an install in a fresh install of Drupal, eg no nodes or comments yet.

1) I found that the function workspace_build_rows() was throwing a 'division by zero' error when attempting to parse a result with no rows. I added some code to protect against this WSOD.

This does not apply to workspace_list_files() as it does not use workspace_build_rows()
Perhaps it should for the sake of consistency and ease of maintenance in the future but that's another issue.

BTW, you will only see this error if display_errors is set in php.ini

2) maxcomments is never set, so I added code to allow the user to choose a pagination value if comments are enabled.

3) The following code:
line 272

This will not work because $account->workspaces can return TRUE even though $account->workspaces['default']['maxcomments'] is not defined so the default of 50 is never applied. This I think is the reason there have been reports about no Pagination. see #376082: Pagination

The same applies to line 219 and line 387

A patch on today's 6.x-1.x-dev is attached, thank you Frank for attending to this.

Attachment	Size
workspace.module-hutchdev_2.patch	5.56 KB

I have just done a new install on a fresh drupal 6.13
I have found that the division by zero error no longer occurs so presumably the function pager_sql() has been updated to just return.
I am also getting the default of '50' showing when it should instead of the '0' I was getting some months ago so I have removed those fixes, no longer needed.

Here is a new patch, it corrects a theming issue where the function theme_workspace_list() wes being called directly instead of via the theme() function. I have tested this by copying function theme_workspace_list() to my theme's template.php, renamed of course and altering the output from 'Your workspace is currently empty.' to 'You have no content.' Flush the cache and the new phrase appears. Tested on acquia-marina theme.

It also adds a textfield to the configure form when comments are enabled so that should sort #376082: Pagination

hope this helps

Attachment	Size
workspace.module-hutchdev_3.patch	3.63 KB

Thanks for the patch! Committed it and updated .pot file and German translation.

Just a few more issues, I made a mistake in the form, had maxfilenames where it should have been maxcomments. Also it wasn't saving maxcomments, and the theme() bug is fixed in the attached patch. Also fixes auto-sort on 'Modified' which was not working. I have also attached a patched copy of workspace.module. This should not affect the translation files

Getting there!

Attachment	Size
workspace.module-hutchdev_4.patch	2.94 KB
workspace.module_4.txt	19.89 KB

Wow, that's fast! Could you check the list of pending patches whether they are included in your recent patch?

tia
Frank

They are on today's cvs ;-)
I'll check that list though

#344585: "Add new item" feature produces bad URLs for CCK content types
yes but needs testing, I don't have a cck setup to test it on ATM

#470646: Authenticated user workspace access when not allowed.
yes but needs testing. It looks to me that it is working but others need to look too.

#376082: Pagination
yes but in a different way, all three tables are put through pager_query()

#488216: Default sort order
is also in the v 4 patch. It worked in 'files' but not content or comments, I haven't got any comments in my devbox sites but it works in content. as the code change is the same in both I reckon it will work.