Refactor

[SteveJonesDev]

This PR refactors the plugin with improved coding standards and testing framework.

• Added: class structure
• Added: autoloader
• Added: linting scripts and workflows
• Added: testing framework
• Added: a check for uninstall that removes the upload_files capability
• Added: unit tests
• Added: distribution script

[pattonwebz]

If the constructor is empty can it just be removed?

[pattonwebz]

All 3 of these methods have the same code almost. 2 are exactly the same. A more DRY approach (and I think more readable) would be:

1. Rename the methods to one public function add_capability() and a second to public static function remove_capability(). Call remove capability on both uninstall and deactivate hooks.
2. Use one static method that can either add or remove the cap. Call that method from the others.

Code suggestion below was written in github UI without autocomplete and is not tested. Use it as example only because it probably has errors, typos or other quirks.

Suggested change

	public function init() {
	register_activation_hook( EDECU_PLUGIN_FILE, array( $this, 'activate' ) );
	register_deactivation_hook( EDECU_PLUGIN_FILE, array( $this, 'deactivate' ) );
	}
	/**
	* Handles tasks to perform upon plugin activation.
	*
	* Adds the 'upload_files' capability to the 'contributor' role.
	*/
	public function activate() {
	$contributor = get_role( 'contributor' );
	if ( null !== $contributor ) {
	$contributor->add_cap( 'upload_files' );
	}
	}
	/**
	* Handles tasks to perform upon plugin deactivation.
	*
	* Removes the 'upload_files' capability from the 'contributor' role.
	*/
	public function deactivate() {
	$contributor = get_role( 'contributor' );
	if ( null !== $contributor ) {
	$contributor->remove_cap( 'upload_files' );
	}
	}
	/**
	* Handles tasks to perform upon plugin uninstallation.
	*
	* Removes the 'upload_files' capability from the 'contributor' role.
	* Note: This method should be called statically and is intended for registration with register_uninstall_hook.
	*/
	public static function uninstall() {
	$contributor = get_role( 'contributor' );
	if ( null !== $contributor ) {
	$contributor->remove_cap( 'upload_files' );
	}
	}
	public function init() {
	register_activation_hook( EDECU_PLUGIN_FILE, array( $this, 'add_capability' ) );
	register_deactivation_hook( EDECU_PLUGIN_FILE, __CLASS__ . '::remove_capability' );
	}
	/**
	* Adds the 'upload_files' capability to the 'contributor' role.
	*
	* To be run on activation.
	*/
	public function add_capability() {
	self::add_or_remove_upload_files_capabiity_from_contributor_role( true );
	}
	/**
	* Removes the 'upload_files' capability from the 'contributor' role.
	*
	* To be run on deactivation and uninstall.
	*/
	public static function remove_capability() {
	self::add_or_remove_upload_files_capabiity_from_contributor_role();
	}
	/**
	* Either adds or removes the upload capability to the contributor role.
	*
	* @param bool $add true to add the cap, false to remove. Default is remove.
	*/
	public static function add_or_remove_upload_files_capabiity_from_contributor_role( $add = false ) {
	$contributor = get_role( 'contributor' );
	if ( null !== $contributor ) {
	if ( $add ) {
	$contributor->add_cap( 'upload_files' );
	} else {
	$contributor->remove_cap( 'upload_files' );
	}
	}
	}

[pattonwebz]

This isn't being called from anywhere except in the tests, did you mean to call this from inside an uninstall.php file?

[pattonwebz]

I think $this->plugin will always be NotNull since it's instantiated. You probably want to check that the 2 actions are registered after calling init.

[pattonwebz]

This would pass even if Plugin::uninstall() wasn't called in this test method. Could you call activate first so that it's added then the code here validates that uninstall removes it?

[github-actions]

Test on Playground
Test this pull request on the Playground