Family updates don't propagates owners/editors changes in the database

[salaun-urennes1]

Hi,

Here at Université de Rennes 1 we have an extensive of list families.
I noticed a significant change in changelog of version 6.2.33.b1 : "Owners and moderators are no longer stored in list config file: They are stored only in database". Therefore I performed some tests to evaluate the impact of such change for us. I noticed that list families updates now fail to propagate owners/editors changes in the database. That's probably what David Verdin meant with "

excluding them from the config file would require to create exceptions for these parameters in the Family code.

" in #49 .

Here are some details about the issue :
The mafamille family has the following configuration :

#/home/sympa/etc/listes.univ-rennes1.fr/families/mafamille/config.tt2
subject [% sujet %]

[% FOREACH o = owner -%]
owner
email [% o.email %]

[% END -%]

I add a list to that family :

/home/sympa/bin/sympa.pl --add_list mafamille --robot listes.univ-rennes1.fr --input_file /home/sympa/etc/listes.univ-rennes1.fr/families/mafamille/maliste.xml 
sympa [notice]  : l'opération a été effectuée

maliste.xml looks like :

  <list>
    <listname>maliste</listname>
    <sujet>This is my list</sujet>
    <owner multiple="1">
      <email>first.owner@univ-rennes1.fr</email>
    </owner>
  </list>

The list looks fine from WWSympa :

Now I reinstantiate that famly with updated maliste.xml file to add a list owner :

  <list>
    <listname>maliste</listname>
    <sujet>This is my list</sujet>
    <owner multiple="1">
      <email>first.owner@univ-rennes1.fr</email>
    </owner>
    <owner multiple="1">
      <email>second.owner@univ-rennes1.fr</email>
    </owner>
   </list>

I run sympa.pl to update that list :

/home/sympa/bin/sympa.pl --modify_list mafamille --robot listes.univ-rennes1.fr --input_file /home/sympa/etc/listes.univ-rennes1.fr/families/mafamille/maliste.xml 

************************************************************
Use of uninitialized value in substitution (s///) at /home/sympa/bin/Sympa/Family.pm line 415.
Use of uninitialized value in print at /home/sympa/bin/Sympa/Family.pm line 422.

************************************************************

The maliste list has been modified.

The list config has been update:

## /home/sympa/list_data/listes.univ-rennes1.fr/maliste/config
	latest_instantiation
	email listmaster@listes.univ-rennes1.fr
	date_epoch 1526571319
	
	subject This is my list
	
	serial 1
	
	update
	email listmaster@listes.univ-rennes1.fr
	date_epoch 1526571319
	
	creation
	email listmaster@listes.univ-rennes1.fr
	date_epoch 1526571319
	
	status open
	
	family_name mafamille
	
	owner
	email first.owner@univ-rennes1.fr
	
	owner
	email second.owner@univ-rennes1.fr

But the web interface does not take these changes into account; the list homepage still shows first.owner@univ-rennes1.fr only.

I did not check the database, but I guess second.owner@univ-rennes1.fr has not been loaded there...

That's a big problem for us. I understand why this change was introduced, but it has at least this side effect, not talking about other organizations that have home grown tools, similar to list families, to automate list creation. The change you introduced will break such mecanisms.

Is there a consensus about this change among the sympa dev/users community?

[ikedas]

Hi @salaun-urennes1,

Thanks for detailed report.

PR #275 says:

• When a list is created, initial owners and moderators are importd from dump file in list directory: owner and editor parameters in config file are no longer loaded.
  • However, when list is created using list creation template, owner and editor are split into dump files automatically: users need not update config.tt2.
  • And however, when administrator manually creates a list by creating list directory and config file, they also have to put dump files and have to run sympa.pl --restore_users.

This change conciders creating, pending, opening, closing (or purging) and restoring lists, but not updating list. It is a bug. I'll try to fix it.

[salaun-urennes1]

Hi Soji,
Thank you for your quick response.
I never noticed the owner.dump and editor.dump files before; it must be something new?

I had a try with sympa.pl --restore_users and it works fine.
However I would expect sympa.pl --modify_list to cope with owner/editor update more transparently.

[ikedas]

Hi,

I never noticed the owner.dump and editor.dump files before; it must be something new?

Yes, they are new things. We have to describe them at least in document before release of 6.2.34.

However I would expect sympa.pl --modify_list to cope with owner/editor update more transparently.

I agree. It has been done on --create_list and --add_list. Committer (me!) missed the case of --modify_list.

[ikedas]

Hi @salaun-urennes1,

Could you please check PR above? (Or use patch).

Thanks,

I prepared documentation page: sympa-community/sympa-community.github.io@9610fc1

[salaun-urennes1]

Hi Soji,

I had a try with the patch.
Now sympa.pl --modify_list updates list owners in the database.

Thank you.

[ikedas]

Thanks for confirming fix!

If you have the time, you'd be better to check bulk instantiation and family closure also (though I didn't touch these functions).

[salaun-urennes1]

During the Hackathon I started working on functional tests (that might be useful for continuous integration). This work should allow automatic testing of bulk instantiation and family closure.

I'll send more information in the sympa-dev mailing list later on...

[salaun-urennes1]

I reopen this bug because we found a new issue related to list families and the new management of list owners and editors.

As stated in the documentation a list belonging to a family can still be customized by list owner. The list of customized parameters is stored in the config_changes file.

We have mailing lists with such customizations, including customization of list owners and editors.
Since we upgraded to Sympa 6.2.36, our daily list instantiation process make us loose such list owners+editors customizations.

The Sympa logs show Sympa::Family::modify_list() taking care of config_changes:

Nov  9 13:58:46 vmperl-tsympa2 sympa[13430]: info Sympa::Family::modify_list() Customizing: Keeping values for parameter owner
Nov  9 13:58:46 vmperl-tsympa2 sympa[13430]: info Sympa::Family::modify_list() Customizing: Keeping values for parameter editor

But now that owner and editor list config parameters are no more used, these customizations are not pushed in the admin_table.

Remember that code you added in Sympa::Request::Handler::update_automatic_list :

sympa/src/lib/Sympa/Request/Handler/update_automatic_list.pm

Lines 160 to 163 in 5b8e3bd

	# Store permanent list users.
	#XXX$list->restore_users('member');
	$list->restore_users('owner');
	$list->restore_users('editor');

The same code should be ran in Sympa::Family::modify_list after that block:

sympa/src/lib/Sympa/Family.pm

Lines 409 to 415 in 5b8e3bd

	## set list customizing
	foreach my $p (keys %{$custom->{'allowed'}}) {
	$list->{'admin'}{$p} = $custom->{'allowed'}{$p};
	delete $list->{'admin'}{'defaults'}{$p};
	$log->syslog('info', 'Customizing: Keeping values for parameter %s',
	$p);
	}

...and also in Sympa::Family::_update_existing_list after that block:

sympa/src/lib/Sympa/Family.pm

Lines 1964 to 1969 in 5b8e3bd

	foreach my $p (keys %{$custom->{'allowed'}}) {
	$list->{'admin'}{$p} = $custom->{'allowed'}{$p};
	delete $list->{'admin'}{'defaults'}{$p};
	$log->syslog('info', 'Customizing: Keeping values for parameter %s',
	$p);
	}

[salaun-urennes1]

In the meanwhile our workaround is to run this code for each list after our daily family instantiation:

cat config| perl -n000e 'while (<STDIN>) {print if /^owner$/mg}' > owner.dump
cat config| perl -n000e 'while (<STDIN>) {print if /^editor$/mg}' > editor.dump
/usr/local/sympa/bin/sympa.pl --restore_users --list=mylist@domain --role=owner,editor

[ikedas]

Olivier, I wish you to review new (after 6.2.32) documentation about dump files. Is this current behavior reasonable?

Notes/questions:

• One thing is missing in description above: Format of config.tt2 in create_list_templates and family templates have not been changed — they still may have owner and editor paragraphs, and split into config file and dump files as soon as it is formatted. Does this behavior matter?

• Honestly I can’t understand what config_changes control file does (that’s why I’m hasitating to refactor Family.pm). Can you please explain how this file should be used?

[salaun-urennes1]

I had a look at https://sympa-community.github.io/manual/customize/basics-list-config.html#dump-files. I understand what is the new behavior. I suggest adding a note in that part of the documentation to let the reader understand what was the previous behavior of Sympa (owners and editors being represented as config file parameters) explaining why it has been changed.

I think it's quite confusing that owner and editor parameters may remain in existing list config files, even though these informations are no more used. I would have expected the upgrade process to get rid of these information, for the sake of clarity.

Regarding config_changes, let me explain what it is used for:
Once a list belonging to a family has been instantiated, the list owner can still customized parameters, as soon as edit_list.conf does not forbid it. The config_changes file keeps track of what parameters have been customized.

Here is a sample config_changes file:

param owner
param editor

When a list (belonging to a family) in updated (through sympa.pl --modify_list or sympa.pl --instantiate_family), config_changes prevents customizations from being lost.

But now that owner and editor list config parameters are no more used, the code needs to be adapted to cope with owners and editors customization. Note that the _get_customizing() subroutine is used to load the config_changes file.

Do these explanations help?

[ikedas]

I had a look at https://sympa-community.github.io/manual/customize/basics-list-config.html#dump-files. I understand what is the new behavior. I suggest adding a note in that part of the documentation to let the reader understand what was the previous behavior of Sympa (owners and editors being represented as config file parameters) explaining why it has been changed.

Or, we may make family instantiation behaving the same as previous behavior. Reading your explanation below, I think the recent behavior replacing owners/moderators everytime lists are modified/instatiated is not the same.

I think it's quite confusing that owner and editor parameters may remain in existing list config files, even though these informations are no more used. I would have expected the upgrade process to get rid of these information, for the sake of clarity.

I agree. I thought of compatibility to config.tt2 (not config) customized for earlier versions. However, it makes sense that removing onwer and editor is less confusing.

If no objection, I'll work for correction on two points above.

---

Regarding config_changes, let me explain what it is used for:
Once a list belonging to a family has been instantiated, the list owner can still customized parameters, as soon as edit_list.conf does not forbid it. The config_changes file keeps track of what parameters have been customized.

Here is a sample config_changes file:

param owner
param editor

When a list (belonging to a family) in updated (through sympa.pl --modify_list or sympa.pl --instantiate_family), config_changes prevents customizations from being lost.

But now that owner and editor list config parameters are no more used, the code needs to be adapted to cope with owners and editors customization. Note that the _get_customizing() subroutine is used to load the config_changes file.

Do these explanations help?

Thank you! I understood.

However, I feel config_changes is essentially unnecessary. Because:

• If a parameter has not been changed, it is obiviously compliant to family constraint (or default constraint). --- Conversely, violations found by checking all parameters will be the same as ones found by checking only chenged parameters.
  Limiting parameters using config_changes can make verification a bit faster, but its complexity will lose much.

• Additionally, I'm working for introducing new config schema handling (Sympa::List::Config etc.) which may apply family constraint whenever configuration is loaded and updated.
  Instantiation looks required because changes by users were not verified immediately.

If my plan above is not so eccentric, I'd like to submit a new issue.

Edit: I was complicated a bit. I'll post another comment afterward.

[salaun-urennes1]

I think the recent behavior replacing owners/moderators everytime lists are modified/instatiated is not the same.

Sorry, I don't understand this statement.
What do you mean by "replacing"?

I agree. I thought of compatibility to config.tt2 (not config) customized for earlier versions. However, it makes sense that removing onwer and editor is less confusing.
If no objection, I'll work for correction on two points above.

That would be great.
However, it would make sense that the upgrade process also keeps a copy of the previous config file (with owners and editors). It could be config.upgrade.<date>

Edit: I was complicated a bit. I'll post another comment afterward.

I must admit I did not fully understand what you meant indeed.

[ikedas]

I think the recent behavior replacing owners/moderators everytime lists are modified/instatiated is not the same.

Sorry, I don't understand this statement.
What do you mean by "replacing"?

Current documentation says:

When a family list is modified ..., dump files including new owners and moderators are created, and owners and moderators in backend database are replaced with content of dump files.

I understood that replacement should occur only when config_changes does not contain owner/editor entry.

I agree. I thought of compatibility to config.tt2 (not config) customized for earlier versions. However, it makes sense that removing onwer and editor is less confusing.
If no objection, I'll work for correction on two points above.

That would be great.
However, it would make sense that the upgrade process also keeps a copy of the previous config file (with owners and editors). It could be config.upgrade.<date>

Backups of past config files will be kept. Do you mean it not sufficient?

[ikedas]

Hi @salaun-urennes1,
I made fixes to restore checks by config_changes on owner and editor: See these changes (especially the latter commit).

• This won't change format of config.tt2.

If there seems no problem, I'll submit changes above as new PR.

[salaun-urennes1]

I understood that replacement should occur only when config_changes does not contain owner/editor entry.

That's right.

Backups of past config files will be kept. Do you mean it not sufficient?

The standard config backup if fine.

I made fixes to restore checks by config_changes on owner and editor: See these changes (especially the latter commit).

I will do my best to have a look at your patch today. If not, please submit a PR; I will test your PR on our staging server.

[salaun-urennes1]

I tried the code from your PR on our staging server.
When running either sympa.pl --instantiate_family or sympa.pl --modify_list the owners customized in one list are preserved; it looks fine.

Thank you

[ikedas]

Thanks for confirming. I'll merge it.