Vocabulary Matrix

welcome,

There are still files other than README.txt in the master branch, make sure to remove them. See also step 5 in http://drupal.org/node/1127732
Review of the 7.x-1.x branch:

• README.txt is missing, see the guidelines for in-project documentation.
• Remove "version" from the info file, it will be added by drupal.org packaging automatically.
• Remove "project" from the info file, it will be added by drupal.org packaging automatically.
• Remove "datestamp" from the info file, it will be added by drupal.org packaging automatically.
• ./vocabulary_matrix.module: all functions should be prefixed with your module/theme name to avoid name clashes. See http://drupal.org/node/318#naming
  

  function _add_child(&$parent_item, $child_item, $curr_depth) {
  function locate_add_child(&$tree_node, $term_item, &$curr_depth, &$max_depth) {
  function to_tree($vid, &$tid_array) {
  function set_spans(&$tree_node, &$tid_array) {
  function make_y_header_rows(&$yaxis_tree_node, &$y_max_depth, &$rows, &$row) {
  function make_x_header_rows(&$tree_node, &$level, &$row, &$x_max_depth) {
  function _get_setting($field, $key ) {
  function build_table_data($field, $nid, $element = NULL) {
  

• Run coder to check your style, some issues were found (please check the Drupal coding standards). See attachment.
• Drupal Code Sniffer has found some code style issues (please check the Drupal coding standards). See attachment.

This automated report was generated with PAReview.sh, your friendly project application review script. You can also use the online version to check your project. Go and review some other project applications, so we can get back to yours sooner.

Don't forget to switch back to needs review after you think fixed all issues.

script.js

/**
 * 
 */
(function($){

Filename and comment are not very informative at least some lines of documentation and inline-comments would not hurt .. ;-)
Spaces must be used to indent lines; tabs are not allowed, also in .js files.

.info

stylesheets[all][] = css/vocabulary-matrix.css
scripts[] = js/script.js

Stylesheets/scripts added here will be loaded on every page, is this really necessary?

Your README exceeds 80 characters and is quite unstructured. Please have a look at typical readme styles: http://drupal.org/node/447604

schema function:

'description' => 'entity_id',
        'type' => 'int',
        'not null' => TRUE,
        'default' => '',

Not null but and type integer but '' for default value ? Allowing NULL or setting 0 as default would make more sense here.
The description key is really for descriptions ;-). Tell us what this value is used for.

But in general I like this, especially the use of API looks good.

not importand but just FYI, you don't have to set the package = Others in your .info as this is the default package anyway

drupal code sniffer has detected bad line endings (http://ventral.org/pareview/httpgitdrupalorgsandboxajaneesh1381316git)

Well, I asked you if it is really necessary to add the css by .info into every page. now you moved it into the formatter and widget hooks with 'every_page' => TRUE. Now it's again included on every page xD.
My question was not rhetoric, I really wanted to know if and why you think it should be added on every page.
If you think it should be added on every page, adding it to the .info as it were before is probably the better way, otherwise you should leave the every_page option and only add the css contextual. For sure (on activated aggregation) showing it on every page would be better.

Seems to me like you got some unused pictures in your image folder. Will you need them soon or can some of them be removed as they'll never be used?

Your module has a functionality that is not probably not clear from the first moment for non-expert users. Some introductions in how to build a simple matrix would be nice to have here. (pro-active-support-issue-prevention ;) )

Line 395: $nid = $form['nid']['#value'];
You can not rely on the existence of the nid key here as the widget form will also be rendered as preview on the fields configuration.

Also default values set on the field configuration page are not saved yet, maybe you can find a way to make this work.

The taxonomy names are not check_plain()'ed before you print them out. As I can not imagine a sense for not filtering these values this should be done to prevent cross site scripting stuff.

Please test the functionality after you made changes, this field could never be saved as you defined a values field in your schema but use 'value' => $item['value'], for saving it. Surely this results in a sql error.
Also note that using the keyword "VALUES" in a sql query will never work as the database wont understand the syntax any longer.
So you got to change the table fieldname to value

Another problem is that using the multiple values option for the field does not work as the values of the first matrix will be used.
Furthermore even if multiple matrix field values are saved there will only be one matrix rendered by the formatter.
You really should implement this functionality! (To do so, make use of the $element[$value] array)

Please only push tested and working code as I had to work by try&fail to figure out whats wrong with your queries

regards

It wasn't that bad at all as your module is quite an interesting thing to me ;)

I think some people probably don't understand the connection between taxonomy and the field from the beginning. That's why a short instructional howto would be nice.

Never ever trust the output of any module as Drupal's way to work is: "Save the original input but output it filtered"!
Only print things out raw if you are really sure that they are clean. Also make sure that the values you give put into arrays should also be checked if it will be handled raw. (for example the #description key for forms will be outputted without filtering)
If I'm not sure about a value I use, I put '';!--"<> in. If it comes out unescaped, you know what's going on.

Also I always test schema/update changes I made by installing it on a clean site and also on a site with the old version working.
At least before creating a release you should do this.

looking forward to your progress

I'm really looking forward to seeing the results!

So...how are things going? Keep up the good work.

Great to hear (not about the hectic times at your day job off course:) ). I'm very curiuos about the first results:)

Hey, it's been a while since you commited. How are things going? You seemed to be on the verge of finishing your project...

I think the module was almost ready to be released, but then development was stopped. How can this module be finalized?
Patrickd, you said the module was interesting for yourself as well and you did some review on it. Is there still much work to do to get this production-ready, you think?

Closing due to lack of activity. Feel free to reopen if you are still working on this application.

@see: Similar Module http://drupal.org/project/views_matrix