Commit 28e5885b authored by IKEDA Soji's avatar IKEDA Soji
Browse files

Refactoring.

  - Changed calling convention of delete_list_member() & delete_list_admin()
  - delete_list_*() may report error via "stash" when the email doesn't exist.
parent 41097c02
......@@ -293,6 +293,9 @@
[%~ ELSIF report_entry == 'unable_to_add_to_database' ~%]
[%|loc(report_param.email)%]Attempts to add some users in database failed.[%END%]
[%~ ELSIF report_entry == 'unable_to_delete_from_database' ~%]
[%|loc(report_param.email)%]Attempts to delete some users in database failed.[%END%]
[%~ END ~%]
[%############################~%]
......
......@@ -4678,7 +4678,7 @@ sub _review_user {
 
foreach my $email (@del_users) {
next if grep { $email eq $_->{email} } @$new_users;
$list->delete_list_admin($role, $email);
$list->delete_list_admin($role, [$email]);
}
foreach
my $user (@{(ref $new_users eq 'ARRAY') ? $new_users : []}) {
......@@ -17348,16 +17348,16 @@ sub do_delete_account {
for my $list (sort keys %{$param->{'which'}}) {
my $l = Sympa::List->new($list, $robot);
# Unsubscribe
$l->delete_list_member('users' => [$email])
$l->delete_list_member([$email])
if $param->{'which'}->{$list}->{'is_subscriber'};
# Remove from the editors
$l->delete_list_admin('editor', $email)
$l->delete_list_admin('editor', [$email])
if $param->{'which'}->{$list}->{'is_editor'};
# Remove from the owners
if ($param->{'which'}->{$list}->{'is_owner'}) {
my @admins = $l->get_admins('owner');
if (scalar(@admins) > 1) {
$l->delete_list_admin('owner', $email);
$l->delete_list_admin('owner', [$email]);
 
# Don't let a list without a privileged admin
my @privileged_admins =
......
......@@ -1527,30 +1527,24 @@ sub send_probe_to_user {
## DEPRECATED: Use Sympa::User::delete_global_user() or $user->expire();
## Delete the indicate list member
## IN : - ref to array
## - option exclude
##
## $list->delete_list_member('users' => \@u, 'exclude' => 1)
## $list->delete_list_member('users' => [$email], 'exclude' => 1)
sub delete_list_member {
$log->syslog('debug2', '(%s, ...)', @_);
my $self = shift;
my %param = @_;
my @u = @{$param{'users'}};
my $exclude = $param{'exclude'};
# Case of deleting: "auto_del" (bounce management), "signoff" (manual
# signoff) or "del" (deleted by admin)?
my $operation = $param{'operation'};
my $users = shift;
my %options = @_;
$log->syslog('debug2', '');
my $exclude = $options{exclude};
my $operation = $options{operation};
my $stash_ref = $options{stash} || [];
my $total = 0;
my $sdm = Sympa::DatabaseManager->instance;
my $sth;
$sdm->begin;
foreach my $who (@u) {
foreach my $who (@$users) {
next unless defined $who and length $who;
$who = Sympa::Tools::Text::canonic_email($who);
......@@ -1563,14 +1557,25 @@ sub delete_list_member {
# Delete record in subscriber_table.
unless (
$sdm
and $sdm->do_prepared_query(
and $sth = $sdm->do_prepared_query(
q{DELETE FROM subscriber_table
WHERE user_subscriber = ? AND
list_subscriber = ? AND robot_subscriber = ?},
$who, $self->{'name'}, $self->{'domain'}
)
) {
$log->syslog('err', 'Unable to remove list member %s', $who);
$log->syslog('err',
'Unable to remove list member %s from list %s',
$who, $self);
push @$stash_ref,
['intern', 'unable_to_delete_from_database', {email => $who}];
next;
} elsif (not $sth->rows) {
$log->syslog('info',
'Unable to remove list member %s from list %s: Not on list',
$who, $self);
push @$stash_ref,
['user', 'user_not_subscriber', {email => $who}];
next;
}
......@@ -1618,23 +1623,29 @@ sub delete_list_member {
## Delete the indicated admin users from the list.
sub delete_list_admin {
$log->syslog('debug2', '(%s, %s, ...)', @_);
my $self = shift;
my $role = shift;
my @u = @_;
my $self = shift;
my $role = shift;
my $users = shift;
my %options = @_;
my $stash_ref = $options{stash} || [];
my $total = 0;
my $sdm = Sympa::DatabaseManager->instance;
my $sth;
$sdm->begin;
foreach my $who (@u) {
$users = [$users] unless ref $users; # compat.
foreach my $who (@$users) {
next unless defined $who and length $who;
$who = Sympa::Tools::Text::canonic_email($who);
# Delete record in ADMIN
unless (
$sdm
and $sdm->do_prepared_query(
and $sth = $sdm->do_prepared_query(
q{DELETE FROM admin_table
WHERE user_admin = ? AND list_admin = ? AND
robot_admin = ? AND role_admin = ?},
......@@ -1642,8 +1653,21 @@ sub delete_list_admin {
$self->{'domain'}, $role
)
) {
$log->syslog('err', 'Unable to remove admin %s of list %s',
$who, $self);
$log->syslog('err', 'Unable to remove %s %s of list %s',
$role, $who, $self);
push @$stash_ref,
[
'intern',
'unable_to_delete_from_database',
{role => $role, email => $who}
];
next;
} elsif (not $sth->rows) {
$log->syslog('info',
'Unable to remove %s %s from list %s: Not on list',
$role, $who, $self);
push @$stash_ref,
['user', 'user_not_admin', {role => $role, email => $who}];
next;
}
......@@ -6168,15 +6192,39 @@ FIXME @todo doc
Note: Since Sympa 6.1.18, this returns an array under array context.
=item delete_list_admin ( ROLE, ARRAY )
=item delete_list_admin ( $role, \@emails, [ stash =E<gt> \@stash ] )
Delete the indicated admin user with the predefined role from the list.
ROLE may be C<'owner'> or C<'editor'>.
$role may be C<'owner'> or C<'editor'>.
=item delete_list_member ( ARRAY )
=item delete_list_member ( \@emails, [ exclude =E<gt> 1 ],
[ operation =E<gt> $operation ], [ stash =E<gt> \@stash ] )
Delete the indicated users from the list.
Options:
=over
=item \@emails
The emails to be deleted.
=item exclude =E<gt> 1
TBD.
=item operation =E<gt> $operation
Case of deleting: C<'auto_del'> (bounce management), C<'signoff'> (manual
signoff) or C<'del'> (deleted by admin).
=item stash =E<gt> \@stash
TBD.
=back
=item delete_list_member_picture ( $email )
Deletes a member's picture file.
......
......@@ -168,11 +168,11 @@ sub _close {
) {
push @users, $user->{'email'};
}
$list->delete_list_member('users' => \@users);
$list->delete_list_member(\@users);
# Remove entries from admin_table.
foreach my $role (qw(editor owner)) {
$list->delete_list_admin($role, $list->get_admins_email($role));
$list->delete_list_admin($role, [$list->get_admins_email($role)]);
}
# Change status & save config.
......
......@@ -57,6 +57,8 @@ sub _twist {
my $sender = $request->{sender};
my $who = $request->{email};
$language->set_lang($list->{'admin'}{'lang'});
unless ($request->{force} or $list->is_subscription_allowed) {
$log->syslog('info', 'List %s not open', $list);
$self->add_stash($request, 'user', 'list_not_open',
......@@ -64,40 +66,27 @@ sub _twist {
return undef;
}
$language->set_lang($list->{'admin'}{'lang'});
# Check if we know this email on the list and remove it. Otherwise
# just reject the message.
my $user_entry = $list->get_list_member($who);
unless (defined $user_entry) {
$self->add_stash($request, 'user', 'user_not_subscriber');
$log->syslog('info', 'DEL %s %s from %s refused, not on list',
$which, $who, $sender);
return undef;
}
# Really delete and rewrite to disk.
unless (
$list->delete_list_member(
'users' => [$who],
'exclude' => ' 1',
'operation' => 'del'
)
) {
my $error =
"Unable to delete user $who from list $which for command 'del'";
Sympa::send_notify_to_listmaster(
$list,
'mail_intern_error',
{ error => $error,
who => $sender,
action => 'Command process',
}
);
$self->add_stash($request, 'intern');
return undef;
my @stash;
$list->delete_list_member(
[$who],
exclude => 1,
operation => 'del',
stash => \@stash
);
foreach my $report (@stash) {
$self->add_stash($request, @$report);
if ($report->[0] eq 'intern') {
Sympa::send_notify_to_listmaster(
$list,
'mail_intern_error',
{ error => $report->[1], #FIXME: Update listmaster tt2
who => $sender,
action => 'Command process',
}
);
}
}
return undef if grep { $_->[0] eq 'user' or $_->[0] eq 'intern' } @stash;
# Only when deletion was done by request, bounce information will be
# cleared. Note that tracking information will be kept.
......
......@@ -635,7 +635,7 @@ sub _expire_users {
$sth->finish;
if ($role eq 'member') {
$list->delete_list_member(users => \@emails);
$list->delete_list_member(\@emails);
} else {
$list->delete_list_admin($role, \@emails);
}
......
......@@ -99,16 +99,20 @@ sub _twist {
# Check if user is already member of the list with their new address
# then we just need to remove the old address.
if ($list->is_list_member($email)) {
unless ($list->delete_list_member('users' => [$current_email])) {
my @stash;
$list->delete_list_member([$current_email], stash => \@stash);
foreach my $report (@stash) {
next
unless grep { $_->[0] eq 'user' or $_->[0] eq 'intern' }
@stash;
$log->syslog('info', 'Could not remove email %s from list %s',
$current_email, $list);
$self->add_stash(
$request, 'user',
'change_member_email_failed_deleting',
{email => $current_email, listname => $list->{'name'}}
);
$log->syslog('info', 'Could not remove email %s from list %s',
$current_email, $list);
}
} else {
unless (
$list->update_list_member(
......@@ -174,20 +178,29 @@ sub _twist {
# then we just need to remove the old address.
if (grep { $_->{role} eq $role and $_->{email} eq $email }
@{$list->get_current_admins || []}) {
unless ($list->delete_list_admin($role, $current_email)) {
my @stash;
$list->delete_list_admin($role, [$current_email],
stash => \@stash);
foreach my $report (@stash) {
next
unless
grep { $_->[0] eq 'user' or $_->[0] eq 'intern' }
@stash;
$log->syslog('info',
'Could not remove email %s from list %s',
$current_email, $list);
$self->add_stash(
$request, 'user',
'change_admin_email_failed_deleting',
'change_member_email_failed_deleting',
{ email => $current_email,
listname => $list->{'name'},
role => $role
}
);
$log->syslog('info',
'Could not remove email %s from list %s',
$current_email, $list);
next;
}
next
if grep { $_->[0] eq 'user' or $_->[0] eq 'intern' }
@stash;
} else {
unless (
$list->update_list_admin(
......
......@@ -55,33 +55,6 @@ sub _twist {
$language->set_lang($list->{'admin'}{'lang'});
# Now check if we know this email on the list and
# remove it if found, otherwise just reject the
# command.
my $user_entry = $list->get_list_member($email);
unless (defined $user_entry) {
unless ($email eq $sender) { # Request from other user?
$self->add_stash($request, 'user', 'user_not_subscriber');
} else {
$self->add_stash($request, 'user', 'not_subscriber');
}
$log->syslog('info', 'SIG %s from %s refused, %s not on list',
$which, $sender, $email);
# Tell the owner somebody tried to unsubscribe.
if ($request->{notify}) {
# Try to find email from same domain or email with same local
# part.
$list->send_notify_to_owner(
'warn-signoff',
{ 'who' => $email,
'gecos' => ($user_entry->{'gecos'} || '')
}
);
}
return undef;
}
# If a list is not 'open' and allow_subscribe_if_pending has been set to
# 'off' returns undef.
unless ($list->{'admin'}{'status'} eq 'open'
......@@ -94,34 +67,34 @@ sub _twist {
return undef;
}
## Really delete and rewrite to disk.
unless (
$list->delete_list_member(
'users' => [$email],
'exclude' => '1',
'operation' => 'signoff',
)
) {
my $error = sprintf 'Unable to delete user %s from list %s',
$email, $list->get_id;
Sympa::send_notify_to_listmaster(
$list,
'mail_intern_error',
{ error => $error,
who => $sender,
action => 'Command process',
}
);
$self->add_stash($request, 'intern');
return undef;
my @stash;
$list->delete_list_member(
[$email],
exclude => 1,
operation => 'signoff',
stash => \@stash
);
foreach my $report (@stash) {
$self->add_stash($request, @$report);
if ($report->[0] eq 'intern') {
Sympa::send_notify_to_listmaster(
$list,
'mail_intern_error',
{ error => $report->[1], #FIXME: Update listmaster tt2
who => $sender,
action => 'Command process',
}
);
}
}
return undef if grep { $_->[0] eq 'user' or $_->[0] eq 'intern' } @stash;
# Notify the owner.
if ($request->{notify}) {
$list->send_notify_to_owner(
'notice',
{ 'who' => $email,
'gecos' => ($user_entry->{'gecos'} || ''),
'gecos' => "",
'command' => 'signoff'
}
);
......
......@@ -417,9 +417,9 @@ sub _twist {
if ($action and $action =~ /do_it/i) {
if ($list->is_list_member($original_rcpt)) {
$list->delete_list_member(
'users' => [$original_rcpt],
'exclude' => ' 1',
'operation' => 'auto_del',
[$original_rcpt],
exclude => 1,
operation => 'auto_del'
);
$log->syslog(
......@@ -502,9 +502,9 @@ sub _twist {
if ($action and $action =~ /do_it/i) {
if ($list->is_list_member($who)) {
$list->delete_list_member(
'users' => [$who],
'exclude' => '1',
'operation' => 'auto_del',
[$who],
exclude => 1,
operation => 'auto_del'
);
$log->syslog(
'notice',
......
......@@ -353,11 +353,8 @@ sub do_delete_subs {
"error in delete_subs command : deletion of $email not allowed"
);
} else {
my $u = $list->delete_list_member(
users => [$email],
operation => 'auto_del'
);
$log->syslog('notice', '--> %s deleted', $email);
$list->delete_list_member([$email], operation => 'auto_del');
$selection{$email} = {};
}
}
......@@ -1311,11 +1308,7 @@ sub _remove_bouncers {
$log->syslog('notice', 'Removing bouncing subsrciber of list %s: %s',
$list, $u);
}
$list->delete_list_member(
users => $users,
exclude => '1',
operation => 'auto_del'
);
$list->delete_list_member($users, exclude => 1, operation => 'auto_del');
return 1;
}
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment