Migrate store/actions from editor package to fields package #65289

gigitux · 2024-09-12T15:25:19Z

What?

This PR continues the work started in PR #65261 by migrating the remaining actions, with the exception of DuplicateTemplatePart, which will be addressed in a separate PR.

Changes Introduced

To avoid code duplication and standardize the way actions are written in the codebase, this PR introduces two new functions in the mutation folder:

deletePostWithNotices
editPostWithNotices

Happy to revisit the naming of these functions!

These functions handle the updating of entities and manage success and error notices. The logic they encapsulate was previously duplicated across multiple actions, so unifying it into these functions should improve maintainability.

Currently, there are other entities in this package that update entities without using these two utils. I'm going to refactor them once this PR is merged.

Old description

This PR is a follow-up of #65261. With this PR, all actions have been migrated except for:

`deletePostAction`

The blocker for this action is the dispatch of removeTemplates action imported by the @wordpress/editor package:

gutenberg/packages/editor/src/dataviews/actions/delete-post.tsx

Lines 53 to 55 in 4356a42

    
           const { removeTemplates } = unlock( 
        
           	useDispatch( editorStore as StoreDescriptor ) 
        
           );

`duplicateTemplatePart`

The blocker for this action is CreateTemplatePartModalContents:

gutenberg/packages/editor/src/dataviews/actions/duplicate-template-part.tsx

Lines 53 to 54 in 4356a42

    
           <CreateTemplatePartModalContents 
        
           	blocks={ blocks }

This component is used in the @wordpress/editor package by another component.

`resetPost`

The blocker for this action is the dispatch of removeTemplates action imported by the @wordpress/editor package:

gutenberg/packages/editor/src/dataviews/actions/reset-post.tsx

Lines 44 to 46 in 4356a42

    
           const { revertTemplate } = unlock( 
        
           	useDispatch( editorStore as StoreDescriptor ) 
        
           );

I'm wondering how we should tackle this. Should we re-implement removeTemplates in @wordpress/fields? 🤔

cc @youknowriad @oandregal

Testing Instructions

Ensure that Custom Dataviews is enabled.

Visit the Site Editor.
Visit the patterns view.
Smoke test all the actions available.
Visit the templates view.
Smoke test all the actions available.
Visit the page view.
Smoke test all the actions available.

Testing Instructions for Keyboard

Screenshots or screencast

…n-dataviews

…n-dataviews-2

youknowriad · 2024-09-12T15:31:17Z

I'm wondering how we should tackle this. Should we re-implement removeTemplates in @wordpress/fields? 🤔

Yes, It can either be moved to the "component" itself or move to core-data. I might prefer in the component directly for now like all the other actions.

packages/editor/src/dataviews/actions/delete-post.tsx

gigitux · 2024-09-13T14:41:02Z

packages/editor/src/dataviews/actions/reset-post.tsx

This action has been renamed into reset-template.

gigitux · 2024-09-13T14:41:39Z

packages/editor/src/dataviews/fields/index.ts

These fields are already available in @wordpress/fields package.

gigitux · 2024-09-13T14:42:01Z

packages/fields/README.md

In a dedicated PR, I will improve documentation.

gigitux · 2024-09-16T09:14:51Z

cc @youknowriad @oandregal I'm marking this PR ready for review!

github-actions · 2024-09-16T09:15:05Z

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: gigitux <gigitux@git.wordpress.org>
Co-authored-by: louwie17 <louwie17@git.wordpress.org>
Co-authored-by: youknowriad <youknowriad@git.wordpress.org>
Co-authored-by: oandregal <oandregal@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

…n-dataviews-2

louwie17

Overall looks good @gigitux and it tested well. I did notice several places where you still had some @wordpress/* imports under the Internal Dependencies comment block. I am not sure why the linter didn't catch this, maybe that is missing in the fields package still?

I left a couple other thoughts, but those aren't blockers.

louwie17 · 2024-09-16T13:47:37Z

packages/fields/src/actions/template/reset-template.tsx

+} from '../utils';
+import type { CoreDataError, Template, TemplatePart } from '../../types';
+import { addQueryArgs } from '@wordpress/url';
+import apiFetch from '@wordpress/api-fetch';


These two should be moved up to WordPress dependencies

louwie17 · 2024-09-16T13:47:49Z

packages/fields/src/actions/template/delete-template.tsx

+ */
+import { isTemplateRemovable, getItemTitle } from '../utils';
+import type { Template, TemplatePart } from '../../types';
+import { decodeEntities } from '@wordpress/html-entities';


This should be moved to to WordPress dependencies

louwie17 · 2024-09-16T13:48:01Z

packages/fields/src/actions/pattern/delete-pattern.tsx

+ */
+import { getItemTitle } from '../utils';
+import type { Pattern } from '../../types';
+import { decodeEntities } from '@wordpress/html-entities';


This should be moved up to WordPress dependencies

louwie17 · 2024-09-16T13:59:15Z

packages/fields/src/actions/template/reset-template.tsx

+		const onConfirm = async () => {
+			try {
+				for ( const template of items ) {
+					await revertTemplate( template, {


Having to copy this from the core/edit-site data store makes me wonder if this is something that should be passed in as a config. A onResetTemplate function that is optional, but can be passed into the action?
Could of-course be a follow up.

Having to copy this from the core/edit-site data store makes me wonder if this is something that should be passed in as a config

From what I understood, it looks like all actions that manipulate WordPress entities should exist in the @wordpress/core-data package.

Given that this PR is already big, and we are still evaluating multiple ideas, I followed the @youknowriad suggestion's to move it into this package.

louwie17 · 2024-09-16T14:09:29Z

packages/fields/src/actions/pattern/delete-pattern.tsx

+	},
+	supportsBulk: true,
+	hideModalHeader: true,
+	RenderModal: ( { items, closeModal, onActionPerformed } ) => {


Could the RenderModal for this one and delete-template be shared?
I realize currently this doesn't have access to the above action, but maybe we can move this into a thin wrapper?
A future idea is to pass the action into the RenderModal https://github.com/WordPress/gutenberg/blob/trunk/packages/dataviews/src/components/dataviews-item-actions/index.tsx#L115

Could the RenderModal for this one and delete-template be shared?

Good point! I would keep them separate and if we will see an overlap with other entities try to create a shared modal.

louwie17 · 2024-09-16T14:09:54Z

packages/fields/src/mutation/index.ts

+ * Internal dependencies
+ */
+import type { CoreDataError, Post } from '../types';
+import { dispatch } from '@wordpress/data';


Should move this up.

…n-dataviews-2

gigitux · 2024-09-18T09:59:26Z

Overall looks good @gigitux and it tested well. I did notice several places where you still had some @wordpress/* imports under the Internal Dependencies comment block. I am not sure why the linter didn't catch this, maybe that is missing in the fields package still?

I left a couple other thoughts, but those aren't blockers.

Fixed with 485237f! It is a nice catch, btw! I noticed that other packages suffer the same issue. I'm going to do a quick investigation and open an issue!

EDIT: I opened the issue: #65440

packages/fields/src/actions/template/index.ts

youknowriad · 2024-09-18T11:51:06Z

packages/fields/src/module.d.ts

@@ -0,0 +1,2 @@
+declare module '@wordpress/editor';
+declare module '@wordpress/patterns';


What is this about?

Why didn't we need something like it before (when the actions were in editor).

Because we are using @ts-ignore to ignore the errors. Both solutions aren't great and given that we are already using @ts-ignore, let's remove this file - 0deeed3

youknowriad · 2024-09-18T13:10:33Z

packages/fields/src/actions/delete-post.tsx

+	},
+};
+
+export default deletePostAction;


I'm assuming there's no change in logic in the two actions that Github didn't caught as files being moved right?

No, I didn't change the logic! Thanks for the review! 🙇

oandregal · 2024-09-25T14:50:29Z

What's the status of this PR? It has an approval, can it be merged, do you need help with that @gigitux ?

oandregal · 2024-09-25T14:51:21Z

packages/fields/src/mutation/index.ts

@@ -0,0 +1,184 @@
+/**


Why these actions are not top-level like any other?

The idea is that these functions are utils and not “proper actions”

Why is the folder called mutation and not utils?

I created a dedicated folder to highlight that these utils “mutate” an entity. I don't have a strong preference, I'm happy to put this file at top level and rename it to utils.ts

Looking at the code:

editPostWithNotices is not used anywhere. Can we removed it?

deletePostWithNotices (and the NoticeSettings type) is just used by the delete-post action. Can we inline it in that file instead?

True! To not make this PR too big, I didn't "migrate" existent actions to these two utils, but deletePostWithNotices could be used:

permanently-delete-post:

gutenberg/packages/fields/src/actions/permanently-delete-post.tsx

Line 16 in 400a67d

const permanentlyDeletePost: Action< PostWithPermissions > = {

trash-post:

gutenberg/packages/fields/src/actions/restore-post.tsx

Line 16 in 400a67d

const restorePost: Action< PostWithPermissions > = {

Instead editPostWithNotices could be used with:

restore-post:

gutenberg/packages/fields/src/actions/restore-post.tsx

Line 16 in 400a67d

const restorePost: Action< PostWithPermissions > = {

reset-post:

gutenberg/packages/fields/src/actions/reset-post.tsx

Line 179 in 400a67d

const resetPostAction: Action< Template | TemplatePart > = {

gigitux · 2024-09-25T14:53:47Z

What's the status of this PR? It has an approval, can it be merged, do you need help with that @gigitux ?

I can't merge it! I was thinking that more feedback was necessary before the merge! On my side, would be great if you merge it, so I can unblock #65390

oandregal · 2024-09-25T15:00:25Z

I can't merge it! I was thinking that more feedback was necessary before the merge! On my side, would be great if you merge it, so I can unblock #65390

Oh, I see. I will merge then to unblock further work. We can still iterate on this all.

oandregal · 2024-09-25T15:04:06Z

btw, there's some automation to generate the changelog for each Gutenberg (plugin) release. One thing we do is to take the PR names to generate the changelog, for example: https://make.wordpress.org/core/2024/09/12/what-is-new-in-gutenberg-19-2-11-september/ The better names they have, the less work the release coordinator has to do :) I'd suggest would be avoiding formats like [ XXXX ] YYY in favor of a more descriptive name, when possible.

gigitux · 2024-09-25T15:28:24Z

btw, there's some automation to generate the changelog for each Gutenberg (plugin) release. One thing we do is to take the PR names to generate the changelog, for example: https://make.wordpress.org/core/2024/09/12/what-is-new-in-gutenberg-19-2-11-september/ The better names they have, the less work the release coordinator has to do :) I'd suggest would be avoiding formats like [ XXXX ] YYY in favor of a more descriptive name, when possible.

Thanks for sharing it! 🙇

gigitux added 11 commits September 12, 2024 09:33

Migrate store and actions from editor package to fields package

67ff043

fix documentation

a71e726

fix import

0141d8c

remove duplicate code

f21a974

Merge branch 'trunk' of github.com:WordPress/gutenberg into add/actio…

7063ee8

…n-dataviews

update package-lock

644dab1

refactor PR

3616964

fix mobile build

f7f1d4d

update package.json

ae0efeb

migrate actions to @wordpress/fields package

4356a42

Merge branch 'trunk' of github.com:WordPress/gutenberg into add/actio…

bf1462a

…n-dataviews-2

reduce change

63adf35

gigitux force-pushed the add/action-dataviews-2 branch from 4db6b4c to 63adf35 Compare September 13, 2024 13:41

gigitux added 4 commits September 13, 2024 15:47

register new action

c938a15

improve code

66479da

fix actionId

0bf760d

update package-lock.json

08bb03d

gigitux commented Sep 13, 2024

View reviewed changes

packages/editor/src/dataviews/actions/delete-post.tsx Outdated Show resolved Hide resolved

gigitux commented Sep 13, 2024

View reviewed changes

packages/editor/src/dataviews/fields/index.ts Outdated

Copy link

Contributor Author

gigitux Sep 13, 2024

Choose a reason for hiding this comment

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

These fields are already available in @wordpress/fields package.

gigitux commented Sep 13, 2024

View reviewed changes

packages/fields/README.md Outdated

Copy link

Contributor Author

gigitux Sep 13, 2024

Choose a reason for hiding this comment

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

In a dedicated PR, I will improve documentation.

gigitux added 4 commits September 13, 2024 16:42

remove comment

b599c42

rename function

337b3e5

rename functions

1016ef8

improve code

39d25d1

gigitux force-pushed the add/action-dataviews-2 branch from b34c830 to 39d25d1 Compare September 13, 2024 20:36

gigitux marked this pull request as ready for review September 16, 2024 09:14

Merge branch 'trunk' of github.com:WordPress/gutenberg into add/actio…

dc03f57

…n-dataviews-2

louwie17 reviewed Sep 16, 2024

View reviewed changes

Merge branch 'trunk' of github.com:WordPress/gutenberg into add/actio…

60e7a58

…n-dataviews-2

gigitux mentioned this pull request Sep 17, 2024

Move duplicateTemplatePart action to the @wordpress/fields package #65390

Open

gigitux added 2 commits September 18, 2024 11:15

restore delete post action

0316536

fix import order

485237f

gigitux mentioned this pull request Sep 18, 2024

[ESLint]: dependency-group rule doesn't work correctly #65440

Open

youknowriad reviewed Sep 18, 2024

View reviewed changes

packages/fields/src/actions/template/index.ts Outdated Show resolved Hide resolved

move to a flat structure

5a4fb59

youknowriad reviewed Sep 18, 2024

View reviewed changes

youknowriad added [Type] Code Quality Issues or PRs that relate to code quality [Feature] DataViews Work surrounding upgrading and evolving views in the site editor and beyond labels Sep 18, 2024

remove not necessary file

0deeed3

youknowriad approved these changes Sep 18, 2024

View reviewed changes

oandregal reviewed Sep 25, 2024

View reviewed changes

oandregal merged commit cb44f51 into WordPress:trunk Sep 25, 2024
64 checks passed

oandregal changed the title ~~[Fields] Migrate store and actions from editor package to fields package #2~~ Continue migrating store/actions from editor package to fields package Sep 25, 2024

github-actions bot added this to the Gutenberg 19.4 milestone Sep 25, 2024

oandregal changed the title ~~Continue migrating store/actions from editor package to fields package~~ Migrate store/actions from editor package to fields package Sep 25, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate store/actions from editor package to fields package #65289

Migrate store/actions from editor package to fields package #65289

gigitux commented Sep 12, 2024 •

edited

Loading

youknowriad commented Sep 12, 2024

gigitux Sep 13, 2024 •

edited

Loading

gigitux Sep 13, 2024

gigitux Sep 13, 2024

gigitux commented Sep 16, 2024

github-actions bot commented Sep 16, 2024 •

edited

Loading

louwie17 left a comment

louwie17 Sep 16, 2024

louwie17 Sep 16, 2024

louwie17 Sep 16, 2024

louwie17 Sep 16, 2024

gigitux Sep 16, 2024

louwie17 Sep 16, 2024

gigitux Sep 16, 2024

louwie17 Sep 16, 2024

gigitux commented Sep 18, 2024 •

edited

Loading

youknowriad Sep 18, 2024

youknowriad Sep 18, 2024

gigitux Sep 18, 2024 •

edited

Loading

youknowriad Sep 18, 2024

gigitux Sep 18, 2024 •

edited

Loading

oandregal commented Sep 25, 2024

oandregal Sep 25, 2024

gigitux Sep 25, 2024

oandregal Sep 25, 2024

gigitux Sep 25, 2024

oandregal Sep 25, 2024

gigitux Sep 25, 2024 •

edited

Loading

gigitux commented Sep 25, 2024

oandregal commented Sep 25, 2024 •

edited

Loading

oandregal commented Sep 25, 2024

gigitux commented Sep 25, 2024

	const { removeTemplates } = unlock(
	useDispatch( editorStore as StoreDescriptor )
	);

	const { revertTemplate } = unlock(
	useDispatch( editorStore as StoreDescriptor )
	);

		@@ -0,0 +1,2 @@
		declare module '@wordpress/editor';
		declare module '@wordpress/patterns';

Migrate store/actions from editor package to fields package #65289

Migrate store/actions from editor package to fields package #65289

Conversation

gigitux commented Sep 12, 2024 • edited Loading

What?

Old description

deletePostAction

duplicateTemplatePart

resetPost

Testing Instructions

Testing Instructions for Keyboard

Screenshots or screencast

youknowriad commented Sep 12, 2024

gigitux Sep 13, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gigitux commented Sep 16, 2024

github-actions bot commented Sep 16, 2024 • edited Loading

louwie17 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gigitux commented Sep 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gigitux Sep 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gigitux Sep 18, 2024 • edited Loading

Choose a reason for hiding this comment

oandregal commented Sep 25, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gigitux Sep 25, 2024 • edited Loading

Choose a reason for hiding this comment

gigitux commented Sep 25, 2024

oandregal commented Sep 25, 2024 • edited Loading

oandregal commented Sep 25, 2024

gigitux commented Sep 25, 2024

gigitux commented Sep 12, 2024 •

edited

Loading

`deletePostAction`

`duplicateTemplatePart`

`resetPost`

gigitux Sep 13, 2024 •

edited

Loading

github-actions bot commented Sep 16, 2024 •

edited

Loading

gigitux commented Sep 18, 2024 •

edited

Loading

gigitux Sep 18, 2024 •

edited

Loading

gigitux Sep 18, 2024 •

edited

Loading

gigitux Sep 25, 2024 •

edited

Loading

oandregal commented Sep 25, 2024 •

edited

Loading