Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 4 additions & 3 deletions projects/packages/forms/.phan/baseline.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,30 +11,31 @@
// # Issue statistics:
// PhanTypeMismatchArgument : 50+ occurrences
// PhanPluginDuplicateConditionalNullCoalescing : 30+ occurrences
// PhanDeprecatedFunction : 8 occurrences
// PhanDeprecatedFunction : 9 occurrences
// PhanTypeMismatchReturnProbablyReal : 8 occurrences
// PhanTypeMismatchArgumentProbablyReal : 6 occurrences
// PhanUndeclaredFunction : 4 occurrences
// PhanPluginDuplicateAdjacentStatement : 3 occurrences
// PhanTypeConversionFromArray : 2 occurrences
// PhanTypeMismatchReturn : 2 occurrences
// PhanDeprecatedClass : 1 occurrence
// PhanPluginMixedKeyNoKey : 1 occurrence
// PhanPossiblyNullTypeMismatchProperty : 1 occurrence
// PhanTypeArraySuspiciousNullable : 1 occurrence
// PhanTypeMismatchReturnNullable : 1 occurrence
// PhanUnreferencedUseNormal : 1 occurrence

// Currently, file_suppressions and directory_suppressions are the only supported suppressions
'file_suppressions' => [
'src/abilities/class-forms-abilities.php' => ['PhanUndeclaredFunction'],
'src/contact-form/class-admin.php' => ['PhanPluginDuplicateConditionalNullCoalescing', 'PhanTypeArraySuspiciousNullable', 'PhanTypeMismatchArgument', 'PhanTypeMismatchArgumentProbablyReal', 'PhanTypeMismatchReturn'],
'src/contact-form/class-admin.php' => ['PhanPluginDuplicateConditionalNullCoalescing', 'PhanTypeMismatchArgument', 'PhanTypeMismatchArgumentProbablyReal', 'PhanTypeMismatchReturn'],
'src/contact-form/class-contact-form-field.php' => ['PhanPluginDuplicateConditionalNullCoalescing', 'PhanPossiblyNullTypeMismatchProperty', 'PhanTypeConversionFromArray', 'PhanTypeMismatchArgument', 'PhanTypeMismatchReturnProbablyReal'],
'src/contact-form/class-contact-form-plugin.php' => ['PhanPluginDuplicateAdjacentStatement', 'PhanPluginDuplicateConditionalNullCoalescing', 'PhanTypeMismatchArgument', 'PhanTypeMismatchArgumentProbablyReal', 'PhanTypeMismatchReturnProbablyReal'],
'src/contact-form/class-contact-form-shortcode.php' => ['PhanPluginDuplicateConditionalNullCoalescing', 'PhanTypeMismatchReturnProbablyReal'],
'src/contact-form/class-contact-form.php' => ['PhanPluginDuplicateConditionalNullCoalescing', 'PhanTypeMismatchArgument', 'PhanTypeMismatchReturnNullable', 'PhanTypeMismatchReturnProbablyReal'],
'src/dashboard/class-dashboard-view-switch.php' => ['PhanDeprecatedFunction', 'PhanUnreferencedUseNormal'],
'src/service/class-google-drive.php' => ['PhanTypeMismatchReturnProbablyReal'],
'tests/php/contact-form/Contact_Form_Plugin_Test.php' => ['PhanPluginMixedKeyNoKey'],
'tests/php/contact-form/Util_Test.php' => ['PhanDeprecatedFunction'],
'tests/php/dashboard/Dashboard_View_Switch_Test.php' => ['PhanDeprecatedClass', 'PhanDeprecatedFunction'],
],
// 'directory_suppressions' => ['src/directory_name' => ['PhanIssueName1', 'PhanIssueName2']] can be manually added if needed.
Expand Down
4 changes: 4 additions & 0 deletions projects/packages/forms/changelog/remove-forms-old-admin
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: major
Type: removed

Forms: Removes classic Admin initialization code
72 changes: 9 additions & 63 deletions projects/packages/forms/src/contact-form/class-admin.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
* Class Admin
*
* Singleton for Grunion admin area support.
*
* This class will be removed in a future version.
*/
class Admin {
/**
Expand All @@ -37,9 +39,12 @@ class Admin {
/**
* Instantiates this singleton class
*
* @deprecated $$next-version$$
*
* @return Admin The Admin class instance.
*/
public static function init() {
_deprecated_function( __METHOD__, 'package-$$next-version$$' );
static $instance = false;

if ( ! $instance ) {
Expand Down Expand Up @@ -81,9 +86,6 @@ public function __construct() {

add_action( 'admin_enqueue_scripts', array( $this, 'admin_enqueue_scripts' ) );
add_action( 'admin_footer-edit.php', array( $this, 'print_export_modal' ) );

add_action( 'wp_ajax_grunion_export_to_gdrive', array( $this, 'export_to_gdrive' ) );
add_action( 'wp_ajax_grunion_gdrive_connection', array( $this, 'test_gdrive_connection' ) );
}
Comment on lines 86 to 89
Copy link

Copilot AI Dec 10, 2025

Choose a reason for hiding this comment

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

The removal of the wp_ajax_grunion_gdrive_connection action registration breaks the Google Drive connection polling functionality. The JavaScript code in grunion-admin.js (line 219) still calls this AJAX action via startPollingConnection(), but the endpoint is no longer registered.

This action handler should be migrated to Contact_Form_Plugin class (similar to how export_to_gdrive was migrated), and the test_gdrive_connection method should either be migrated or deprecated with delegation.

Copilot uses AI. Check for mistakes.
Copy link
Contributor

Choose a reason for hiding this comment

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

@CGastrell - I'm curious about this too. Do/will we still need the logic and hook for test_gdrive_connection?


/**
Expand Down Expand Up @@ -151,70 +153,14 @@ public function print_export_modal() {
* Ajax handler for wp_ajax_grunion_export_to_gdrive.
* Exports data to Google Drive, based on POST data.
*
* @deprecated $$next-version$$
*
* @see Contact_Form_Plugin::get_feedback_entries_from_post
*/
public function export_to_gdrive() {
$post_data = wp_unslash( $_POST );
if (
! current_user_can( 'export' )
|| empty( sanitize_text_field( $post_data[ $this->export_nonce_field_gdrive ] ) )
|| ! wp_verify_nonce( sanitize_text_field( $post_data[ $this->export_nonce_field_gdrive ] ), 'feedback_export' )
) {
wp_send_json_error(
__( 'You aren\'t authorized to do that.', 'jetpack-forms' ),
403,
JSON_UNESCAPED_SLASHES
);

return;
}

$grunion = Contact_Form_Plugin::init();
$export_data = $grunion->get_feedback_entries_from_post();

$fields = is_array( $export_data ) ? array_keys( $export_data ) : array();
$row_count = ! is_array( $export_data ) || empty( $export_data ) ? 0 : count( reset( $export_data ) );

$sheet_data = array( $fields );

for ( $i = 0; $i < $row_count; $i++ ) {

$current_row = array();
_deprecated_function( __METHOD__, 'package-$$next-version$$', 'Automattic\Jetpack\Forms\ContactForm\Contact_Form_Plugin::init()->export_to_gdrive()' );

/**
* Put all the fields in `$current_row` array.
*/
foreach ( $fields as $single_field_name ) {
$current_row[] = $export_data[ $single_field_name ][ $i ];
}

$sheet_data[] = $current_row;
}

$user_id = (int) get_current_user_id();

if ( ! empty( $post_data['post'] ) && $post_data['post'] !== 'all' ) {
$spreadsheet_title = sprintf(
'%1$s - %2$s',
$this->get_export_filename( get_the_title( (int) $post_data['post'] ) ),
gmdate( 'Y-m-d H:i' )
);
} else {
$spreadsheet_title = sprintf( '%s - %s', $this->get_export_filename(), gmdate( 'Y-m-d H:i' ) );
}

$sheet = Google_Drive::create_sheet( $user_id, $spreadsheet_title, $sheet_data );

$grunion->record_tracks_event( 'forms_export_responses', array( 'format' => 'gsheets' ) );

wp_send_json(
array(
'success' => ! is_wp_error( $sheet ),
'data' => $sheet,
),
200,
JSON_UNESCAPED_SLASHES
);
Copy link
Contributor

Choose a reason for hiding this comment

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

Test results:

  • Sanity check that menu items and dashboard work as expected
  • Confirm CSV exports work
  • Confirm Google Drive exports work
  • Confirm that if I add code to call Admin::init() directly, I do get deprecation notices in debug.log
  • Confirm tests pass

Notes:

  • The PR description says "Fixes issue with CSV column array building to handle empty values properly by checking if array index exists before accessing", but I don't think the PR actually makes any changes for this?
  • The testing instructions say go to Dashboard → Feedback → Select a form... but we don't load a Feedback menu item or page anymore. If I try to load that page directly, I'm just redirected to our new forms dashboard. Also, when on our forms dashboard, you can't select a form when on the Jetpack > Forms page. So I wasn't quite sure what this was asking?

return Contact_Form_Plugin::init()->export_to_gdrive();
}

/**
Expand Down
103 changes: 103 additions & 0 deletions projects/packages/forms/src/contact-form/class-contact-form-plugin.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
use Automattic\Jetpack\Extensions\Contact_Form\Contact_Form_Block;
use Automattic\Jetpack\Forms\Jetpack_Forms;
use Automattic\Jetpack\Forms\Service\Form_Webhooks;
use Automattic\Jetpack\Forms\Service\Google_Drive;
use Automattic\Jetpack\Forms\Service\Hostinger_Reach_Integration;
use Automattic\Jetpack\Forms\Service\MailPoet_Integration;
use Automattic\Jetpack\Forms\Service\Post_To_Url;
Expand Down Expand Up @@ -97,6 +98,13 @@ class Contact_Form_Plugin {
'feedback_id' => '',
);

/**
* GDrive export nonce field name
*
* @var string The nonce field name for GDrive export.
*/
private $export_nonce_field_gdrive = 'feedback_export_nonce_gdrive';

Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if it makes any sense to move all the Google drive logic to the Google_Drive class that was created in August? That's in forms/src/services/.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess it would? But I'm also on the side of sticking to what we have tested and worked well. We can always follow up or find optimizations, specially in terms of code readability and organization. I just wanted to stay as simple as possible, after all, this is janitorial and not meant to take so much of us.

That said, feel free to push changes and add tests or test instructions. Or create a PR branching out of this one if that helps as a "follow up reminder".

/**
* Initializing function.
*/
Expand Down Expand Up @@ -221,6 +229,7 @@ protected function __construct() {
if ( is_admin() ) {
add_action( 'wp_ajax_feedback_export', array( $this, 'download_feedback_as_csv' ) );
add_action( 'wp_ajax_create_new_form', array( $this, 'create_new_form' ) );
add_action( 'wp_ajax_grunion_export_to_gdrive', array( $this, 'export_to_gdrive' ) );
}
add_action( 'admin_menu', array( $this, 'admin_menu' ) );
add_action( 'current_screen', array( $this, 'unread_count' ) );
Expand Down Expand Up @@ -3653,4 +3662,98 @@ public function redirect_edit_feedback_to_jetpack_forms() {
wp_safe_redirect( $redirect_url );
exit;
}

/**
* Validates the export to Google Drive request.
*
* @param array $post_data The POST data to validate.
* @return bool True if the request is valid, false otherwise.
*/
public function validate_export_to_gdrive_request( $post_data ) {
if ( ! current_user_can( 'export' ) ) {
return false;
}

if ( empty( $post_data[ $this->export_nonce_field_gdrive ] ) ) {
return false;
}

$nonce = sanitize_text_field( $post_data[ $this->export_nonce_field_gdrive ] );
if ( ! wp_verify_nonce( $nonce, 'feedback_export' ) ) {
return false;
}

return true;
}

/**
* Ajax handler for wp_ajax_grunion_export_to_gdrive.
* Exports data to Google Drive, based on POST data.
*
* @see Contact_Form_Plugin::get_feedback_entries_from_post
*/
public function export_to_gdrive() {
// phpcs:ignore WordPress.Security.NonceVerification.Missing -- verification is done on validate_export_to_gdrive_request function
$post_data = wp_unslash( $_POST );

if ( ! $this->validate_export_to_gdrive_request( $post_data ) ) {
wp_send_json_error(
__( 'You aren\'t authorized to do that.', 'jetpack-forms' ),
403,
JSON_UNESCAPED_SLASHES
);
return;
}

$grunion = self::init();
$export_data = $grunion->get_feedback_entries_from_post();

$fields = is_array( $export_data ) ? array_keys( $export_data ) : array();
$row_count = ! is_array( $export_data ) || empty( $export_data ) ? 0 : count( reset( $export_data ) );

$sheet_data = array( $fields );

for ( $i = 0; $i < $row_count; $i++ ) {

$current_row = array();

/**
* Put all the fields in `$current_row` array.
*/
foreach ( $fields as $single_field_name ) {
if ( isset( $export_data[ $single_field_name ][ $i ] ) ) {
$current_row[] = $this->esc_csv( $export_data[ $single_field_name ][ $i ] );
} else {
$current_row[] = '';
}
}

$sheet_data[] = $current_row;
}

$user_id = (int) get_current_user_id();

if ( ! empty( $post_data['post'] ) && $post_data['post'] !== 'all' ) {
$spreadsheet_title = sprintf(
'%1$s - %2$s',
Util::get_export_filename( get_the_title( (int) $post_data['post'] ) ),
gmdate( 'Y-m-d H:i' )
);
} else {
$spreadsheet_title = sprintf( '%s - %s', Util::get_export_filename(), gmdate( 'Y-m-d H:i' ) );
}

$sheet = Google_Drive::create_sheet( $user_id, $spreadsheet_title, $sheet_data );

$grunion->record_tracks_event( 'forms_export_responses', array( 'format' => 'gsheets' ) );

wp_send_json(
array(
'success' => ! is_wp_error( $sheet ),
'data' => $sheet,
),
is_wp_error( $sheet ) ? 500 : 200,
JSON_UNESCAPED_SLASHES
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ public static function add_hooks( $screen ) {
* Admin header.
*/
public static function admin_head() {
remove_action( 'media_buttons', array( Admin::init(), 'grunion_media_button' ), 999 );
add_action( 'media_buttons', array( __CLASS__, 'grunion_media_button' ), 999 );
}

Expand Down
3 changes: 0 additions & 3 deletions projects/packages/forms/src/contact-form/class-util.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,6 @@ class Util {
* Registers all relevant actions and filters for this class.
*/
public static function init() {
if ( is_admin() ) {
Admin::init();
}

add_filter( 'template_include', '\Automattic\Jetpack\Forms\ContactForm\Util::grunion_contact_form_set_block_template_attribute' );

Expand Down
Loading
Loading