Skip to content

Feature: #16 - Sanitise version number #24

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
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Feature: #16 - Sanitise version number #24

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
5a27a6b
feat: update meta type
HarryC05 Feb 3, 2025
5b9aa2f
feat: add new version input
HarryC05 Feb 3, 2025
a29854f
feat: update admin bar
HarryC05 Feb 3, 2025
33c9c7d
feat: create version_object meta
HarryC05 Feb 3, 2025
4304f37
feat: remove new meta, store version as string
HarryC05 Feb 3, 2025
efc1b1f
feat: remove is_pre_release meta
HarryC05 Feb 3, 2025
59c499f
chore: lint
HarryC05 Feb 3, 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
8 changes: 3 additions & 5 deletions inc/class-admin-bar.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,18 +29,16 @@ public function register_link( \WP_Admin_Bar $wp_admin_bar ): void {
$last_week = strtotime( '-1 week' );
$release_date = strtotime( $latest_release->post_date_gmt );

$is_pre_release = get_post_meta( $latest_release->ID, 'is_pre_release', true );

$version = get_post_meta( $latest_release->ID, 'version', true );
$base_url = admin_url( 'admin.php?page=release-notes' );
$version = get_post_meta( $latest_release->ID, 'version', true );
$base_url = admin_url( 'admin.php?page=release-notes' );

$wp_admin_bar->add_node( [
'id' => 'release-note-version',
'title' => sprintf( 'Version %s', $version ),
'href' => sprintf( '%s&release-id=%d', $base_url, $latest_release->ID ),
'parent' => 'top-secondary',
'meta' => [
'class' => $is_pre_release ? 'release-note is-pre-release' : 'release-note',
'class' => str_contains( $version, '-') ? 'release-note is-pre-release' : 'release-note',
Copy link
Member

@g-elwell g-elwell Feb 3, 2025

Choose a reason for hiding this comment

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

Although you're dropping the meta, I would recommend keeping a boolean variable in place, i.e: $is_pre_release = str_contains( $version, '-'); as it helps make the intent clearer.

],
] );

Expand Down
20 changes: 8 additions & 12 deletions inc/class-post-type.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,17 @@ public function register_meta(): void {
'release-note',
'version',
[
'show_in_rest' => true,
'show_in_rest' => [
'schema' => [
'type' => 'string',
'default' => '0.0.0',
],
],
'single' => true,
'type' => 'string',
]
],
);

register_post_meta(
'release-note',
'release_date',
Expand All @@ -38,16 +44,6 @@ public function register_meta(): void {
'type' => 'string',
]
);
register_post_meta(
'release-note',
'is_pre_release',
[
'show_in_rest' => true,
'single' => true,
'type' => 'boolean',
'default' => false,
]
);
}

/**
Expand Down
6 changes: 4 additions & 2 deletions inc/class-release-publish.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -374,12 +374,14 @@ public function test_scheduled_event( $id ) {

$content_arr = explode( "\n", $content );

$version = get_post_meta( $id, 'version', true );

$header_title = [
[
'type' => 'header',
'text' => [
'type' => 'plain_text',
'text' => get_post_meta( $id, 'is_pre_release', true ) ? __( 'New Pre-Release 🎉', 'release-notes' ) : __( 'New Release 🎉', 'release-notes' ),
'text' => str_contains( $version, '-' ) ? __( 'New Pre-Release 🎉', 'release-notes' ) : __( 'New Release 🎉', 'release-notes' ),
Copy link
Member

@g-elwell g-elwell Feb 3, 2025

Choose a reason for hiding this comment

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

Same as above, although this wasn't in place originally it may be an improvement to have a variable $is_pre_release defined here

],
],
];
Expand All @@ -392,7 +394,7 @@ public function test_scheduled_event( $id ) {
'text' => sprintf(
/* translators: %s: The version number. */
__( 'Version: %s', 'release-notes' ),
get_post_meta( $id, 'version', true )
$version,
),
],
],
Expand Down
2 changes: 0 additions & 2 deletions inc/class-rest-endpoints.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ public function new_release( \WP_REST_Request $req ) {
$title = $params['name'];
$published_at = $params['publishedAt'];
$tag = $params['tagName'];
$is_pre_release = $params['isPrerelease'];

if ( $is_draft ) {
return;
Expand Down Expand Up @@ -76,7 +75,6 @@ public function new_release( \WP_REST_Request $req ) {
'meta_input' => [
'release_date' => explode( 'T', gmdate( 'c', strtotime( $published_at ) ) )[0],
'version' => $tag,
'is_pre_release' => $is_pre_release,
],
]);
}
Expand Down
7 changes: 3 additions & 4 deletions inc/views/release-notes-item.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
<?php
$version = get_post_meta( get_the_ID(), 'version', true );
$date = get_post_meta( get_the_ID(), 'release_date', true );
$is_pre_release = get_post_meta( get_the_ID(), 'is_pre_release', true );
$version = get_post_meta( get_the_ID(), 'version', true );
$date = get_post_meta( get_the_ID(), 'release_date', true );
?>

<article class="release-note-single<?php $is_pre_release && print ' is-pre-release'; ?>">
<article class="release-note-single<?php str_contains( $version, '-') && print ' is-pre-release'; ?>">
Copy link
Member

@g-elwell g-elwell Feb 3, 2025

Choose a reason for hiding this comment

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

Same as above, it may be an beneficial to keep the variable $is_pre_release defined here

<header class="release-note-header">
<h1 class="release-note-title"><?php the_title(); ?></h1>
<div class="release-note-meta-wrapper">
Expand Down
4 changes: 2 additions & 2 deletions inc/views/release-notes-widget.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<?php
$version = get_post_meta( get_the_ID(), 'version', true );
$date = get_post_meta( get_the_ID(), 'release_date', true );
$version = get_post_meta( get_the_ID(), 'version', true );
$date = get_post_meta( get_the_ID(), 'release_date', true );
?>

<article class="release-note-widget">
Expand Down
4 changes: 4 additions & 0 deletions src/dashboard/styles/styles.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,3 +135,7 @@ li.release-note.is-pre-release > a {
background: #ca4a1f !important;
}
}

.components-base-control:has(div > .release-notes__prerelease) {
flex: none;
}
116 changes: 100 additions & 16 deletions src/editor/register-plugins/CustomBlockFields.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,36 +2,128 @@ const { PluginDocumentSettingPanel } = wp.editPost;
const {
PanelRow,
DatePicker,
ToggleControl,
__experimentalInputControl: InputControl,
SelectControl,
__experimentalNumberControl: NumberControl,
__experimentalText: Text,
__experimentalVStack: VStack,
__experimentalHStack: HStack,
} = wp.components;
const { useEntityProp } = wp.coreData;
const { useSelect } = wp.data;
const { __ } = wp.i18n;
const { useState, useEffect } = wp.element;
Copy link
Member

@g-elwell g-elwell Feb 3, 2025

Choose a reason for hiding this comment

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

There are a few steps I'd recommend going through to improve the organisation of this JS code, and reduce complexity/chance of bugs:

  1. Extract splitVersion into a standalone function, defined outside of the React component (perhaps in a utils folder) This will force the removal of closures and side effects, clarifying that the function takes in X and returns Y. This makes it easier to understand, maintain and test.
  2. Move existing logic into an opposite joinVersion function to transform a version string into an object, for the same reason. It encapsulates that logic and gets it out of your component
  3. Now the component is cleaner, it should be easier to see that some state/effects are unnecessary. const versionObject = splitVersion(version); will achieve the same result you have here, as we're just calculating the version object based on existing props/state - https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state
  4. (optional) consider grouping the multiple input fields for the version into a dedicated component, ie <VersionControl value={version} onChange={handleChange} /> which can accept and return a version string - handling the conversions internally.


/** Adds the date selector to the post settings */
function CustomBlockFields() {
const postType = useSelect((select) => select('core/editor').getCurrentPostType(), []);
if (postType !== 'release-note') return null;

const [meta, setMeta] = useEntityProp('postType', postType, 'meta');
const { version, is_pre_release: isPrerelease, release_date: releaseDate } = meta;
const { version, release_date: releaseDate } = meta;

const [versionObject, setVersionObject] = useState({});

/**
* Splits the version string into an object
*/
const splitVersion = () => {
const prerelease = version.split('-');
const versionArray = prerelease[0].split('.');
const prereleaseVersion = prerelease[1] ? prerelease[1].split('.')[1] : '';
const prereleaseType = prerelease[1] ? prerelease[1].split('.')[0] : '';

setVersionObject({
major: versionArray[0],
minor: versionArray[1],
patch: versionArray[2],
prerelease: prereleaseType,
prerelease_version: prereleaseVersion,
});
};

/**
* Updates the version object
*
* @param {integer || string} val value
* @param {string} key key
*/
const onVersionChange = (val, key) => {
const tempVersionObject = { ...versionObject };
tempVersionObject[key] = val;

let versionString = `${tempVersionObject.major}.${tempVersionObject.minor}.${tempVersionObject.patch}`;

if (tempVersionObject.prerelease) {
versionString += `-${tempVersionObject.prerelease}`;
if (tempVersionObject.prerelease_version) {
versionString += `.${tempVersionObject.prerelease_version}`;
}
}

setVersionObject(tempVersionObject);
setMeta({ ...meta, version: versionString });
};

const onDateChange = (val) => {
setMeta({ ...meta, release_date: val });
};

useEffect(() => {
splitVersion();
}, []);

return (
<>
<PluginDocumentSettingPanel initialOpen name="release-notes" title="Release Info">
<PanelRow>
<InputControl
value={version}
onChange={(val) => setMeta({ ...meta, version: val })}
label={__('Version Number')}
/>
<HStack>
<NumberControl
shiftStep={1}
min={0}
value={versionObject.major}
onChange={(val) => onVersionChange(val, 'major')}
spinControls="none"
/>
<Text variant="label">.</Text>
<NumberControl
shiftStep={1}
min={0}
value={versionObject.minor}
onChange={(val) => onVersionChange(val, 'minor')}
spinControls="none"
/>
<Text variant="label">.</Text>
<NumberControl
shiftStep={1}
min={0}
value={versionObject.patch}
onChange={(val) => onVersionChange(val, 'patch')}
spinControls="none"
/>
<SelectControl
className="release-notes__prerelease"
value={versionObject.prerelease}
options={[
{ label: 'None', value: '' },
{ label: '-Alpha', value: 'alpha' },
{ label: '-Beta', value: 'beta' },
{ label: '-RC', value: 'rc' },
]}
onChange={(val) => onVersionChange(val, 'prerelease')}
/>
{versionObject.prerelease && (
<>
<Text variant="label">.</Text>
<NumberControl
shiftStep={1}
min={0}
value={versionObject.prerelease_version}
onChange={(val) => onVersionChange(val, 'prerelease_version')}
spinControls="none"
/>
</>
)}
</HStack>
</PanelRow>
<br />
<PanelRow>
Expand All @@ -43,14 +135,6 @@ function CustomBlockFields() {
/>
</VStack>
</PanelRow>
<br />
<PanelRow>
<ToggleControl
label={__('Pre-Release Toggle')}
checked={isPrerelease}
onChange={(val) => setMeta({ ...meta, is_pre_release: val })}
/>
</PanelRow>
</PluginDocumentSettingPanel>
</>
);
Expand Down