Merge pull request #1536 from ikedas/issue-1535_rebase by ikedas

Fix and improve custom attribute (#1535)
sympa-community · Dec 8, 2022 · 088651f · 088651f
2 parents 5b3924d + 6084f23
commit 088651f
Show file tree

Hide file tree

Showing 14 changed files with 114 additions and 117 deletions.
diff --git a/default/mail_tt2/listowner_notification.tt2 b/default/mail_tt2/listowner_notification.tt2
@@ -60,13 +60,6 @@ Subject: [%"Subscription request to list %1"|loc(list.name)|qencode%]
   AUTH [%keyauth%] ADD [%list.name%] [%who%] [%gecos%]
 [%- END %]
 
-[% IF custom_attribute %]
-[%|loc%]The user has provided the following information:[%END%]
-[% FOREACH attribute = custom_attribute %]
-[% attribute.key %][%|loc%]:[%END%] [% attribute.value.value -%]
-[%END-%]
-[% END %]
-
 [% ELSIF type == 'sigrequest' -%]
 Subject: [%"UNsubscription request from list %1"|loc(list.name)|qencode%]
 

diff --git a/default/web_tt2/compose_mail.tt2 b/default/web_tt2/compose_mail.tt2
@@ -61,13 +61,12 @@
                 <br />
                 <ul>
                     <li> <b>[&#37; listname &#37;]</b>[%|loc%]: the list name; always available.[%END%]</li>
-                    <li> <b>[&#37; robot &#37;]</b>[%|loc%]: the name of the host the list is installed on; always available.[%END%]</li>
+                    <li> <b>[&#37; domain &#37;]</b>[%|loc%]: the name of the host the list is installed on; always available.[%END%]</li>
                     <li> <b>[&#37; user.email &#37;]</b>[%|loc%]: the user email; always available.[%END%]</li>
                     <li> <b>[&#37; user.gecos &#37;]</b>[%|loc%]: the user name associated to her email; always available.[%END%]</li>
-                    <li> <b>[&#37; user.friendly_date &#37;]</b>[%|loc%]: the - human readable - user's subscription date; always available.[%END%]</li>
-                    <li> <b>[&#37; user.custom_attribute.title.value &#37;]</b>[%|loc%]: can be anything you like; available if you defined a user custom attribute named "title" (see the list configuration, section "Miscellaneous").[%END%]</li>
-                    <li> <b>[&#37; user.custom_attribute.name.value &#37;]</b>[%|loc%]: can be anything you like; available if you defined a user custom attribute named "name".[%END%]</li>
-                    <li> <b>[&#37; user.custom_attribute.organization.value &#37;]</b>[%|loc%]: can be anything you like; available if you defined a user custom attribute named "organization".[%END%]</li>
+                    <li> <b>[&#37; user.attrib.title &#37;]</b>[%|loc%]: can be anything you like; available if you defined a user custom attribute named "title" (see the list configuration, section "Miscellaneous").[%END%]</li>
+                    <li> <b>[&#37; user.attrib.name &#37;]</b>[%|loc%]: can be anything you like; available if you defined a user custom attribute named "name".[%END%]</li>
+                    <li> <b>[&#37; user.attrib.organization &#37;]</b>[%|loc%]: can be anything you like; available if you defined a user custom attribute named "organization".[%END%]</li>
                 </ul>
             [% END %]
         [%  END %]

diff --git a/default/web_tt2/confirm_action.tt2 b/default/web_tt2/confirm_action.tt2
@@ -543,7 +543,7 @@
         <input type="hidden" name="gecos" value="[% gecos %]" />
         [% FOREACH i = custom_attribute ~%]
             <input type="hidden" name="custom_attribute.[% i.key %]"
-             value="[% i.value.value.replace('\n', '&#10;') %]" />
+             value="[% i.value.replace('\n', '&#10;') %]" />
         [%~ END %]
     [%~ END %]
 

diff --git a/default/web_tt2/edit_attributes.tt2 b/default/web_tt2/edit_attributes.tt2
@@ -11,7 +11,7 @@
         <div class="small-9 large-10 columns">
             [% FOREACH k IN list_conf.custom_attribute ~%]
                 [% SET ca_id = "custom_attribute.${k.id}" ~%]
-                [% SET ca_value = subscriber.custom_attribute.item(k.id).value ~%]
+                [% SET ca_value = subscriber.attrib.item(k.id) ~%]
 
                 <label for="[% ca_id %]">
                     [%|loc(k.name)%]%1:[% END %]

diff --git a/default/web_tt2/subindex.tt2 b/default/web_tt2/subindex.tt2
@@ -52,8 +52,8 @@
                             [% FOREACH ca_k IN listconf.custom_attribute ~%]
                                 <p>
                                     <b>[% ca_k.name %][%|loc%]:[%END%] </b>
-                                    [% IF sub.value.custom_attribute.item(ca_k.id).value ~%]
-                                        [% sub.value.custom_attribute.item(ca_k.id).value %]
+                                    [% IF sub.value.attrib.item(ca_k.id) ~%]
+                                        [% sub.value.attrib.item(ca_k.id) %]
                                     [%~ ELSE ~%]
                                         -
                                     [%~ END %]

diff --git a/default/web_tt2/subscriber_table.tt2 b/default/web_tt2/subscriber_table.tt2
@@ -179,7 +179,7 @@
                     [% FOREACH ca_k IN listconf.custom_attribute %]
                         <td>
                             [% SET id = ca_k.id %]
-                            [% u.custom_attribute.$id.value %]
+                            [% u.attrib.$id %]
                         </td>
                     [% END %]
                 [% END %]

diff --git a/src/cgi/wwsympa.fcgi.in b/src/cgi/wwsympa.fcgi.in
@@ -2071,9 +2071,8 @@ sub get_parameters {
         } elsif ($pname =~ /^custom_attribute(.*)$/) {
             my $key = $tokens[1];
             $regexp = $in_regexp{'custom_attribute'};
-            # $log->syslog('debug2', '() (%s)(%s) %s %s %s', $p, $key, $name,
-            #     $in{$p}, $Conf::Conf{$key}->{type});
-            $custom_attribute->{$key} = {value => $in{$p}};
+            $custom_attribute ||= {};
+            $custom_attribute->{$key} = $in{$p};
             undef $in{$p};
         } elsif ($pname eq 'plugin' and $#tokens >= 2) {
             my $plugin_name = $tokens[1];
@@ -5307,7 +5306,7 @@ sub do_set {
     } else {
         $update->{'gecos'} = undef;
     }
-    $update->{'custom_attribute'} = $in{custom_attribute}
+    $update->{attrib} = $in{custom_attribute}
         if $in{custom_attribute};
 
     unless ($list->update_list_member($email, $update)) {
@@ -5345,7 +5344,7 @@ sub _check_custom_attribute {
     my $isOK              = 1;
 
     foreach my $ca (@custom_attributes) {
-        my $value = $custom_attribute->{$ca->{id}}{value};
+        my $value = $custom_attribute->{$ca->{id}};
         if (    $ca->{optional}
             and $ca->{optional} eq 'required'
             and not(defined $value and length $value)) {

diff --git a/src/lib/Sympa/DataSource/SQL.pm b/src/lib/Sympa/DataSource/SQL.pm
@@ -76,7 +76,7 @@ sub _open {
                     next unless defined $email and length $email;
                     $email =~ s/[\t\r\n]+/ /g;
 
-                    print $tmpfh "%s\t%s\n", $email,
+                    printf $tmpfh "%s\t%s\n", $email,
                         Sympa::Tools::Data::encode_custom_attribute($row);
                 }
             } else {

diff --git a/src/lib/Sympa/List.pm b/src/lib/Sympa/List.pm
@@ -491,11 +491,13 @@ sub dump_users {
             $user = $self->get_next_list_member()
         ) {
             foreach my $k (sort keys %map_field) {
-                if ($k eq 'custom_attribute') {
+                if ($k eq 'attrib') {
                     next unless ref $user->{$k} eq 'HASH' and %{$user->{$k}};
                     my $encoded = Sympa::Tools::Data::encode_custom_attribute(
                         $user->{$k});
                     printf $lock_fh "%s %s\n", $k, $encoded;
+                    # Compat.<=6.2.70.
+                    printf $lock_fh "custom_attribute %s\n", $encoded;
                 } else {
                     next unless defined $user->{$k} and length $user->{$k};
                     printf $lock_fh "%s %s\n", $k, $user->{$k};
@@ -2026,6 +2028,7 @@ sub _map_list_member_cols {
         email       => 'user_subscriber',
         startdate   => 'suspend_start_date_subscriber',
         enddate     => 'suspend_end_date_subscriber',
+        attrib      => 'custom_attribute_subscriber',
     );
 
     my $fields =
@@ -2099,15 +2102,14 @@ sub get_list_member {
         $user->{'visibility'}  ||= 'noconceal';
         $user->{'update_date'} ||= $user->{'date'};
 
-        $log->syslog(
-            'debug2',
-            'Custom_attribute = (%s)',
-            $user->{custom_attribute}
-        );
-        if (defined $user->{custom_attribute}) {
-            $user->{'custom_attribute'} =
-                Sympa::Tools::Data::decode_custom_attribute(
-                $user->{'custom_attribute'});
+        $log->syslog('debug2', 'attrib = (%s)', $user->{attrib});
+        if (defined $user->{attrib}) {
+            my $ca =
+                Sympa::Tools::Data::decode_custom_attribute($user->{attrib});
+            $user->{attrib} = $ca;
+            # Compat.<=6.2.70.
+            $user->{custom_attribute} =
+                {map { ($_ => {value => $ca->{$_}}) } keys %$ca};
         }
 
         # Compat.<=6.2.44 FIXME: needed?
@@ -2238,10 +2240,13 @@ sub get_first_list_member {
         $user->{'visibility'}  ||= 'noconceal';
         $user->{'update_date'} ||= $user->{'date'};
 
-        if (defined $user->{custom_attribute}) {
-            $user->{'custom_attribute'} =
-                Sympa::Tools::Data::decode_custom_attribute(
-                $user->{'custom_attribute'});
+        if (defined $user->{attrib}) {
+            my $ca =
+                Sympa::Tools::Data::decode_custom_attribute($user->{attrib});
+            $user->{attrib} = $ca;
+            # Compat.<=6.2.70.
+            $user->{custom_attribute} =
+                {map { ($_ => {value => $ca->{$_}}) } keys %$ca};
         }
 
         # Compat.<=6.2.44 FIXME: needed?
@@ -2289,18 +2294,13 @@ sub get_next_list_member {
         $user->{'visibility'}  ||= 'noconceal';
         $user->{'update_date'} ||= $user->{'date'};
 
-        if (defined $user->{custom_attribute}) {
-            my $custom_attr = Sympa::Tools::Data::decode_custom_attribute(
-                $user->{'custom_attribute'});
-            unless (defined $custom_attr) {
-                $log->syslog(
-                    'err',
-                    "Failed to parse custom attributes for user %s, list %s",
-                    $user->{'email'},
-                    $self
-                );
-            }
-            $user->{'custom_attribute'} = $custom_attr;
+        if (defined $user->{attrib}) {
+            my $ca =
+                Sympa::Tools::Data::decode_custom_attribute($user->{attrib});
+            $user->{attrib} = $ca;
+            # Compat.<=6.2.70.
+            $user->{custom_attribute} =
+                {map { ($_ => {value => $ca->{$_}}) } keys %$ca};
         }
 
         # Compat.<=6.2.44 FIXME: needed?
@@ -2519,7 +2519,16 @@ sub get_first_bouncing_list_member {
         $log->syslog('err',
             'Warning: Entry with empty email address in list %s',
             $self->{'name'})
-            unless defined $user->{'email'} and length $user->{'email'};
+            unless length($user->{email} // '');
+
+        if (defined $user->{attrib}) {
+            my $ca =
+                Sympa::Tools::Data::decode_custom_attribute($user->{attrib});
+            $user->{attrib} = $ca;
+            # Compat.<=6.2.70.
+            $user->{custom_attribute} =
+                {map { ($_ => {value => $ca->{$_}}) } keys %$ca};
+        }
 
         # Compat.<=6.2.44 FIXME: needed?
         $user->{'included'} = 1
@@ -2552,12 +2561,15 @@ sub get_next_bouncing_list_member {
         $log->syslog('err',
             'Warning: Entry with empty email address in list %s',
             $self->{'name'})
-            if (!$user->{'email'});
+            unless length($user->{email} // '');
 
-        if (defined $user->{custom_attribute}) {
-            $user->{'custom_attribute'} =
-                Sympa::Tools::Data::decode_custom_attribute(
-                $user->{'custom_attribute'});
+        if (defined $user->{attrib}) {
+            my $ca =
+                Sympa::Tools::Data::decode_custom_attribute($user->{attrib});
+            $user->{attrib} = $ca;
+            # Compat.<=6.2.70.
+            $user->{custom_attribute} =
+                {map { ($_ => {value => $ca->{$_}}) } keys %$ca};
         }
 
         # Compat.<=6.2.44 FIXME: needed?
@@ -2688,18 +2700,13 @@ sub get_members {
         $user->{visibility}  ||= 'noconceal';
         $user->{update_date} ||= $user->{date};
 
-        if (defined $user->{custom_attribute}) {
-            my $custom_attr = Sympa::Tools::Data::decode_custom_attribute(
-                $user->{custom_attribute});
-            unless (defined $custom_attr) {
-                $log->syslog(
-                    'err',
-                    "Failed to parse custom attributes for user %s, list %s",
-                    $user->{email},
-                    $self
-                );
-            }
-            $user->{custom_attribute} = $custom_attr;
+        if (defined $user->{attrib}) {
+            my $ca =
+                Sympa::Tools::Data::decode_custom_attribute($user->{attrib});
+            $user->{attrib} = $ca;
+            # Compat.<=6.2.70.
+            $user->{custom_attribute} =
+                {map { ($_ => {value => $ca->{$_}}) } keys %$ca};
         }
 
         # Compat.<=6.2.44 FIXME: needed?
@@ -2897,7 +2904,7 @@ sub update_list_member {
             unless $map_field{$field};
 
         push @set_list, $map_field{$field};
-        if ($field eq 'custom_attribute') {
+        if ($field eq 'attrib') {
             push @val_list,
                 Sympa::Tools::Data::encode_custom_attribute($value);
         } elsif ($numeric_field{$map_field{$field}}) {
@@ -3210,7 +3217,7 @@ sub add_list_member {
                 unless $map_field{$field};
 
             push @set_list, $map_field{$field};
-            if ($field eq 'custom_attribute') {
+            if ($field eq 'attrib') {
                 push @val_list,
                     Sympa::Tools::Data::encode_custom_attribute($value);
             } elsif ($numeric_field{$map_field{$field}}) {
@@ -3713,8 +3720,9 @@ sub restore_users {
                     if (/^\s*(suspend|subscribed|included)\s+(\S+)\s*$/) {
                         # Note: "included" is kept for comatibility.
                         ($1 => !!$2);
-                    } elsif (/^\s*(custom_attribute)\s+(.+)\s*$/) {
-                        my $k = $1;
+                    } elsif (/^\s*(attrib|custom_attribute)\s+(.+)\s*$/) {
+                        # 'custom_attribute' was obsoleted on 6.2.71b.1.
+                        my $k = 'attrib';
                         my $decoded =
                             Sympa::Tools::Data::decode_custom_attribute($2);
                         ($decoded and %$decoded) ? ($k => $decoded) : ();

diff --git a/src/lib/Sympa/Request/Handler/add.pm b/src/lib/Sympa/Request/Handler/add.pm
@@ -73,9 +73,13 @@ sub _twist {
 
         $list->add_list_member(
             {   email => $email,
+                (   defined $request->{custom_attribute}
+                    ? (attrib => $request->{custom_attribute})
+                    : ()
+                ),
                 map { ($_ => $request->{$_}) }
                     grep { defined $request->{$_} }
-                    qw(gecos reception visibility custom_attribute)
+                    qw(gecos reception visibility)
             },
             stash => \@stash
         );

diff --git a/src/lib/Sympa/Request/Handler/include.pm b/src/lib/Sympa/Request/Handler/include.pm
@@ -722,7 +722,7 @@ sub _update_custom_attribute {
 
         my $member = $list->get_list_member($email);
         next unless $member;
-        my $ca = $member->{custom_attribute} || {};
+        my $ca = $member->{attrib} || {};
 
         my $changed;
         foreach my $key (sort keys %{$ca_update || {}}) {
@@ -737,7 +737,7 @@ sub _update_custom_attribute {
         }
         next unless $changed;
 
-        $list->update_list_member($email, custom_attribute => $ca_update);
+        $list->update_list_member($email, attrib => $ca_update);
         $updated++;
     }
 

diff --git a/src/lib/Sympa/Request/Handler/subscribe.pm b/src/lib/Sympa/Request/Handler/subscribe.pm
@@ -84,7 +84,7 @@ sub _twist {
 
     my @stash;
     $list->add_list_member(
-        {email => $email, gecos => $comment, custom_attribute => $ca},
+        {email => $email, gecos => $comment, ($ca ? (attrib => $ca) : ())},
         stash => \@stash);
     foreach my $report (@stash) {
         $self->add_stash($request, @$report);

diff --git a/src/lib/Sympa/Tools/Data.pm b/src/lib/Sympa/Tools/Data.pm
@@ -311,6 +311,7 @@ sub sort_uniq {
 # Old name: Sympa::List::parseCustomAttribute().
 sub decode_custom_attribute {
     my $xmldoc = shift;
+
     return undef unless defined $xmldoc and length $xmldoc;
 
     my $parser = XML::LibXML->new();
@@ -332,31 +333,33 @@ sub decode_custom_attribute {
     foreach my $ca (@custom_attr) {
         my $id    = Encode::encode_utf8($ca->getAttribute('id'));
         my $value = Encode::encode_utf8($ca->getElementsByTagName('value'));
-        $ca{$id} = {value => $value};
+        $ca{$id} = $value;
     }
+
     return \%ca;
 }
 
 # Old name: Sympa::List::createXMLCustomAttribute().
 sub encode_custom_attribute {
-    my $custom_attr = shift;
+    my $ca = shift;
+
     return
         '<?xml version="1.0" encoding="UTF-8" ?><custom_attributes></custom_attributes>'
-        if (not defined $custom_attr);
-    my $XMLstr = '<?xml version="1.0" encoding="UTF-8" ?><custom_attributes>';
-    foreach my $k (sort keys %{$custom_attr}) {
-        my $value = $custom_attr->{$k}{value};
-        $value = '' unless defined $value;
-
-        $XMLstr .=
-              "<custom_attribute id=\"$k\"><value>"
-            . Sympa::Tools::Text::encode_html($value, '\000-\037')
-            . "</value></custom_attribute>";
-    }
-    $XMLstr .= "</custom_attributes>";
-    $XMLstr =~ s/\s*\n\s*/ /g;
+        unless $ca;
+    my $ret =
+        '<?xml version="1.0" encoding="UTF-8" ?><custom_attributes>' . join(
+        '',
+        map {
+            sprintf
+                '<custom_attribute id="%s"><value>%s</value></custom_attribute>',
+                $_,
+                Sympa::Tools::Text::encode_html($ca->{$_} // '', '\000-\037');
+            }
+            sort keys %$ca
+        ) . '</custom_attributes>';
+    $ret =~ s/\s*\n\s*/ /g;
 
-    return $XMLstr;
+    return $ret;
 }
 
 my $language = Sympa::Language->instance;