Improve input arguments for Restricted_Site_Access::add_ips function

[Sidsector9]

Is your enhancement related to a problem? Please describe.

The general behaviour of the plugin when adding IPs from the backend or CLI is:

1. IPs are unique
2. IPs can share the same label

But when using the method Restricted_Site_Access::add_ips( array( 'five' => '192.168.1.5', 'six' => '192.168.1.6') );, the labels are forced to be unique.

The CLI and the WP-admin settings allow you to add IPs without labels but the above function doesn't when few of the IPs have labels and others don't.

Describe the solution you'd like My suggestion is to deprecate this method in favour of a new improved method which accepts an array argument which is instead mapped from IP -> Label.

In the new function, we can add IPs without label in the following manner:

Restricted_Site_Access::add_ips(
	array(
		'8.8.8.8'      => 'Google',
		'192.168.0.1'  => '',
		'1.1.1.1'      => 'Cloudflare',
		'201.34.25.56' => '',
		'34.12.25.56'  => '',
	)
);

This will also help with further development.

[mikelking]

Some weirdness tied to this suggestion. The docs in the current main readme state that the add_ips() method has been available since 7.1.1. The really weird part is that I am running 7.2.0 on my sites and the method does not exist in the class.

https://github.com/10up/restricted-site-access

If it is not available in the current released version then the change @Sidsector9 is recommending is less disruptive.

As an aside, would it be possible to wrap this into some sort of filter action so as to eliminate the need to deeply couple anyone's code directly to this class and it's underlying methods?

Restricted_Site_Access::remove_ips( $address_array);

If I build a RSA helper plugin to add or set IPs my plugin must know about the inner workings of this plugin which seems kind of brittle to me. Sure I can add class_exists() checks and all that but calling a filter action hook is less WSODish.

[jeffpaul]

@Sidsector9 I see that you self-assigned on this issue. Are you intending to open a PR to resolve or should we un-assign you and look to pull someone else with availability?

[Sidsector9]

@mikelking apologies for the super late reply.

I agree with your comment. I opened this issue by reading the document and looking at the code in the develop branch, where the add_ips() method was present. 😅

Answering your question:

As an aside, would it be possible to wrap this into some sort of filter action so as to eliminate the need to deeply couple anyone's code directly to this class and it's underlying methods?

If we go ahead with the filter, the only area it will work is when the IPs are added through the RSA admin settings page. This plugin provides an API to add IPs programatically within contexts other than the admin page.

I see 2 options here:

1. Deprecate the method for a new one.
2. Modify the current method. (breaking change)

Do you have any suggestions other than these 2?

@jeffpaul this is open for grabs.