Search package: move search dashboard to package ( the base ) (1/2) #21865

kangzj · 2021-11-24T03:45:42Z

Changes proposed in this Pull Request:

The PR is a combination of #21840, #21841 and #21842, which includes the dependencies duplicated from Jetpack and a new state store. The PR doesn't touch the Jetpack the plugin (except for a dependency update) and will serve as a base to migrate the search dashboard on.

As the individual PRs were all reviewed by the search team already ( and most the team is on AFK now ), so I'm sending this for crew review straightly.

#21840 (commit)

Copied dependencies from Jetpack the plugin with zero change as a base. tests and state are skipped as we'll add new.

#21841 (commit)

Added new state store based on @wordpress/data to replace the state store in Jetpack client.

#21842 (commit)

Removed unnecessary dependencies from copied components.
Added simple tests for components
Make form toggle component grid layout compatible

Jetpack product discussion

n/a

Does this pull request change what data or activity we track or use?

no

Testing instructions:

Run (cd projects/packages/search && pnpm test)
Run (cd projects/packages/search && composer phpunit)
Ensure all tests pass
Ensure no changes to Jetpack the plugin except for a dependency version bump

…e rest controller

github-actions · 2021-11-24T03:46:53Z

Thank you for your PR!

When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:

✅ Include a description of your PR changes.
⚠️ All commits were linted before commit.
✅ Add a "[Status]" label (In Progress, Needs Team Review, ...).
✅ Add testing instructions.
✅ Specify whether this PR includes any changes to data or privacy.
✅ Add changelog entries to affected projects

This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖

The e2e test report can be found here. Please note that it can take a few minutes after the e2e tests checks are complete for the report to be available.

Once your PR is ready for review, check one last time that all required checks (other than "Required review") appearing at the bottom of this PR are passing or skipped.
Then, add the "[Status] Needs Team review" label and ask someone from your team review the code.
Once you’ve done so, switch to the "[Status] Needs Review" label; someone from Jetpack Crew will then review this PR and merge it to be included in the next Jetpack release.

Jetpack plugin:

Next scheduled release: December 7, 2021.
Scheduled code freeze: November 30, 2021.

Backup plugin:

Next scheduled release: December 7, 2021.
Scheduled code freeze: November 29, 2021.

jeherve

I noted things below, but nothing that cannot be fixed in follow-up PRs so I'm approving so we can move on.

jeherve · 2021-11-26T17:27:18Z

projects/packages/search/src/dashboard/components/button/style.scss

+@import "../../scss/calypso-colors";
+@import "../../scss/color-functions";
+@import "../../scss/functions/rem";
+@import "../../scss/typography";


We'll probably want to move towards using the base-styles package instead.

will try to use the package in the following PR.

jeherve · 2021-11-26T17:30:01Z

projects/packages/search/src/dashboard/store/actions/jetpack-settings.js

+		return successNotice( __( 'Updated settings.' ) );
+	} catch ( e ) {
+		yield setJetpackSettings( oldSettings );
+		return errorNotice( __( 'Error Update settings…' ) );


Suggested change

return successNotice( __( 'Updated settings.' ) );

} catch ( e ) {

yield setJetpackSettings( oldSettings );

return errorNotice( __( 'Error Update settings…' ) );

return successNotice( __( 'Updated settings.', 'jetpack' ) );

} catch ( e ) {

yield setJetpackSettings( oldSettings );

return errorNotice( __( 'Error Update settings…', 'jetpack' ) );

jeherve · 2021-11-26T17:30:53Z

projects/packages/search/src/dashboard/store/resolvers.js

+			return setSearchPlanInfo( planInfo );
+		}
+	} catch ( e ) {
+		return errorNotice( __( 'Error fetching search plan…' ) );


Suggested change

return errorNotice( __( 'Error fetching search plan…' ) );

return errorNotice( __( 'Error fetching search plan…', 'jetpack' ) );

@jeherve oops, thanks for catching this, will address in the following PR. And will see how to add the check here.

jeherve · 2021-11-26T17:31:03Z

projects/packages/search/src/dashboard/store/resolvers.js

+			return setJetpackSettings( settings );
+		}
+	} catch ( e ) {
+		return errorNotice( __( 'Error fetching settings…' ) );


Suggested change

return errorNotice( __( 'Error fetching settings…' ) );

return errorNotice( __( 'Error fetching settings…', 'jetpack' ) );

@jeherve oops, thanks for catching this, will address in the following PR.

matticbot · 2021-11-26T19:25:04Z

Caution: This PR has changes that must be merged to WordPress.com
Hello kangzj! These changes need to be synced to WordPress.com - If you 're an a11n, please commandeer and confirm D70721-code works as expected before merging this PR. Once this PR is merged, please commit the changes to WP.com. Thank you!
This revision will be updated with each commit to this PR

github-actions · 2021-11-26T19:30:33Z

Great news! One last step: head over to your WordPress.com diff, D70721-code, and commit it.
Once you've done so, come back to this PR and add a comment with your changeset ID.

Thank you!

kangzj · 2021-11-29T01:04:20Z

🚢 r235709-wpcom

* master: (22 commits) Update storybook monorepo to v6.4.0 (#21897) Refresh the site's modules and settings after successful product activation (#21886) Plugin Install: Bump MC stat on success or fail. (#21893) Nav Unification: Use correct user capability for the Inbox menu item (#21882) Jetpack: Extend disconnection dialog component to handle multiple steps and accept more props (#21048) Disable CSSTidy property shorthand optimization in the editor or headstart (#21891) Connection UI: Update Composer instructions in README.md (#21890) Partner Compatibility: Adding a unique connection screen for customers who receive a coupon from a Jetpack partner (#21813) Search package: move search dashboard to package - the base (#21865) JITM: wrap CTA below text on small viewports (#21688) Licensing JS Package – fix icon positioning and width (#21878) JITM: new JS and CSS builder (#21874) Support site_intent for `/me/sites` endpoint (#21880) Don't render Critical CSS while generating CritCSS. (#21879) ConnectScreen: make button full width on small screens (#21683) Improve the copy of the license activation banner (#21876) Rename Webpack-built files back from `.min.js` and remove hashes (#21748) Search: Migrate more helper classes to package (#21751) Search: migrate/add search rest API (#21584) Add initial state to the connection package (#21864) ...

kangzj added 30 commits November 18, 2021 14:41

[not verified] search plan API

8a86790

[not verified] changelog

ad18172

[not verified] phpunit working

333887e

[not verified] placeholder for other APIs

309a66d

[not verified] added module settings api

4394f46

[not verified] added private search API

4cfcb5f

[not verified] moved forward wpcom/v2/search request to search packag…

aa39794

…e rest controller

[not verified] changelog

a62b490

[not verified] jetpack-connection package version

205cba2

[not verified] Register Search endpoints when WP REST API is initialized

a152c8f

[not verified] use make_proper_response

c5355fd

[not verified] API version v2=>2

9467874

[not verified] align module activate/deactivate process with Jetpack

954ed38

[not verified] added unit test for Module_Control

a81a93c

[not verified] comment

2db774f

[not verified] comment

440ad34

[not verified] change module_status to module_active

010d3b7

[not verified] fix typo

ac2ae08

[not verified] remove unused comment

2c125a1

[not verified] fixed logic error for toggling

d91ebd7

[not verified] add more unit tests for search rest controller

8989171

[not verified] add another invalid argument test

45ff681

[not verified] add more tests

cf0c7a1

[not verified] update lock file for Jetpack

b86701d

[not verified] set plan option

89a42ae

[not verified] added Plan class and more tests

0ec456a

hash conflict

d6c0a6b

[not verified] jetpack plugin content-hash

6d806a6

[not verified] added search test for admin user

f3983df

[not verified] cleaned up typo in comments

3442cc3

github-actions bot added [Plugin] Jetpack Issues about the Jetpack plugin. https://wordpress.org/plugins/jetpack/ RNA labels Nov 24, 2021

kangzj changed the title ~~Search package: move search dashboard to package - the base (1/2)~~ Search package: move search dashboard to package - the base Nov 24, 2021

kangzj added 5 commits November 24, 2021 22:08

merge from master

72b0111

update content hash

414d84a

[not verified] merge from master

89c3f76

[not verified] merge from service api

9254b02

update api version

f69ee92

kangzj added this to In progress in Jetpack Search - Standalone Nov 25, 2021

kangzj added 3 commits November 26, 2021 11:05

merge branch master

50d5f52

make jetpack-config happy..

b732edd

[not verified] revert formatting on Jetpack plugin

d3c724d

kangzj added [Status] Needs Review To request a review from Crew. Label will be renamed soon. and removed [Status] In Progress labels Nov 25, 2021

kangzj requested a review from a team November 25, 2021 23:19

jeherve added this to the jetpack/10.4 milestone Nov 26, 2021

jeherve approved these changes Nov 26, 2021

View reviewed changes

jeherve added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review To request a review from Crew. Label will be renamed soon. labels Nov 26, 2021

kangzj merged commit 8404f65 into master Nov 26, 2021

kangzj deleted the feature/move-search-dashboard-to-package branch November 26, 2021 19:29

github-actions bot removed the [Status] Ready to Merge Go ahead, you can push that green button! label Nov 26, 2021

kangzj changed the title ~~Search package: move search dashboard to package - the base~~ Search package: move search dashboard to package ( the base ) (1/2) Nov 28, 2021

kangzj mentioned this pull request Nov 29, 2021

Search: move search dashboard to package #21723

Closed

3 tasks

jsnmoon moved this from In progress to Done in Jetpack Search - Standalone Dec 1, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Search package: move search dashboard to package ( the base ) (1/2) #21865

Search package: move search dashboard to package ( the base ) (1/2) #21865

kangzj commented Nov 24, 2021 •

edited

Loading

github-actions bot commented Nov 24, 2021 •

edited

Loading

jeherve left a comment

jeherve Nov 26, 2021

kangzj Nov 28, 2021

jeherve Nov 26, 2021

jeherve Nov 26, 2021

kangzj Nov 26, 2021

jeherve Nov 26, 2021

kangzj Nov 26, 2021

matticbot commented Nov 26, 2021

github-actions bot commented Nov 26, 2021

kangzj commented Nov 29, 2021

	return errorNotice( __( 'Error fetching search plan…' ) );
	return errorNotice( __( 'Error fetching search plan…', 'jetpack' ) );

Search package: move search dashboard to package ( the base ) (1/2) #21865

Search package: move search dashboard to package ( the base ) (1/2) #21865

Conversation

kangzj commented Nov 24, 2021 • edited Loading

Changes proposed in this Pull Request:

Jetpack product discussion

Does this pull request change what data or activity we track or use?

Testing instructions:

github-actions bot commented Nov 24, 2021 • edited Loading

jeherve left a comment

Choose a reason for hiding this comment

jeherve Nov 26, 2021

Choose a reason for hiding this comment

kangzj Nov 28, 2021

Choose a reason for hiding this comment

jeherve Nov 26, 2021

Choose a reason for hiding this comment

jeherve Nov 26, 2021

Choose a reason for hiding this comment

kangzj Nov 26, 2021

Choose a reason for hiding this comment

jeherve Nov 26, 2021

Choose a reason for hiding this comment

kangzj Nov 26, 2021

Choose a reason for hiding this comment

matticbot commented Nov 26, 2021

github-actions bot commented Nov 26, 2021

kangzj commented Nov 29, 2021

kangzj commented Nov 24, 2021 •

edited

Loading

github-actions bot commented Nov 24, 2021 •

edited

Loading