Skip to content

Jetpack Forms: cleanup dashboard migration code #43825

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.

Sign up for GitHub

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

Open
CGastrell wants to merge 12 commits into
base: trunk
Choose a base branch
from
+32 −380
Open

Jetpack Forms: cleanup dashboard migration code #43825

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
0dc6936
update phan baseline
CGastrell Jun 9, 2025
84a5b97
remove TODOs and code from legaacy view requests
CGastrell Jun 6, 2025
4460246
changelog
CGastrell Jun 6, 2025
437727c
remove all now unused view switch methods
CGastrell Jun 6, 2025
d95d9a6
assume first 3 filters for the migration are now active, remove wrapp…
CGastrell Jun 6, 2025
09e9276
update docblock, add note on how to handle polldaddy interaction
CGastrell Jun 6, 2025
1d728b8
remove force param on get_forms_admin_url and append_tab_to_url as we…
CGastrell Jun 24, 2025
5eaea44
update phan baseline for forms
CGastrell Jun 24, 2025
d3a9f6a
update phan baseline
CGastrell Jul 4, 2025
b7d5dd6
fixes after rebase
CGastrell Sep 23, 2025
0bf2756
update tests
CGastrell Nov 20, 2025
9c7b5ac
update phan baselines
CGastrell Dec 8, 2025
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
Prev Previous commit
Next Next commit
assume first 3 filters for the migration are now active, remove wrapp… 
…ed conditional code
  • Loading branch information
@CGastrell
CGastrell committed Dec 8, 2025
commit d95d9a6e080ff31bf565733153409e609c2d1896
16 changes: 0 additions & 16 deletions projects/packages/forms/src/contact-form/class-contact-form-plugin.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1485,22 +1485,6 @@ public function unread_count() {
}
return;
}

if ( isset( $submenu['feedback'] ) && is_array( $submenu['feedback'] ) && ! empty( $submenu['feedback'] ) ) {
foreach ( $submenu['feedback'] as $index => $menu_item ) {
if ( 'edit.php?post_type=feedback' === $menu_item[2] ) {
$unread = self::get_unread_count();

if ( $unread > 0 ) {
$unread_count = current_user_can( 'publish_pages' ) ? " <span class='feedback-unread jp-feedback-unread-counter count-{$unread} awaiting-mod'><span class='feedback-unread-count'>" . number_format_i18n( $unread ) . '</span></span>' : '';

// phpcs:ignore WordPress.WP.GlobalVariablesOverride.Prohibited
$submenu['feedback'][ $index ][0] .= $unread_count;
}
break;
}
}
}
}

/**
Expand Down
23 changes: 7 additions & 16 deletions projects/packages/forms/src/dashboard/class-dashboard-view-switch.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,32 +43,25 @@ public function is_jetpack_forms_admin_page() {
* @deprecated 6.6.0 Use Dashboard::get_forms_admin_url() instead.
*
* @param string|null $tab Tab to open in the forms admin page.
* @param boolean $force_inbox Whether to force the inbox view URL.
*
* @return string
*/
public function get_forms_admin_url( $tab = null, $force_inbox = false ) {
public function get_forms_admin_url( $tab = null ) {
_deprecated_function( __METHOD__, 'jetpack-6.6.0', 'Dashboard::get_forms_admin_url' );
$is_classic = $this->get_preferred_view() === self::CLASSIC_VIEW;
$switch_is_available = $this->is_jetpack_forms_view_switch_available();
$base_url = get_admin_url() . 'admin.php?page=jetpack-forms-admin';

$base_url = $is_classic && $switch_is_available && ! $force_inbox
? get_admin_url() . 'edit.php?post_type=feedback'
: get_admin_url() . ( $this->is_jetpack_forms_admin_page_available() ? 'admin.php?page=jetpack-forms-admin' : 'admin.php?page=jetpack-forms' );

return $this->append_tab_to_url( $base_url, $tab, $is_classic && $switch_is_available && ! $force_inbox );
return self::append_tab_to_url( $base_url, $tab, false );
}

/**
* Appends the appropriate tab parameter to the URL based on the view type.
*
* @param string $url Base URL to append to.
* @param string $tab Tab to open.
* @param boolean $is_classic_view Whether we're using the classic view.
* @param string $url Base URL to append to.
* @param string $tab Tab to open.
*
* @return string
*/
private function append_tab_to_url( $url, $tab, $is_classic_view ) {
Copy link
Contributor

@edanzer edanzer Dec 8, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Any specific reason to convert this to a static function? No big deal. Just curious. We should be sure we catch any other places the function is used (if relevant).

Copy link
Contributor Author

@CGastrell CGastrell Dec 8, 2025

Choose a reason for hiding this comment

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

Yes, we don't want to be instancing the dashboard switch class when we don't need it, it's a simple mechanism. And previous changes remove the initial instantiation of the class, so there's no more a way to call it as a class method, only static. The same function exists on Dashboard class. The original idea was to delete the method (or the entire class), this is the safer approach.

private static function append_tab_to_url( $url, $tab ) {
if ( ! $tab ) {
return $url;
}
Expand All @@ -83,9 +76,7 @@ private function append_tab_to_url( $url, $tab, $is_classic_view ) {
return $url;
}

return $is_classic_view
? add_query_arg( 'post_status', $status_map[ $tab ], $url )
: $url . '#/responses?status=' . $status_map[ $tab ];
return $url . '#/responses?status=' . $status_map[ $tab ];
}

/**
Expand Down