Fixes and improvements for selecting WP version #1189

nightnei · 2025-04-10T14:40:44Z

Fixes STU-113

Followup to Add mu-plugin to prevent updates on selected WordPress versions #1120

Proposed Changes

Previously, users could select specific version for their WP site, but it worked incorrectly - it was still autoupdated to the latest. With this PR we give opportunity to select specific version and opportunity to keep always latest (with auto-updating it). Also This PR adds a few UI improvements and some refacrtoring:

Created a new shared component for selecting WP version. So it will be consistently used in different places, w/o duplication of code.
Previously, we went with approach to store wpVersion inside appdata-v1.json. It's an option, but there are a few disadvantages:
2.1. With wpVersion were have 2 source of truth what is definitely not good. Much more reliable and stable to retrieve wp version from site instance, as we did it before.
2.2 If user selected latest and we store it in appdata-v1.json, then to get specific version we always had to do soothing like wpVersions.find(iteam => item.version === 'latest'), what is not very nice.
2.3 Folowing to 2.2 point above - even if we get the specific version with item.value === 'latest', we can't be sure that it' exactly the version which is used on this specific wp instance, so this way we can bugs.
2.4. Storing isWpAutoUpdating: boolean makes code much cleaner, as you can see in this PR, since we don't need to add extra condition and we can easily distinguish the version and auto-updating status, since it's 2 different things.

Testing Instructions

I decided to record video, since there are a lot of testing instructions and video will simplify go through them.
Also, it will help you (the reviewer) to quickly understand how it works in all scenarios.

https://drive.google.com/file/d/1eXwUk_B8Gkd3-28cVIEj4Sg7YCV9t4Sq/view?usp=sharing

nightnei · 2025-04-10T14:45:12Z

src/components/wp-version-selector/add-wp-version-to-list.ts

+
+type Option = { label: string; value: string };
+
+export const addWpVersionToList = ( newOption: Option, options: Option[] ): Option[] => {


I rewrote this function from mutability to immutability.

Moved close to wpVersionsSelector, since it doesn';t make sense to keep it globally now, when we have only one pleace where we use it.

nightnei · 2025-04-10T14:46:05Z

src/components/wp-version-selector/index.tsx

@@ -0,0 +1,121 @@
+import { SelectControl, Icon } from '@wordpress/components';


Now we have separate component which doesn't store any context, so it's already used in 2 places and can be easily reused in the future, w/o extra conditions.
Also now we have consistency with rendering it for creating/updating website, so it works the same way, we don't need to copy-paste etc.

nightnei · 2025-04-10T14:48:03Z

src/modules/site-settings/edit-site-details.tsx

 	const { __ } = useI18n();
 	const { updateSite, selectedSite, stopServer, startServer } = useSiteDetails();
-	const [ isChangeWpError, setIsChangeWpError ] = useState( '' );
+	const [ errorUpdatingWpVersion, setErrorUpdatingWpVersion ] = useState< string | null >( null );


Just renamed, since it's not a predicate

nightnei · 2025-04-10T14:50:22Z

vendor/wp-now/src/wp-now.ts


-	// Only add the auto-updates disabling plugin for non-latest WordPress versions
-	if ( options?.wordPressVersion && options.wordPressVersion !== 'latest' ) {
+	if ( ! options.isWpAutoUpdating ) {


Also good example of benefit from isWpAutoUpdating - the condition is much more simple.

nightnei · 2025-04-10T14:56:01Z

src/site-server.ts


+// We use SiteDetails for storing it in appdata-v1.json, so this meta was introduced for extra data which is not stored locally
+type SiteServerMeta = {
+	wpVersion?: string;


@sejas This is only one point where I am not sure that it's the best approach. I am not very familiar with wpnow etc, so could you please take a look on my approach to say - is it good, or maybe it's better to go another way.

sejas

I tested multiple cases of existing and new sites, and everything worked correctly. I'll give it a second pass tomorrow.

First, I like that we have a common component. We could rename the label Keep Latest Stable to Latest or Autoupdate Latest. Sites with older versions are marked as "Keep Latest Stable," which I believe is correct, as they will eventually be auto-updated.

For now, the edge case I'm uncertain about are Beta/Nightly versions. Existing sites with those versions are marked as Keep Latest Stable when they are not stable at all.
We add the version to the options, which is great. I think we should also mark that version as selected.
Additionally, I think we should continue auto-updating those beta versions, but it's not a strong opinion.

sejas · 2025-04-14T16:17:55Z

src/components/site-form.tsx

-												</Tooltip>
-											</div>
+
+											<WPVersionSelector


Nice! A single component 💪

sejas · 2025-04-14T16:28:26Z

src/hooks/use-add-site.ts

 						};
 					}

-					if ( newSite.wpVersion !== wpVersion ) {


Everything seems to be working correctly, creating new sites and editing sites.
So removing the test should update site with WordPress version after creation makes sense.

wojtekn · 2025-04-15T06:35:24Z

Can we display the actual version, e.g., "6.7.2 (latest)" instead of "Keep Latest Stable"?

For now, the edge case I'm uncertain about are Beta/Nightly versions. Existing sites with those versions are marked as Keep Latest Stable when they are not stable at all.
We add the version to the options, which is great. I think we should also mark that version as selected.
Additionally, I think we should continue auto-updating those beta versions, but it's not a strong opinion.

I think those shouldn't be auto-updated, and the first one should be the only one that is getting auto-updated.

Regarding the edit form, it seems there is a bug there as the "Keep Latest Stable" is displayed for each site. I think the currently used version should be preselected.

sejas · 2025-04-15T08:28:04Z

All existing sites are auto-updated, so eventually if those sites are used they will get updated to the latest version. That's the logic behind the picker showing "Keep Latest Stable" and not the specific version.

These existing sites are an edge case and any logic we add to handle them should be something "temporal". @nightnei, Maybe the first time Studio starts we could check the WP version in all existing sites and add isWpAutoUpdating: false in all of them that doesn't have the 6.7.2 version.

Regarding the edit form, it seems there is a bug there as the "Keep Latest Stable" is displayed for each site. I think the currently used version should be preselected.

Cool!, let's keep them as "pinned" versions.

nightnei · 2025-04-15T08:48:53Z

Can we display the actual version, e.g., "6.7.2 (latest)" instead of "Keep Latest Stable"?

@wojtekn regarding this point - I found it confusing to have both "6.7.2" and "6.7.2 latest", more details I descibed in this thread -p1744195024724019-slack-C04GESRBWKW.

Also, I was talking with ChatGPT about it and even w/o providing my thoughts he said that it's not clear for users to understand the difference bethween these options, since in both cases 6.7.2 is indeed latest. So he recommended to flag that the first option keeps site wiuth latest WP version and in teh second it uses static version.

Take a look please on the thread.
WDYT?

nightnei · 2025-04-15T08:54:53Z

All existing sites are auto-updated, so eventually if those sites are used they will get updated to the latest version. That's the logic behind the picker showing "Keep Latest Stable" and not the specific version.

These existing sites are an edge case and any logic we add to handle them should be something "temporal".

@wojtekn @sejas I have teh same thoughts. Also, additionally, as @sejas mentioned before - we would have inconsistency with rendering this warning

nightnei · 2025-04-15T08:56:37Z

@nightnei, Maybe the first time Studio starts we could check the WP version in all existing sites and add isWpAutoUpdating: false in all of them that doesn't have the 6.7.2 version.

@sejas hm, it seems it's great idea to achive consistency 👍 I will think about it

sejas

I didn't test it yet. I left a few comments in the new code. Let me know what you think.

src/storage/user-data.ts

wojtekn

Feature-wise works well.

sejas

Thanks for the changes.
I confirm new created sites work as expected and latest will be autoupdated, while betas and stables have "isWpAutoUpdating": false,

Existing sites are identified with the correct version too.

sejas · 2025-04-21T10:52:31Z

src/components/wp-version-selector/index.tsx

+						text={ __( 'WordPress Core automatic updates will be disabled for this site.' ) }
+						placement="top"
+					>
+						<Icon icon={ warning } className="text-[#ae5c00]" size={ 16 } />


I don't think this color is required. I don't see any change in the UI after removing it.

Suggested change

<Icon icon={ warning } className="text-[#ae5c00]" size={ 16 } />

<Icon icon={ warning } size={ 16 } />

sejas · 2025-04-21T11:03:16Z

I noticed the performance metrics is failing in this PR. I think we need to fix that before merging it.

bcotrim and others added 15 commits April 5, 2025 10:59

add mu-plugin to prevent updates on selected versions

387cd83

prevent wp version updates

8d18fb6

fix unit tests

77c55aa

change tooltip placement

995a52a

mount mu-plugin internally

bd47da2

persist wordpress version in app data

fb4405a

new wp version select ui

7e1f1c3

new wp version select ui

01cb442

remove unnecessary filter

a044248

add selected site to the list

e0d46d8

fix edit site wp version groups

741b350

remove console log

e4e75d9

Fixes and improvements for selecting WP version

723f3e5

chore: small adjustments

7fc4e89

chore: comment

73fdf7f

nightnei commented Apr 10, 2025

View reviewed changes

nightnei requested review from a team, sejas and wojtekn April 10, 2025 14:56

test: fix

88f70be

sejas reviewed Apr 14, 2025

View reviewed changes

sejas mentioned this pull request Apr 14, 2025

Add mu-plugin to prevent updates on selected WordPress versions #1120

Closed

1 task

sejas reviewed Apr 15, 2025

View reviewed changes

src/storage/user-data.ts Outdated Show resolved Hide resolved

src/storage/user-data.ts Outdated Show resolved Hide resolved

src/storage/user-data.ts Outdated Show resolved Hide resolved

src/storage/user-data.ts Outdated Show resolved Hide resolved

nightnei force-pushed the add/preventUpdatesWpVersion branch from c334dcd to 88f70be Compare April 15, 2025 13:18

nightnei added 2 commits April 15, 2025 14:22

chore: Latest

5b45eb7

chore: copy

7b535cc

bcotrim force-pushed the add/prevent_updates_wp_versions branch from e4e75d9 to fad6c3d Compare April 15, 2025 15:14

nightnei changed the base branch from add/prevent_updates_wp_versions to trunk April 16, 2025 09:29

wojtekn approved these changes Apr 16, 2025

View reviewed changes

sejas approved these changes Apr 21, 2025

View reviewed changes

wojtekn assigned nightnei Apr 21, 2025

nightnei added 2 commits April 21, 2025 12:21

chore: nit

fcd57de

Merge branch 'trunk' into add/preventUpdatesWpVersion

6ace737

nightnei merged commit 452ff62 into trunk Apr 21, 2025
8 checks passed

nightnei deleted the add/preventUpdatesWpVersion branch April 21, 2025 12:05

bgrgicak pushed a commit that referenced this pull request Apr 23, 2025

Fixes and improvements for selecting WP version (#1189)

5ee6d91


		type Option = { label: string; value: string };

		export const addWpVersionToList = ( newOption: Option, options: Option[] ): Option[] => {

		@@ -0,0 +1,121 @@
		import { SelectControl, Icon } from '@wordpress/components';

	<Icon icon={ warning } className="text-[#ae5c00]" size={ 16 } />
	<Icon icon={ warning } size={ 16 } />

Fixes and improvements for selecting WP version #1189

Fixes and improvements for selecting WP version #1189

Uh oh!

Conversation

nightnei commented Apr 10, 2025

Proposed Changes

Testing Instructions

Uh oh!

nightnei Apr 10, 2025

Choose a reason for hiding this comment

Uh oh!

nightnei Apr 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nightnei Apr 10, 2025

Choose a reason for hiding this comment

Uh oh!

nightnei Apr 10, 2025

Choose a reason for hiding this comment

Uh oh!

nightnei Apr 10, 2025

Choose a reason for hiding this comment

Uh oh!

sejas left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sejas Apr 14, 2025

Choose a reason for hiding this comment

Uh oh!

sejas Apr 14, 2025

Choose a reason for hiding this comment

Uh oh!

wojtekn commented Apr 15, 2025

Uh oh!

sejas commented Apr 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nightnei commented Apr 15, 2025

Uh oh!

nightnei commented Apr 15, 2025

Uh oh!

nightnei commented Apr 15, 2025

Uh oh!

sejas left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

wojtekn left a comment

Choose a reason for hiding this comment

Uh oh!

sejas left a comment

Choose a reason for hiding this comment

Uh oh!

sejas Apr 21, 2025

Choose a reason for hiding this comment

Uh oh!

sejas commented Apr 21, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

nightnei Apr 10, 2025 •

edited

Loading

sejas left a comment •

edited

Loading

sejas commented Apr 15, 2025 •

edited

Loading