P2: Add pre-approved domains setting for hubs #57227

annemirasol · 2021-10-21T13:15:53Z

Changes proposed in this Pull Request

Adds a new setting to P2 hubs for controlling pre-approved domains
This is part of a new P2 feature that will allow users with specific email domains to join a P2 on their own (no need for invites).

Testing instructions

Only P2 hubs should have this setting available.
Validation should work as expected. Rejected domains should display why they are rejected.
Saving and loading should work as expected.

Screens

Related to 3887-gh-Automattic/p2

github-actions · 2021-10-21T13:16:13Z

Link to Calypso live: https://calypso.live?image=registry.a8c.com/calypso/app:build-20285
Link to Jetpack Cloud live: https://calypso.live?image=registry.a8c.com/calypso/app:build-20285&env=jetpack

matticbot · 2021-10-21T13:23:57Z

Here is how your PR affects size of JS and CSS bundles shipped to the user's browser:

App Entrypoints (~79 bytes added 📈 [gzipped])

name                   parsed_size           gzip_size
entry-main                  +197 B  (+0.0%)      +79 B  (+0.0%)
entry-login                 +126 B  (+0.0%)      +46 B  (+0.0%)
entry-gutenboarding         +126 B  (+0.0%)      +46 B  (+0.0%)
entry-domains-landing       +126 B  (+0.0%)      +46 B  (+0.0%)
entry-browsehappy           +126 B  (+0.1%)      +46 B  (+0.2%)

Common code that is always downloaded and parsed every time the app is loaded, no matter which route is used.

Sections (~4018 bytes added 📈 [gzipped])

name      parsed_size           gzip_size
settings     +15149 B  (+2.5%)    +4018 B  (+2.2%)

Sections contain code specific for a given set of routes. Is downloaded and parsed only when a particular route is navigated to.

Async-loaded Components (~164 bytes removed 📉 [gzipped])

name                                        parsed_size           gzip_size
async-load-calypso-post-editor-media-modal       -737 B  (-0.0%)     -164 B  (-0.0%)

React components that are loaded lazily, when a certain part of UI is displayed for the first time.

Legend

What is parsed and gzip size?

Parsed Size: Uncompressed size of the JS and CSS files. This much code needs to be parsed and stored in memory.
Gzip Size: Compressed size of the JS and CSS files. This much data needs to be downloaded over network.

Generated by performance advisor bot at iscalypsofastyet.com.

client/my-sites/site-settings/section-general.jsx

client/my-sites/site-settings/settings-p2/preapproved-domains.jsx

naxoc

This is looking good! I tested locally and it works great.

I added a couple of small non-blocking comments :)

client/my-sites/site-settings/settings-p2/preapproved-domains.jsx

lamosty

Great work, Anne!

Left a few comments.

config/wpcalypso.json

lamosty · 2021-11-08T22:14:51Z

client/my-sites/site-settings/section-general.jsx

@@ -1,15 +1,27 @@
+import config from '@automattic/calypso-config';


Nit: you could also import just isEnabled like import { isEnabled } from ....

client/my-sites/site-settings/settings-p2/preapproved-domains.jsx

lamosty · 2021-11-08T22:27:11Z

client/my-sites/site-settings/settings-p2/preapproved-domains.jsx

+								checked={ isToggledOn }
+								disabled={ isRequestingSettings || isSavingSettings }
+								onChange={ handleDomainsToggle }
+								label={ translate(


Not sure if the second param is missing here (the text domain).

This one does not have a text domain param.

client/my-sites/site-settings/section-general.jsx

client/my-sites/site-settings/settings-p2/preapproved-domains.jsx

lamosty · 2021-11-08T22:36:34Z

client/my-sites/site-settings/settings-p2/preapproved-domains.jsx

+import { getSelectedSiteId } from 'calypso/state/ui/selectors';
+import wrapSettingsForm from '../wrap-settings-form';
+
+const P2PreapprovedDomainsForm = ( props ) => {


Maybe subjective: do you think we could move most of the smaller helpers out of the component body? Would make scanning P2PreapprovedDomainsForm a bit easier. In the past, we used to have stateful, class based React components. Methods there were easy to spot or scan. Not sure about these const components, though. I always worked only with small ones.

I think I'm gonna keep them inside the component as many of them use stuff from state. For consistency, I'll keep them all together.

unDemian

Looking great! 🎩

a8ci18n · 2021-11-24T14:17:14Z

This Pull Request is now available for translation here: https://translate.wordpress.com/deliverables/6991483

Thank you @annemirasol for including a screenshot in the description! This is really helpful for our translators.

a8ci18n · 2021-12-02T10:28:34Z

Translation for this Pull Request has now been finished.

annemirasol force-pushed the add/p2-setting-preapproved-domain branch from 97bcac6 to 2663576 Compare October 22, 2021 08:23

lamosty reviewed Oct 26, 2021

View reviewed changes

client/my-sites/site-settings/section-general.jsx Outdated Show resolved Hide resolved

annemirasol force-pushed the add/p2-setting-preapproved-domain branch 2 times, most recently from 0960490 to 0aafb59 Compare November 2, 2021 11:22

annemirasol marked this pull request as ready for review November 2, 2021 12:04

annemirasol requested a review from a team November 2, 2021 12:04

matticbot added the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Nov 2, 2021

github-actions bot assigned annemirasol Nov 2, 2021

naxoc reviewed Nov 5, 2021

View reviewed changes

client/my-sites/site-settings/settings-p2/preapproved-domains.jsx Outdated Show resolved Hide resolved

annemirasol force-pushed the add/p2-setting-preapproved-domain branch from 743ddfb to fd8bd23 Compare November 6, 2021 08:17

naxoc reviewed Nov 8, 2021

View reviewed changes

client/my-sites/site-settings/settings-p2/preapproved-domains.jsx Outdated Show resolved Hide resolved

client/my-sites/site-settings/settings-p2/preapproved-domains.jsx Outdated Show resolved Hide resolved

lamosty reviewed Nov 8, 2021

View reviewed changes

annemirasol requested review from naxoc, lamosty and a team November 15, 2021 11:29

unDemian approved these changes Nov 23, 2021

View reviewed changes

unDemian added [Status] Ready to Merge and removed [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Nov 23, 2021

annemirasol added 10 commits November 24, 2021 21:18

Add 'Approved domains' setting box (UI only)

4f9e73b

Switch to TokenField and add token validation stubs

54cad5a

Add token (domain) validation

4b72981

Disable save button if at least one error

fcf7eda

Load stored setting

5b1c227

Refactor

d453aba

Remove unnecessary double bangs

f80e191

Do not submit form if invalid tokens

70f3c61

Add tracks for domains validation

0f812a1

Update /validate method to GET

e7b3b36

annemirasol added 19 commits November 24, 2021 21:18

Remove data-tip-target attribute

2d58c2d

Follow tracks event name convention

59a6989

Set feature flags

f52a460

Rename file and component

2fa5184

Convert to functional component

d2bcb68

Refactor: spread props in args

d91546c

Add fields to useEffect dependency

62c77a7

Remove need for connected component

08aaf49

Rename feature flag; turn on for staging and test envs

586bff6

Refactor: import config's isEnabled specifically

81f00af

Cleanup: mark external vs internal deps

01c5638

Cleanup: use optional chaining

8fbe922

Pass only siteId to component

7e7ba38

Remove flowRight

9612810

Update className

3a23c0d

Cleanup: remove classes-related intermediate var

dc090ad

Update more classNames

aa60c81

Return null instead of empty component

554d6a1

Bug fix: set isValidating flag only when about to send off request

4d42b87

annemirasol force-pushed the add/p2-setting-preapproved-domain branch from 9ed9712 to 4d42b87 Compare November 24, 2021 13:19

annemirasol merged commit 378c32d into trunk Nov 24, 2021

annemirasol deleted the add/p2-setting-preapproved-domain branch November 24, 2021 13:30

github-actions bot removed the [Status] Ready to Merge label Nov 24, 2021

nelsonec87 pushed a commit that referenced this pull request Dec 9, 2021

P2: Add pre-approved domains setting for hubs (#57227)

1f45f8d

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

P2: Add pre-approved domains setting for hubs #57227

P2: Add pre-approved domains setting for hubs #57227

annemirasol commented Oct 21, 2021 •

edited

Loading

github-actions bot commented Oct 21, 2021 •

edited by matticbot

Loading

matticbot commented Oct 21, 2021 •

edited

Loading

naxoc left a comment

lamosty left a comment

lamosty Nov 8, 2021

lamosty Nov 8, 2021

annemirasol Nov 15, 2021

lamosty Nov 8, 2021

annemirasol Nov 15, 2021

unDemian left a comment

a8ci18n commented Nov 24, 2021

a8ci18n commented Dec 2, 2021

		@@ -1,15 +1,27 @@
		import config from '@automattic/calypso-config';

P2: Add pre-approved domains setting for hubs #57227

P2: Add pre-approved domains setting for hubs #57227

Conversation

annemirasol commented Oct 21, 2021 • edited Loading

Changes proposed in this Pull Request

Testing instructions

Screens

github-actions bot commented Oct 21, 2021 • edited by matticbot Loading

matticbot commented Oct 21, 2021 • edited Loading

naxoc left a comment

Choose a reason for hiding this comment

lamosty left a comment

Choose a reason for hiding this comment

lamosty Nov 8, 2021

Choose a reason for hiding this comment

lamosty Nov 8, 2021

Choose a reason for hiding this comment

annemirasol Nov 15, 2021

Choose a reason for hiding this comment

lamosty Nov 8, 2021

Choose a reason for hiding this comment

annemirasol Nov 15, 2021

Choose a reason for hiding this comment

unDemian left a comment

Choose a reason for hiding this comment

a8ci18n commented Nov 24, 2021

a8ci18n commented Dec 2, 2021

annemirasol commented Oct 21, 2021 •

edited

Loading

github-actions bot commented Oct 21, 2021 •

edited by matticbot

Loading

matticbot commented Oct 21, 2021 •

edited

Loading