[PATCH luci] luci-mod-network: split config migration into 2 steps

[Hauke Mehrtens]

On 5/29/21 5:25 PM, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal at milecki.pl>
> 
> Problem with handling all migrations in 1 step is that uci.sections()
> doesn't include changes queued using uci.callAdd() and uci.callSet().
> That could result in unexpected behaviour and generating invalid
> configs.
> 
> For the sake of simplicity and reliability use 2 steps migration. The
> downside is that users may get prompted twice to migrate.
> 
> Reported-by: Hauke Mehrtens <hauke at hauke-m.de>
> Fixes: 74be304e541f ("treewide: use "device" option in UCI "interface" sections")
> Signed-off-by: Rafał Miłecki <rafal at milecki.pl>

Tested-by: Hauke Mehrtens <hauke at hauke-m.de>

I tested it in OpenWrt 21.02 doing the migration in one step by clicking 
continue 2 times and also interrupting it in between, both worked.

> ---
>   .../resources/view/network/interfaces.js      | 48 +++++++++++++------
>   1 file changed, 34 insertions(+), 14 deletions(-)
> 
> diff --git a/modules/luci-mod-network/htdocs/luci-static/resources/view/network/interfaces.js b/modules/luci-mod-network/htdocs/luci-static/resources/view/network/interfaces.js
> index 6f8fd614b1..343bc31cfd 100644
> --- a/modules/luci-mod-network/htdocs/luci-static/resources/view/network/interfaces.js
> +++ b/modules/luci-mod-network/htdocs/luci-static/resources/view/network/interfaces.js
> @@ -317,11 +317,10 @@ return view.extend({
>   		});
>   	},
>   
> -	handleMigration: function(ev) {
> -		var interfaces = this.interfaceBridgeWithIfnameSections();
> +	handleBridgeMigration: function(ev) {
>   		var tasks = [];
>   
> -		interfaces.forEach(function(ns) {
> +		this.interfaceBridgeWithIfnameSections().forEach(function(ns) {
>   			var device_name = 'br-' + ns['.name'];
>   
>   			tasks.push(uci.callAdd('network', 'device', null, {

This is currently an unnamed section should this be named after br-*?

> @@ -332,12 +331,32 @@ return view.extend({
>   
>   			tasks.push(uci.callSet('network', ns['.name'], {
>   				'type': '',
> -				'device': device_name
> +				'ifname': device_name

Should we directly assign this to "device" and set "ifname" to an empty 
string?

>   			}));
>   		});
>   
> +		return Promise.all(tasks)
> +			.then(L.bind(ui.changes.init, ui.changes))
> +			.then(L.bind(ui.changes.apply, ui.changes));
> +	},
> +
> +	renderBridgeMigration: function() {
> +		ui.showModal(_('Network bridge configuration migration'), [
> +			E('p', _('The existing network configuration needs to be changed for LuCI to function properly.')),
> +			E('p', _('Upon pressing "Continue", bridges configuration will be updated and the network will be restarted to apply the updated configuration.')),
> +			E('div', { 'class': 'right' },
> +				E('button', {
> +					'class': 'btn cbi-button-action important',
> +					'click': ui.createHandlerFn(this, 'handleBridgeMigration')
> +				}, _('Continue')))
> +		]);
> +	},
> +
> +	handleIfnameMigration: function(ev) {
> +		var tasks = [];
> +
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_0x93DD20630910B515.asc
Type: application/pgp-keys
Size: 9895 bytes
Desc: OpenPGP public key
URL: <http://lists.openwrt.org/pipermail/openwrt-devel/attachments/20210529/d97aff4d/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.openwrt.org/pipermail/openwrt-devel/attachments/20210529/d97aff4d/attachment.sig>