Unverified Commit 3721115e authored by IKEDA Soji's avatar IKEDA Soji Committed by GitHub
Browse files

Merge pull request #905 from ikedas/issue-858_trial-3 by ikedas

Fixes issues about synchronization of owners/moderators (#857 & #858)
parents 4e2730f4 49fb290a
......@@ -242,10 +242,10 @@
[%~ ELSIF report_entry == 'unable_to_load_create_list_templates' ~%]
[%|loc%]Unable to load create_list templates.[%END%]
[%~ ELSIF report_entry == 'sync_include_failed' ~%]
[%~ ELSIF report_entry == 'sync_include_failed' # No longer used ~%]
[%|loc%]Failed to include members[%END%]
[%~ ELSIF report_entry == 'sync_include_admin_failed' ~%]
[%~ ELSIF report_entry == 'sync_include_admin_failed' # No longer used ~%]
[%|loc%]Failed to include list admins[%END%]
[%~ ELSIF report_entry == 'no_owner_defined' ~%]
......@@ -408,12 +408,27 @@
[%~ ELSIF report_entry == 'no_msg_document' ~%]
[%|loc(report_param.list)%]No message and no document to moderate for list %1[%END%]
[%~ ELSIF report_entry == 'subscribers_updated' ~%]
[%|loc%]The list of list members have been built/updated.[%END%]
[%~ ELSIF report_entry == 'member_updated' # No longer used ~%]
[%|loc%]The list of list subscribers have been built/updated.[%END%]
[%~ ELSIF report_entry == 'subscribers_updated_soon' ~%]
[%~ ELSIF report_entry == 'owner_updated' # No longer used ~%]
[%|loc%]The list of list owners have been built/updated.[%END%]
[%~ ELSIF report_entry == 'editor_updated' # No longer used ~%]
[%|loc%]The list of list moderators have been built/updated.[%END%]
[%~ ELSIF report_entry == 'subscribers_updated_soon' # No longer used ~%]
[%|loc%]The list of list members will be built/updated soon (a few minutes).[%END%]
[%~ ELSIF report_entry == 'member_updated_soon' ~%]
[%|loc%]The list of list subscribers will be built/updated soon (in an hour). If you wish immediate update, click "Synchronize members with data sources".[%END%]
[%~ ELSIF report_entry == 'owner_updated_soon' ~%]
[%|loc%]The list of list owners will be built/updated soon (in an hour). If you wish immediate update, click "Synchronize owners with data sources".[%END%]
[%~ ELSIF report_entry == 'editor_updated_soon' ~%]
[%|loc%]The list of moderators will be built/updated soon (in an hour). If you wish immediate update, click "Synchronize moderators with data sources".[%END%]
[%~ ELSIF report_entry == 'subscribers_noticed_deleted_topics' ~%]
[%|loc%]Concerned subscribers have been notified about deleted topics.[%END%]
......
......@@ -55,12 +55,13 @@
</a>
<br />
[% IF may_sync %]
[% IF may_include.member %]
<br />
<form name="synchronize_list_members" action="[% path_cgi %]" method="post">
<input class="MainMenuLinks heavyWork" type="submit" name="action_sync_include"
value="[%|loc%]Synchronize members with data sources[%END%]" />
<input type="hidden" name="list" value="[% list %]"/>
<input type="hidden" name="role" value="member"/>
</form>
[% END %]
<hr>
......@@ -456,6 +457,21 @@
</fieldset>
</form>
[% IF may_include.$role ~%]
<br />
<form name="sync_include" action="[% path_cgi %]" method="post">
<input class="MainMenuLinks heavyWork" type="submit"
name="action_sync_include"
value="[% IF role == 'owner' ~%]
[%|loc%]Synchronize owners with data sources[%END%]
[%~ ELSE ~%]
[%|loc%]Synchronize moderators with data sources[%END%]
[%~ END %]" />
<input type="hidden" name="list" value="[% list %]"/>
<input type="hidden" name="role" value="[% role %]"/>
</form>
[%~ END %]
[%# AJAX modal dialog ~%]
<div id="edit" class="reveal medium" data-reveal role="dialog"
aria-labelledby="[%|loc%]View user[%END%]" aria-hidden="true">
......
......@@ -2841,10 +2841,21 @@ sub check_param_out {
$param->{'may_post_reason'} = $reason;
}
 
if ( $list->has_include_data_sources()
&& $param->{'is_owner'}) {
$param->{'may_sync'} = 1;
}
$param->{'may_include'} = {
member => (
$param->{'is_owner'} and $list->has_data_sources('member')
),
owner => (
$param->{'is_privileged_owner'}
and $list->has_data_sources('owner')
),
editor => (
$param->{'is_privileged_owner'}
and $list->has_data_sources('editor')
),
};
# Compat.<=6.2.54
$param->{'may_sync'} = $param->{'may_include'}{'member'};
}
 
## Should Not be used anymore ##
......@@ -4781,20 +4792,6 @@ sub _review_member {
my @additional_fields = split ',',
$Conf::Conf{'db_additional_subscriber_fields'};
 
## Members list synchronization if list has included data sources.
if ($list->has_include_data_sources()) {
my $sync_result = $list->on_the_fly_sync_include(use_ttl => 1);
unless (defined $sync_result) {
Sympa::WWW::Report::reject_report_web('intern',
'sync_include_failed',
{}, $param->{'action'}, $list, $param->{'user'}{'email'},
$robot);
} elsif ($sync_result) {
Sympa::WWW::Report::notice_report_web('subscribers_updated', {},
$param->{'action'});
}
}
# Members list
# Some review pages may be empty while viewed by subscribers.
my @members = $list->get_members(
......@@ -10567,9 +10564,13 @@ sub do_edit_list {
{}, $param->{'action'});
}
 
my $data_source_updated = 1
my $data_source_updated_member = 1
if grep { $config->get_change($_) }
grep { $_ =~ /\Ainclude_/ or $_ eq 'ttl' } $config->keys;
grep { $_ =~ /\Ainclude_/ or $_ eq 'member_include' } $config->keys;
my $data_source_updated_owner = 1
if $config->get_change('owner_include');
my $data_source_updated_editor = 1
if $config->get_change('editor_include');
 
# Update config in memory.
$config->commit;
......@@ -10603,33 +10604,17 @@ sub do_edit_list {
return undef;
}
 
## If list has included data sources, update them and delete sync_include
## task.
if ($data_source_updated) {
if ($list->on_the_fly_sync_include('use_ttl' => 0)) {
Sympa::WWW::Report::notice_report_web('subscribers_updated', {},
$param->{'action'});
} else {
Sympa::WWW::Report::reject_report_web('intern',
'sync_include_failed',
{}, $param->{'action'}, $list, $param->{'user'}{'email'},
$robot);
}
if ($data_source_updated_member) {
Sympa::WWW::Report::notice_report_web('member_updated_soon', {},
$param->{'action'});
}
# Call sync_include_admin if there are changes about owners or editors.
#FIXME:Update only when owner or editor was updated.
unless (defined $list->sync_include_admin) {
Sympa::WWW::Report::reject_report_web('intern',
'sync_include_admin_failed', {}, $param->{'action'}, $list,
$param->{'user'}{'email'}, $robot);
wwslog('info', '() Failed');
web_db_log(
{ 'status' => 'error',
'error_type' => 'internal'
}
);
return undef;
if ($data_source_updated_owner) {
Sympa::WWW::Report::notice_report_web('owner_updated_soon', {},
$param->{'action'});
}
if ($data_source_updated_editor) {
Sympa::WWW::Report::notice_report_web('editor_updated_soon', {},
$param->{'action'});
}
 
Sympa::WWW::Report::notice_report_web('list_config_updated', {},
......@@ -16509,16 +16494,43 @@ sub do_wsdl {
 
## Synchronize list members with data sources
sub do_sync_include {
wwslog('info', '(%s)', $in{'list'});
wwslog('info', '(%s, %s)', $in{'list'}, $in{'role'});
 
unless ($list->sync_include()) {
Sympa::WWW::Report::reject_report_web('intern',
'sync_include_failed', {},
$param->{'action'}, $list, $param->{'user'}{'email'}, $robot);
my $role = $in{'role'} || 'member'; # Compat.<=6.2.54
$in{'page'} = $role unless $role eq 'member';
$param->{'list'} = $list->{'name'};
$param->{'role'} = $role;
$param->{'page'} = $role unless $role eq 'member';
my $spindle = Sympa::Spindle::ProcessRequest->new(
context => $list,
action => 'include',
role => $role,
sender => $param->{'user'}{'email'},
scenario_context => {skip => 1},
);
unless ($spindle and $spindle->spin) {
wwslog('err', 'Failed to sync role %s of list %s with data sources',
$role, $list);
return undef;
}
Sympa::WWW::Report::notice_report_web('subscribers_updated', {},
$param->{'action'});
foreach my $report (@{$spindle->{stash} || []}) {
if ($report->[1] eq 'notice') {
Sympa::WWW::Report::notice_report_web(@{$report}[2, 3],
$param->{'action'});
} else {
Sympa::WWW::Report::reject_report_web(@{$report}[1 .. 3],
$param->{action});
}
}
unless (@{$spindle->{stash} || []}) {
Sympa::WWW::Report::notice_report_web('performed', {},
$param->{'action'});
web_db_log({'parameters' => $in{'email'}, 'status' => 'success'});
}
return 'review';
}
 
......
......@@ -71,12 +71,8 @@ my @sources_providing_listmembers = qw/
include_sympa_list
/;
#XXX include_admin
my @more_data_sources = qw/
editor_include
owner_include
member_include
/;
# No longer used.
#my @more_data_sources;
# All non-pluggable sources are in the admin user file
# NO LONGER USED.
......@@ -407,11 +403,10 @@ sub new {
or $options->{'force_sync_admin'}
)
) {
## Update admin_table
unless (defined $list->sync_include_admin()) {
$log->syslog('err', '')
unless ($options->{'just_try'});
}
# Update admin_table.
$list->sync_include('owner');
$list->sync_include('editor');
if (not @{$list->get_admins('owner') || []}
and $list->{'admin'}{'status'} ne 'error_config') {
$log->syslog('err', 'The list "%s" has got no owner defined',
......@@ -3919,7 +3914,7 @@ sub add_list_member {
# Delete from exclusion_table and force a sync_include if new_user was
# excluded
if ($self->insert_delete_exclusion($who, 'delete')) {
$self->sync_include();
$self->sync_include('member');
if ($self->is_list_member($who)) {
$self->{'add_outcome'}{'added_members'}++;
next;
......@@ -4779,29 +4774,46 @@ sub _load_include_admin_user_file {
#sub get_list_of_sources_id;
# -> No longer used.
#sub sync_include_ca;
# -> sync_include().
# -> sync_include('member').
#sub purge_ca;
# -> Never used.
sub sync_include {
$log->syslog('debug2', '(%s, %s)', @_);
my $self = shift;
my $role = shift;
$role ||= 'member'; # Compat.<=6.2.54
return 0 unless $self->has_include_data_sources;
return 0
unless $self->has_data_sources($role)
or $self->has_included_users($role);
my $spindle = Sympa::Spindle::ProcessRequest->new(
context => $self,
action => 'include',
role => 'member',
role => $role,
scenario_context => {skip => 1},
);
unless ($spindle and $spindle->spin) {
Sympa::send_notify_to_listmaster($self, 'sync_include_failed', {});
$log->syslog('err',
'Could not get users (%s) from an data source for list %s',
$role, $self);
if ($role eq 'member') {
Sympa::send_notify_to_listmaster($self,
'sync_include_failed', {});
} else {
Sympa::send_notify_to_listmaster($self,
'sync_include_admin_failed', {});
}
return undef;
}
# Get and save total of subscribers.
$self->_cache_publish_expiry('member');
$self->_cache_publish_expiry('last_sync');
$self->_cache_publish_expiry(
($role eq 'member') ? 'member' : 'admin_user');
$self->_cache_publish_expiry(
($role eq 'member') ? 'last_sync' : 'last_sync_admin_user');
return 1;
}
......@@ -4810,7 +4822,7 @@ sub sync_include {
# -> _update_inclusion_table() and/or _clean_inclusion_table() in
# Sympa::Request::Handler::include class.
# The function sync_include() is to be called by the task_manager.
# The function sync_include('member') is to be called by the task_manager.
# This one is to be called from anywhere else. This function deletes the
# scheduled sync_include task. If this deletion happened in sync_include(),
# it would disturb the normal task_manager.pl functionning.
......@@ -4826,7 +4838,8 @@ sub on_the_fly_sync_include {
if (not $options{'use_ttl'}
or $self->_cache_read_expiry('last_sync') < time - $pertinent_ttl) {
$log->syslog('notice', "Synchronizing list members...");
my $return_value = $self->sync_include();
my $return_value = $self->sync_include('member');
$log->syslog('notice', "...done");
if ($return_value) {
$self->remove_task('sync_include');
return 1;
......@@ -4837,34 +4850,8 @@ sub on_the_fly_sync_include {
return 0;
}
sub sync_include_admin {
$log->syslog('debug2', '(%s)', @_);
my $self = shift;
return 0
unless @{$self->{'admin'}{'owner_include'} || []}
or @{$self->{'admin'}{'editor_include'} || []};
my $spindle = Sympa::Spindle::ProcessRequest->new(
context => $self,
action => 'include',
role => [qw(owner editor)],
scenario_context => {skip => 1},
);
unless ($spindle and $spindle->spin) {
Sympa::send_notify_to_listmaster($self, 'sync_include_failed', {});
$log->syslog('err',
'Could not get uses from an include source for list %s', $self);
Sympa::send_notify_to_listmaster($self,
'sync_include_admin_failed', {});
return undef;
}
$self->_cache_publish_expiry('admin_user');
$self->_cache_publish_expiry('last_sync_admin_user');
return scalar @{$self->get_admins('owner')};
}
# DEPRECATED. Use sync_include('owner') & sync_include('editor').
#sub sync_include_admin;
#sub _load_list_admin_from_config;
# -> No longer used.
......@@ -6630,13 +6617,68 @@ sub remove_task {
# DDEPRECATED: Use Sympa::WWW::SharedDocument::create().
#sub create_shared;
## check if a list has include-type data sources
sub has_include_data_sources {
# Check if a list has data sources
# Old name: Sympa::List::has_include_data_sources(), without $role parameter.
sub has_data_sources {
my $self = shift;
my $role = shift;
my @parameters;
if (not $role or $role eq 'member') {
push @parameters, @sources_providing_listmembers, 'member_include';
}
if (not $role or $role eq 'owner') {
push @parameters, 'owner_include';
}
if (not $role or $role eq 'editor') {
push @parameters, 'editor_include';
}
foreach my $type (@sources_providing_listmembers, @more_data_sources) {
foreach my $type (@parameters) {
my $resource = $self->{'admin'}{$type} || [];
return 1 if ref $resource eq 'ARRAY' && @$resource;
return 1 if ref $resource eq 'ARRAY' and @$resource;
}
return 0;
}
sub has_included_users {
my $self = shift;
my $role = shift;
my $sdm = Sympa::DatabaseManager->instance;
my $sth;
if (not $role or $role eq 'member') {
unless (
$sdm and $sth = $sdm->do_prepared_query(
q{SELECT COUNT(*)
FROM subscriber_table
WHERE list_subscriber = ? AND robot_subscriber = ? AND
inclusion_subscriber IS NOT NULL},
$self->{'name'}, $self->{'domain'}
)
) {
return undef;
}
my ($count) = $sth->fetchrow_array;
return 1 if $count;
}
if (not $role or $role ne 'member') {
unless (
$sdm and $sth = $sdm->do_prepared_query(
q{SELECT COUNT(*)
FROM admin_table
WHERE list_admin = ? AND robot_admin = ? AND
inclusion_admin IS NOT NULL AND
(role_admin = ? OR role_admin = ?)},
$self->{'name'}, $self->{'domain'},
($role || 'owner'), ($role || 'editor')
)
) {
return undef;
}
my ($count) = $sth->fetchrow_array;
return 1 if $count;
}
return 0;
......@@ -6972,12 +7014,14 @@ sub _update_list_db {
# If inclusion settings do no longer exist, inclusion_table won't be
# sync'ed anymore. Rows left behind should be removed.
unless ($self->has_include_data_sources) {
$sdm and $sdm->do_prepared_query(
q{DELETE FROM inclusion_table
WHERE target_inclusion = ?},
$self->get_id
);
foreach my $role (qw(member owner editor)) {
unless ($self->has_data_sources($role)) {
$sdm and $sdm->do_prepared_query(
q{DELETE FROM inclusion_table
WHERE target_inclusion = ? AND role_inclusion = ?},
$self->get_id, $role
);
}
}
$sth = pop @sth_stack;
......
......@@ -271,11 +271,10 @@ sub _twist {
#FIXME: add_stat().
## Synchronize list members if required
if ($list->has_include_data_sources()) {
$log->syslog('notice', "Synchronizing list members...");
$list->sync_include();
}
# Synchronize list members if required
$log->syslog('notice', "Synchronizing list members...");
$list->sync_include('member');
$log->syslog('notice', "...done");
# config_changes
if (open my $fh, '>', "$list->{'dir'}/config_changes") {
......@@ -319,11 +318,10 @@ sub _twist {
}
}
## Synchronize list members if required
if ($list->has_include_data_sources()) {
$log->syslog('notice', "Synchronizing list members...");
$list->sync_include();
}
# Synchronize list members if required
$log->syslog('notice', "Synchronizing list members...");
$list->sync_include('member');
$log->syslog('notice', "...done");
return 1;
}
......
......@@ -258,10 +258,9 @@ sub _twist {
);
# Synchronize list members if required
if ($list->has_include_data_sources()) {
$log->syslog('notice', "Synchronizing list members...");
$list->sync_include();
}
$log->syslog('notice', "Synchronizing list members...");
$list->sync_include('member');
$log->syslog('notice', "...done");
$list->save_config($sender);
return 1;
......
......@@ -4,8 +4,8 @@
# Sympa - SYsteme de Multi-Postage Automatique
#
# Copyright 2019 The Sympa Community. See the AUTHORS.md file at
# the top-level directory of this distribution and at
# Copyright 2019, 2020 The Sympa Community. See the AUTHORS.md
# file at the top-level directory of this distribution and at
# <https://github.com/sympa-community/sympa.git>.
#
# This program is free software; you can redistribute it and/or modify
......@@ -127,10 +127,13 @@ sub _twist {
my $role = $request->{role};
die 'bug in logic. Ask developer'
unless grep { $role and $role eq $_ } qw(member owner editor);
unless $role and grep { $role eq $_ } qw(member owner editor);
return 0
unless $list->has_data_sources($role)
or $list->has_included_users($role);
my $dss = _get_data_sources($list, $role);
return 0 unless $dss and @$dss;
# Get an Exclusive lock.
my $lock_file = $list->{'dir'} . '/' . $role . '.include';
......
......@@ -4,8 +4,8 @@
# Sympa - SYsteme de Multi-Postage Automatique
#
# Copyright 2017, 2018 The Sympa Community. See the AUTHORS.md file at the
# top-level directory of this distribution and at
# Copyright 2017, 2018, 2020 The Sympa Community. See the AUTHORS.md
# file at the top-level directory of this distribution and at
# <https://github.com/sympa-community/sympa.git>.
#
# This program is free software; you can redistribute it and/or modify
......@@ -83,10 +83,11 @@ sub _twist {
$list->restore_users('owner');
$list->restore_users('editor');
} elsif ($mode eq 'install') {
# Since initial poermanent list users have been stored by create_list
# Since initial permanent list users have been stored by create_list
# or create_automatic_list, add transitional owners/editors from
# external data sources.
$list->sync_include_admin;
$list->sync_include('owner');
$list->sync_include('editor');
}
# Install new aliases.
......
......@@ -8,6 +8,9 @@
# Copyright (c) 1997, 1998, 1999, 2000, 2001, 2002, 2003, 2004, 2005,
# 2006, 2007, 2008, 2009, 2010, 2011 Comite Reseau des Universites
# Copyright (c) 2011, 2012, 2013, 2014, 2015, 2016, 2017 GIP RENATER
# Copyright 2020 The Sympa Community. See the AUTHORS.md
# # file at the top-level directory of this distribution and at
# # <https://github.com/sympa-community/sympa.git>.
#
# This program is free software; you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
......@@ -56,11 +59,9 @@ sub _twist {
$language->set_lang($list->{'admin'}{'lang'});
# Members list synchronization if include is in use.
if ($list->has_include_data_sources) {
unless (defined $list->on_the_fly_sync_include(use_ttl => 1)) {
$log->syslog('notice', 'Unable to synchronize list %s', $list);
#FIXME: Abort if synchronization failed.
}
unless (defined $list->on_the_fly_sync_include(use_ttl => 1)) {
$log->syslog('notice', 'Unable to synchronize list %s', $list);
#FIXME: Abort if synchronization failed.
}
my @users;
......
......@@ -282,11 +282,10 @@ sub _twist {
$list->{'admin'}{'status'} = $param->{'status'} || 'open';
$list->{'admin'}{'family_name'} = $family->{'name'};
## Synchronize list members if required
if ($list->has_include_data_sources()) {
$log->syslog('notice', "Synchronizing list members...");
$list->sync_include();
}
# Synchronize list members if required
$log->syslog('notice', "Synchronizing list members...");
$list->sync_include('member');
$log->syslog('notice', "...done");
# (Note: Following block corresponds to previous _set_status_changes()).
my $current_status = $list->{'admin'}{'status'} || 'open';
......