Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hide GTIN field for new installs make readonly for existing installs #2622

Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 50 additions & 0 deletions src/Admin/Input/Input.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
namespace Automattic\WooCommerce\GoogleListingsAndAds\Admin\Input;

use Automattic\WooCommerce\GoogleListingsAndAds\PluginHelper;
use Automattic\WooCommerce\GoogleListingsAndAds\Options\OptionsInterface;
use Automattic\WooCommerce\GoogleListingsAndAds\HelperTraits\Utilities;

defined( 'ABSPATH' ) || exit;

Expand All @@ -15,6 +17,7 @@
class Input extends Form implements InputInterface {

use PluginHelper;
use Utilities;

/**
* @var string
Expand Down Expand Up @@ -51,6 +54,16 @@ class Input extends Form implements InputInterface {
*/
protected $value;

/**
* @var bool
*/
protected $is_readonly = false;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really necessary since we don't do it in most other classes. But in theory we support PHP 7.4+ which means we can declare type for class properties. Just looks odd to set the type only in the doc-block and not actually restrict it in the class.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason I didn't do it is because the rest of the files (and other files like this one) have not the type set neither. So I bet for consistency in regards this topic.


/**
* @var bool
*/
protected $is_disabled = false;

/**
* Input constructor.
*
Expand All @@ -60,6 +73,9 @@ class Input extends Form implements InputInterface {
public function __construct( string $type, string $block_name ) {
$this->type = $type;
$this->block_name = $block_name;

$this->set_options_object( woogle_get_container()->get( OptionsInterface::class ) );
puntope marked this conversation as resolved.
Show resolved Hide resolved

parent::__construct();
}

Expand Down Expand Up @@ -142,6 +158,34 @@ public function set_value( $value ): InputInterface {
return $this;
}

/**
* @param bool $value
*
* @return InputInterface
*/
public function set_readonly( $value = false ) {
$this->is_readonly = $value;

return $this;
}
/**
* @param bool $value
*
* @return InputInterface
*/
public function set_disabled( $value = false ) {
$this->is_disabled = $value;

return $this;
}
puntope marked this conversation as resolved.
Show resolved Hide resolved

/**
* @return bool
*/
public function is_disabled(): bool {
return $this->is_disabled;
}

/**
* Return the data used for the input's view.
*
Expand All @@ -157,6 +201,12 @@ public function get_view_data(): array {
'desc_tip' => true,
];

if ( $this->is_readonly ) {
$view_data['custom_attributes'] = [
'readonly' => 'readonly',
];
}

return array_merge( parent::get_view_data(), $view_data );
}

Expand Down
9 changes: 6 additions & 3 deletions src/Admin/Product/Attributes/AttributesForm.php
Original file line number Diff line number Diff line change
Expand Up @@ -166,10 +166,13 @@ public function add_attribute( string $attribute_type, ?string $input_type = nul
$this->validate_interface( $input_type, InputInterface::class );

$attribute_input = self::init_input( new $input_type(), new $attribute_type() );
$this->add( $attribute_input );

$attribute_id = call_user_func( [ $attribute_type, 'get_id' ] );
$this->attribute_types[ $attribute_id ] = $attribute_type;
if ( ! $attribute_input->is_disabled() ) {
$this->add( $attribute_input );

$attribute_id = call_user_func( [ $attribute_type, 'get_id' ] );
$this->attribute_types[ $attribute_id ] = $attribute_type;
}

return $this;
}
Expand Down
30 changes: 30 additions & 0 deletions src/Admin/Product/Attributes/Input/GTINInput.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
namespace Automattic\WooCommerce\GoogleListingsAndAds\Admin\Product\Attributes\Input;

use Automattic\WooCommerce\GoogleListingsAndAds\Admin\Input\Text;
use Automattic\WooCommerce\GoogleListingsAndAds\Options\OptionsInterface;

defined( 'ABSPATH' ) || exit;

Expand All @@ -16,6 +17,13 @@
*/
class GTINInput extends Text {

/**
* Initial install version to disable the GTIN field for.
*
* @var string
*/
private $hidden_from = '2.8.5';
puntope marked this conversation as resolved.
Show resolved Hide resolved

/**
* GTINInput constructor.
*/
Expand All @@ -24,5 +32,27 @@ public function __construct() {

$this->set_label( __( 'Global Trade Item Number (GTIN)', 'google-listings-and-ads' ) );
$this->set_description( __( 'Global Trade Item Number (GTIN) for your item. These identifiers include UPC (in North America), EAN (in Europe), JAN (in Japan), and ISBN (for books)', 'google-listings-and-ads' ) );

$this->conditionally_restrict();
}

/**
* Controls the inputs visibility based on the WooCommerce version and the
* initial version of Google for WooCommerce at the time of installation.
*
* @since x.x.x
* @return void
*/
public function conditionally_restrict(): void {
$initial_version = $this->options->get( OptionsInterface::INSTALL_VERSION, false );

if ( version_compare( WC_VERSION, '9.2', '>=' ) ) {
if ( version_compare( $initial_version, $this->hidden_from, '>=' ) ) {
$this->set_disabled( true );
} else {
$this->set_readonly( true );
$this->set_description( __( 'The Global Trade Item Number (GTIN) for your item can now be entered on the "Inventory" tab', 'google-listings-and-ads' ) );
}
}
}
}
16 changes: 16 additions & 0 deletions src/HelperTraits/Utilities.php
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,22 @@ protected function gla_setup_for( $seconds ): bool {
return ( ( time() - $gla_completed_setup ) >= $seconds );
}

/**
* Test if the extension was installed after a specific date
*
* @param string $date Date to compare against the installation date
* @return bool
*/
protected function gla_installed_after( $date ): bool {
$gla_installed = $this->options->get( OptionsInterface::INSTALL_TIMESTAMP, false );

if ( false === $gla_installed ) {
return false;
}

return ( $gla_installed >= strtotime( $date ) );
}
puntope marked this conversation as resolved.
Show resolved Hide resolved

/**
* Is Jetpack connected?
*
Expand Down
3 changes: 3 additions & 0 deletions src/Internal/InstallTimestamp.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
use Automattic\WooCommerce\GoogleListingsAndAds\Options\OptionsAwareInterface;
use Automattic\WooCommerce\GoogleListingsAndAds\Options\OptionsAwareTrait;
use Automattic\WooCommerce\GoogleListingsAndAds\Options\OptionsInterface;
use Automattic\WooCommerce\GoogleListingsAndAds\PluginHelper;

defined( 'ABSPATH' ) || exit;

Expand All @@ -21,11 +22,13 @@ class InstallTimestamp implements Conditional, FirstInstallInterface, OptionsAwa

use AdminConditional;
use OptionsAwareTrait;
use PluginHelper;

/**
* Logic to run when the plugin is first installed.
*/
public function first_install(): void {
$this->options->add( OptionsInterface::INSTALL_TIMESTAMP, time() );
$this->options->add( OptionsInterface::INSTALL_VERSION, $this->get_version() );
}
}
2 changes: 2 additions & 0 deletions src/Options/OptionsInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ interface OptionsInterface {
public const GOOGLE_CONNECTED = 'google_connected';
public const GOOGLE_WPCOM_AUTH_NONCE = 'google_wpcom_auth_nonce';
public const INSTALL_TIMESTAMP = 'install_timestamp';
public const INSTALL_VERSION = 'install_version';
public const JETPACK_CONNECTED = 'jetpack_connected';
public const MC_SETUP_COMPLETED_AT = 'mc_setup_completed_at';
public const MERCHANT_ACCOUNT_STATE = 'merchant_account_state';
Expand Down Expand Up @@ -61,6 +62,7 @@ interface OptionsInterface {
self::FILE_VERSION => true,
self::GOOGLE_CONNECTED => true,
self::INSTALL_TIMESTAMP => true,
self::INSTALL_VERSION => true,
self::JETPACK_CONNECTED => true,
self::MC_SETUP_COMPLETED_AT => true,
self::MERCHANT_ACCOUNT_STATE => true,
Expand Down
Loading