Feedback on FormPivotPlugin

The zip file is constructed differently to the other plugins; the files appear in a subdirectory, not directly in their destination directories.

        0  04-16-02 20:37   FormPivotPlugin/pub/
        0  04-16-02 20:37   FormPivotPlugin/pub/TWiki/
        0  04-16-02 20:37   FormPivotPlugin/pub/TWiki/FormPivotPlugin/
     5306  04-16-02 20:34   FormPivotPlugin/pub/TWiki/FormPivotPlugin/formpivot-
example.zip
     5561  04-16-02 20:34   FormPivotPlugin/pub/TWiki/FormPivotPlugin/formpivot-
example.zip,v
        0  04-16-02 20:37   FormPivotPlugin/lib/
        0  04-16-02 20:37   FormPivotPlugin/lib/TWiki/
        0  04-16-02 20:37   FormPivotPlugin/lib/TWiki/Plugins/
     7731  04-15-02 19:30   FormPivotPlugin/lib/TWiki/Plugins/FormPivotPlugin.pm
        0  04-16-02 20:37   FormPivotPlugin/data/
        0  04-16-02 20:37   FormPivotPlugin/data/TWiki/
     2632  04-16-02 20:37   FormPivotPlugin/data/TWiki/FormPivotPlugin.txt
     5871  04-16-02 20:37   FormPivotPlugin/data/TWiki/FormPivotPlugin.txt,v
        0  04-16-02 20:37   FormPivotPlugin/

Arguably this is better than the others but it is inconsistant and therefore confusing.

Ideally there would be a install-plugin script (probably should be part of the management interface). This would:

1. Change to a empty subdir
2. Download the plugin
3. Ensure the plugin consists only of directories
4. Install the plugin
5. Optionally report the plugin being installed for stats purposes.

-- MartinCleaver - 19 Jul 2002

Very nice job. It will be a real timesaver. It already allowed me to answer a couple of questions that my boss had. However, I had a number of problems with the plugin. I run RedHat linux with apache 1.3.2, modperl/PerlRun with perl 5.6.1. PerlWarn and PerlTaint are turned on. My patch is attached to this topic FormPivot.diff. I will describe the portions of the patch here.

I had to change the searchval regexp to fix this error in my twiki logs: egrep: Invalid content of \{\}. In my setup at least I had to add another \ to the regexp as seen below. Without this fix, I didn't get any data.

*** 109,115 ****

     # Find all topics with this form.

!    my $searchVal = "%META:FORM\{.*name=\\\"$form\\\".*\}%";

     my $search = &TWiki::Search::searchWeb( "1", $web, $searchVal, "",
            "", "on", "", "",
--- 109,115 ----

     # Find all topics with this form.

!    my $searchVal = "%META:FORM\\{.*name=\\\"$form\\\".*\\}%";

     my $search = &TWiki::Search::searchWeb( "1", $web, $searchVal, "",

To deal with the warning: = [Fri Aug  9 16:16:53 2002] view: Use of uninitialized value in numeric le (<) at ../lib/TWiki/Plugins/FormPivotPlugin.pm line 125.== I added an initializer to all the my $i declarations in for loop. This warning was generated in three places. I added the initializer in all three.

*** 122,128 ****
     my @found = ();
     my @foundTopic = ();

!    for( my $i; $i<=$#fields; $i++ ) {
         my %hash = ();
         my %hashTopic = ();
         $found[$i] = \%hash;
--- 122,128 ----
     my @found = ();
     my @foundTopic = ();

!    for( my $i=0; $i<=$#fields; $i++ ) {
         my %hash = ();
         my %hashTopic = ();
         $found[$i] = \%hash;

I had a problem when clicking on the field values in the table. They would execute the query, but no items were displayed in the topic. I tracked that down to the original search val requiring the key be right at the beginning of the value string, which isn't right if its a multi-value (multi-checkbox) value. My fix was to change the regexp so that any string not including a " could preceed the key we were looking for. I figured out using the %22 encoding in the url by accident, but it works.

*** 166,172 ****
                 # FIXME should use field name not title without spaces
                 $field =~ s/\s*//go;
                 # Problems passing = and " to URL
!                my $searchVal = "%META:FIELD\{.*name..$field..*value..$key.*\}%";
                 $title = "$title";
                 $table .= "| $title | " . $found[$i]->{$key} . " |\n";
             }
--- 167,175 ----
                 # FIXME should use field name not title without spaces
                 $field =~ s/\s*//go;
                 # Problems passing = and " to URL
!              # Key may not be first in multi select value. Extend RE
!              # to match in with any non " preceeding characters.
!                my $searchVal = qq(%META:FIELD\\{.*name..$field..*value..[^%22]*$key.*\\}%);
                 $title = "$title";
                 $table .= "| $title | " . $found[$i]->{$key} . " |\n";
             }

I also got a warning = [Fri Aug  9 16:16:53 2002] view: Use of uninitialized value in array element at ../lib/TWiki/Plugins/FormPivotPlugin.pm line 128.= showing up at many different lines. I think that was caused when $meta->findOne didn't find anything and undef was returned. Then the next line unconditionally dereferences it. My fix was to initalize $field0{"value"} to the empty string if it was undefined. This is really a bad kludge, but I didn't understand the code well enough to trust putting in an if(defined() {...} block.

*** 132,141 ****

     foreach my $formTopic ( split( /\s/, $search ) ) {
         my( $meta, $text ) = TWiki::Store::readTopic( $web, $formTopic );
!        for( my $i; $i<=$#fields; $i++ ) {
             my $name = $fields[$i];
             $name =~ s/\s*//go;
             my %field0 = $meta->findOne( "FIELD", $name );
             my @values = split( /,/, $field0{"value"} );
             foreach my $value ( @values ) {
                 $value =~ s/^\s*//go; # Trim left
--- 132,142 ----

     foreach my $formTopic ( split( /\s/, $search ) ) {
         my( $meta, $text ) = TWiki::Store::readTopic( $web, $formTopic );
!        for( my $i=0; $i<=$#fields; $i++ ) {
             my $name = $fields[$i];
             $name =~ s/\s*//go;
             my %field0 = $meta->findOne( "FIELD", $name );
+          $field0{"value"} = '' if ! defined($field0{"value"});
             my @values = split( /,/, $field0{"value"} );
             foreach my $value ( @values ) {
                 $value =~ s/^\s*//go; # Trim left

The last part of the patch again doubles up on \\ protecting the { and } characters in the pattern. I did this analogous to the other one. I don't think I actually had a test that checked this portion of the code.

*** 193,200 ****
                foreach my $rowTopic ( keys %$hashRowTopics ) {
                   $count++ if( $hashColTopics->{$rowTopic} );
                }
!               my $searchVal = "%META:FIELD\{.*name%3D.$fieldRow..*value..$valueRow.*\}%%3B" .
!                               "%META:FIELD\{.*name%3D.$fieldCol..*value..$valueCol.*\}%";
                #my $searchVal = "FIELD,$fieldRow,value,$valueRow,FIELD,$fieldCol,value,$valueCol";
                my $link = "$count";
                $pivot .= " $link |";
--- 196,203 ----
                foreach my $rowTopic ( keys %$hashRowTopics ) {
                   $count++ if( $hashColTopics->{$rowTopic} );
                }
!               my $searchVal = "%META:FIELD\\{.*name%3D.$fieldRow..*value..$valueRow.*\\}%%3B" .
!                               "%META:FIELD\\{.*name%3D.$fieldCol..*value..$valueCol.*\\}%";
                #my $searchVal = "FIELD,$fieldRow,value,$valueRow,FIELD,$fieldCol,value,$valueCol";
                my $link = "$count";
                $pivot .= " $link |";

Now for the $50 question? Why did I have to put in \\ for the regexps. Obviously they were working on the author's server. Is this a result of using gnu grep/egrep?

- JohnRouillard - 10 Aug 2002

I have noticed a warning message complaining about a my $title redefining the $title variable already defined in the same scope. Removing the my as follows stops the warning. Here is the patch:

--- FormPivotPlugin.pm.orig   2002-04-15 19:30:20.000000000 +0200
+++ FormPivotPlugin.pm   2002-08-16 09:34:30.000000000 +0200
@@ -162,7 +162,8 @@
                $field =~ s/\s*//go;
                # Problems passing = and " to URL
                my $searchVal = "%META:FIELD\{.*name..$field..*value..$key.*\}%";
-               my $title = "<a href=\"" . &TWiki::getScriptUrl( $web, "", "search" ) . "?regex=on&search=$searchVal&nosearch=on\">$title</a>";
+#AS               my $title = "<a href=\"" . &TWiki::getScriptUrl( $web, "", "search" ) . "?regex=on&search=$searchVal&nosearch=on\">$title</a>";
+               $title = "<a href=\"" . &TWiki::getScriptUrl( $web, "", "search" ) . "?regex=on&search=$searchVal&nosearch=on\">$title</a>";
                $table .= "| $title | " . $found[$i]->{$key} . " |\n";
            }
            $pivot .= "$table";

-- AndreaSterbini - 16 Aug 2002