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

Allow PHP version to be changed from Site Settings #225

Merged
merged 9 commits into from
Jun 17, 2024
2 changes: 2 additions & 0 deletions jest-setup.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import '@testing-library/jest-dom';
// eslint-disable-next-line import/no-unresolved
import 'web-streams-polyfill/polyfill';
import nock from 'nock';

if ( typeof window !== 'undefined' ) {
Expand Down
16 changes: 16 additions & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@
"ts-loader": "^9.2.2",
"ts-node": "^10.0.0",
"typescript": "~5.3.2",
"web-streams-polyfill": "^4.0.0",
"webpack-dev-middleware": "5.3.4"
},
"dependencies": {
Expand Down
9 changes: 9 additions & 0 deletions src/components/content-tab-settings.tsx
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
import { Icon, file } from '@wordpress/icons';
import { useI18n } from '@wordpress/react-i18n';
import { PropsWithChildren } from 'react';
import { useGetPhpVersion } from '../hooks/use-get-php-version';
import { useGetWpVersion } from '../hooks/use-get-wp-version';
import { getIpcApi } from '../lib/get-ipc-api';
import { decodePassword } from '../lib/passwords';
import Button from './button';
import { CopyTextButton } from './copy-text-button';
import DeleteSite from './delete-site';
import EditPhpVersion from './edit-php-version';
import EditSite from './edit-site';

interface ContentTabSettingsProps {
Expand All @@ -30,6 +32,7 @@ export function ContentTabSettings( { selectedSite }: ContentTabSettingsProps )
// Empty strings account for legacy sites lacking a stored password.
const storedPassword = decodePassword( selectedSite.adminPassword ?? '' );
const password = storedPassword === '' ? 'password' : storedPassword;
const phpVersion = useGetPhpVersion( selectedSite );
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the hook useGetPhpVersion is not necessary because the PHP version is already coming from the site details in selectedSite.

	const phpVersion = selectedSite.phpVersion ?? DEFAULT_PHP_VERSION;

Not sure if this approach has been followed based on the other hook useGetWpVersion. If so, the WordPress version needs to be retrieved from the WordPress files as it's not referenced in the site details. That's the main reason for needing that hook.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'll add these changes in a separate PR to avoid blocking this one.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'll add these changes in a separate PR to avoid blocking this one.

#239

const wpVersion = useGetWpVersion( selectedSite );
return (
<div className="p-8">
Expand Down Expand Up @@ -67,6 +70,12 @@ export function ContentTabSettings( { selectedSite }: ContentTabSettingsProps )
</Button>
</SettingsRow>
<SettingsRow label={ __( 'WP Version' ) }>{ wpVersion }</SettingsRow>
<SettingsRow label={ __( 'PHP Version' ) }>
<div className="flex">
<span className="line-clamp-1 break-all">{ phpVersion }</span>
<EditPhpVersion />
</div>
</SettingsRow>

<tr>
<th colSpan={ 2 } className="pb-4 ltr:text-left rtl:text-right">
Expand Down
116 changes: 116 additions & 0 deletions src/components/edit-php-version.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
import { SupportedPHPVersions } from '@php-wasm/universal';
import { SelectControl } from '@wordpress/components';
import { useI18n } from '@wordpress/react-i18n';
import { FormEvent, useCallback, useEffect, useState } from 'react';
import { DEFAULT_PHP_VERSION } from '../../vendor/wp-now/src/constants';
import { useSiteDetails } from '../hooks/use-site-details';
import Button from './button';
import Modal from './modal';

export default function EditPhpVersion() {
const { __ } = useI18n();
const { updateSite, selectedSite, stopServer, startServer } = useSiteDetails();
const [ editPhpVersionError, setEditPhpVersionError ] = useState( '' );
const [ selectedPhpVersion, setSelectedPhpVersion ] = useState( DEFAULT_PHP_VERSION );
const [ needsToEditPhpVersion, setNeedsToEditPhpVersion ] = useState( false );
const [ isEditingSite, setIsEditingSite ] = useState( false );

useEffect( () => {
if ( selectedSite ) {
setSelectedPhpVersion( selectedSite.phpVersion );
}
}, [ selectedSite ] );

const resetLocalState = useCallback( () => {
setNeedsToEditPhpVersion( false );
setSelectedPhpVersion( '' );
setEditPhpVersionError( '' );
}, [] );

const onSiteEdit = useCallback(
async ( event: FormEvent ) => {
event.preventDefault();
if ( ! selectedSite ) {
return;
}
setIsEditingSite( true );
try {
const running = selectedSite.running;
await updateSite( {
...selectedSite,
phpVersion: selectedPhpVersion,
} );
if ( running ) {
await stopServer( selectedSite.id );
await startServer( selectedSite.id );
}
setNeedsToEditPhpVersion( false );
resetLocalState();
} catch ( e ) {
setEditPhpVersionError( ( e as Error )?.message );
}
setIsEditingSite( false );
},
[ updateSite, selectedSite, selectedPhpVersion, resetLocalState, startServer, stopServer ]
);

return (
<>
{ needsToEditPhpVersion && (
<Modal
size="medium"
title={ __( 'Edit PHP version' ) }
isDismissible
focusOnMount="firstContentElement"
onRequestClose={ resetLocalState }
>
<form onSubmit={ onSiteEdit }>
<label className="flex flex-col gap-1.5 leading-4">
<span className="font-semibold">{ __( 'PHP version' ) }</span>
<SelectControl
value={ selectedPhpVersion }
options={ SupportedPHPVersions.map( ( version ) => ( {
label: version,
value: version,
} ) ) }
onChange={ ( version ) => setSelectedPhpVersion( version ) }
/>
</label>
<div className="flex flex-row justify-end gap-x-5 mt-6">
<Button onClick={ resetLocalState } disabled={ isEditingSite } variant="tertiary">
{ __( 'Cancel' ) }
</Button>
<Button
type="submit"
variant="primary"
isBusy={ isEditingSite }
disabled={ Boolean(
isEditingSite ||
! selectedSite ||
selectedSite?.phpVersion === selectedPhpVersion ||
editPhpVersionError
) }
>
{ isEditingSite ? __( 'Restarting server…' ) : __( 'Save' ) }
</Button>
</div>
</form>
</Modal>
) }
<Button
disabled={ ! selectedSite }
className="!mx-4 shrink-0"
onClick={ () => {
if ( selectedSite ) {
setSelectedPhpVersion( selectedSite.phpVersion );
}
setNeedsToEditPhpVersion( true );
} }
label={ __( 'Edit PHP version' ) }
variant="link"
>
{ __( 'Edit' ) }
</Button>
</>
);
}
1 change: 1 addition & 0 deletions src/components/tests/content-tab-assistant.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ const runningSite = {
port: 8881,
path: '/path/to/site',
running: true,
phpVersion: '8.0',
id: 'site-id',
url: 'http://example.com',
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ const selectedSite: StartedSiteDetails = {
port: 8881,
path: '/path/to/site',
running: true,
phpVersion: '8.0',
id: 'site-id',
url: 'http://example.com',
};
Expand Down
2 changes: 2 additions & 0 deletions src/components/tests/content-tab-overview.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ const runningSite: StartedSiteDetails = {
name: 'Test Site',
port: 8881,
path: '/path/to/site',
phpVersion: '8.0',
running: true,
id: 'site-id',
url: 'http://example.com',
Expand All @@ -18,6 +19,7 @@ const notRunningSite: SiteDetails = {
name: 'Test Site',
port: 8881,
path: '/path/to/site',
phpVersion: '8.0',
running: false,
id: 'site-id',
};
Expand Down
4 changes: 4 additions & 0 deletions src/components/tests/content-tab-settings.test.tsx
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
// To run tests, execute `npm run test -- src/components/content-tab-settings.test.tsx` from the root directory
import { fireEvent, render, screen } from '@testing-library/react';
import { userEvent } from '@testing-library/user-event';
import { useGetPhpVersion } from '../../hooks/use-get-php-version';
import { useGetWpVersion } from '../../hooks/use-get-wp-version';
import { useOffline } from '../../hooks/use-offline';
import { useSiteDetails } from '../../hooks/use-site-details';
import { getIpcApi } from '../../lib/get-ipc-api';
import { ContentTabSettings } from '../content-tab-settings';

jest.mock( '../../hooks/use-get-wp-version' );
jest.mock( '../../hooks/use-get-php-version' );
jest.mock( '../../hooks/use-site-details' );
jest.mock( '../../lib/get-ipc-api' );

Expand All @@ -17,6 +19,7 @@ const selectedSite: SiteDetails = {
path: '/path/to/site',
adminPassword: btoa( 'test-password' ),
running: false,
phpVersion: '8.0',
id: 'site-id',
};

Expand All @@ -27,6 +30,7 @@ describe( 'ContentTabSettings', () => {
beforeEach( () => {
jest.clearAllMocks();
( useGetWpVersion as jest.Mock ).mockReturnValue( '7.7.7' );
( useGetPhpVersion as jest.Mock ).mockReturnValue( '8.0' );
( getIpcApi as jest.Mock ).mockReturnValue( {
copyText,
openLocalPath,
Expand Down
1 change: 1 addition & 0 deletions src/components/tests/content-tab-snapshots.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ const selectedSite = {
name: 'Test Site',
running: false as const,
path: '/test-site',
phpVersion: '8.0',
adminPassword: btoa( 'test-password' ),
};

Expand Down
1 change: 1 addition & 0 deletions src/hooks/tests/use-update-demo-site.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ describe( 'useUpdateDemoSite', () => {
const mockLocalSite: SiteDetails = {
name: 'Test Site',
running: false,
phpVersion: '8.0',
id: '54321',
path: '/path/to/site',
};
Expand Down
11 changes: 11 additions & 0 deletions src/hooks/use-get-php-version.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import { useEffect, useState } from 'react';
import { DEFAULT_PHP_VERSION } from '../../vendor/wp-now/src/constants';
import { getIpcApi } from '../lib/get-ipc-api';

export function useGetPhpVersion( site: SiteDetails ) {
const [ phpVersion, setPhpVersion ] = useState( DEFAULT_PHP_VERSION );
useEffect( () => {
getIpcApi().getPhpVersion( site.id ).then( setPhpVersion );
}, [ site.id, site.running, site.phpVersion ] );
return phpVersion;
}
11 changes: 10 additions & 1 deletion src/ipc-handlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import nodePath from 'path';
import * as Sentry from '@sentry/electron/main';
import archiver from 'archiver';
import { copySync } from 'fs-extra';
import { SQLITE_FILENAME } from '../vendor/wp-now/src/constants';
import { SQLITE_FILENAME, DEFAULT_PHP_VERSION } from '../vendor/wp-now/src/constants';
import { downloadSqliteIntegrationPlugin } from '../vendor/wp-now/src/download';
import { executeWPCli } from '../vendor/wp-now/src/execute-wp-cli';
import { LIMIT_ARCHIVE_SIZE } from './constants';
Expand Down Expand Up @@ -157,6 +157,7 @@ export async function createSite(
path,
adminPassword: createPassword(),
running: false,
phpVersion: DEFAULT_PHP_VERSION,
} as const;

const server = SiteServer.create( details );
Expand Down Expand Up @@ -464,6 +465,14 @@ export async function getAppGlobals( _event: IpcMainInvokeEvent ): Promise< AppG
};
}

export async function getPhpVersion( _event: IpcMainInvokeEvent, id: string ) {
const server = SiteServer.get( id );
if ( ! server ) {
return DEFAULT_PHP_VERSION;
}
return server.details.phpVersion || DEFAULT_PHP_VERSION;
}

export async function getWpVersion( _event: IpcMainInvokeEvent, id: string ) {
const server = SiteServer.get( id );
if ( ! server ) {
Expand Down
1 change: 1 addition & 0 deletions src/ipc-types.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ interface StoppedSiteDetails {
name: string;
path: string;
port?: number;
phpVersion: string;
adminPassword?: string;
themeDetails?: {
name: string;
Expand Down
1 change: 1 addition & 0 deletions src/preload.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ const api: IpcApi = {
copyText: ( text: string ) => ipcRenderer.invoke( 'copyText', text ),
getAppGlobals: () => ipcRenderer.invoke( 'getAppGlobals' ),
removeTemporalFile: ( path: string ) => ipcRenderer.invoke( 'removeTemporalFile', path ),
getPhpVersion: ( id: string ) => ipcRenderer.invoke( 'getPhpVersion', id ),
getWpVersion: ( id: string ) => ipcRenderer.invoke( 'getWpVersion', id ),
generateProposedSitePath: ( siteName: string ) =>
ipcRenderer.invoke( 'generateProposedSitePath', siteName ),
Expand Down
4 changes: 4 additions & 0 deletions src/site-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import nodePath from 'path';
import * as Sentry from '@sentry/electron/main';
import { getWpNowConfig } from '../vendor/wp-now/src';
import { WPNowMode } from '../vendor/wp-now/src/config';
import { DEFAULT_PHP_VERSION } from '../vendor/wp-now/src/constants';
import { getWordPressVersionPath } from '../vendor/wp-now/src/download';
import { pathExists, recursiveCopyDirectory, isEmptyDir } from './lib/fs-utils';
import { decodePassword } from './lib/passwords';
Expand Down Expand Up @@ -74,6 +75,7 @@ export class SiteServer {
port,
adminPassword: decodePassword( this.details.adminPassword ?? '' ),
siteTitle: this.details.name,
php: this.details.phpVersion ?? DEFAULT_PHP_VERSION,
} );
const absoluteUrl = `http://localhost:${ port }`;
options.absoluteUrl = absoluteUrl;
Expand All @@ -99,6 +101,7 @@ export class SiteServer {
...this.details,
url: this.server.url,
port: this.server.options.port,
phpVersion: this.server.options.phpVersion || DEFAULT_PHP_VERSION,
Copy link
Contributor

Choose a reason for hiding this comment

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

Nitpick: The phpVersion can be a supported value or undefined.

Suggested change
phpVersion: this.server.options.phpVersion || DEFAULT_PHP_VERSION,
phpVersion: this.server.options.phpVersion ?? DEFAULT_PHP_VERSION,

running: true,
themeDetails,
};
Expand All @@ -109,6 +112,7 @@ export class SiteServer {
...this.details,
name: site.name,
path: site.path,
phpVersion: site.phpVersion,
};
}

Expand Down
3 changes: 2 additions & 1 deletion src/storage/user-data.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ export async function saveUserData( data: UserData ): Promise< void > {
function toDiskFormat( { sites, ...rest }: UserData ): PersistedUserData {
return {
version: 1,
sites: sites.map( ( { id, path, adminPassword, port, name, themeDetails } ) => {
sites: sites.map( ( { id, path, adminPassword, port, phpVersion, name, themeDetails } ) => {
// No object spreading allowed. TypeScript's structural typing is too permissive and
// will permit us to persist properties that aren't in the type definition.
// Add each property explicitly instead.
Expand All @@ -85,6 +85,7 @@ function toDiskFormat( { sites, ...rest }: UserData ): PersistedUserData {
path,
adminPassword,
port,
phpVersion,
themeDetails: {
name: themeDetails?.name || '',
path: themeDetails?.path || '',
Expand Down
Loading
Loading