WordAds: Stop collecting WordAds owner information for US residents #58600

cphilleo · 2021-11-29T18:51:10Z

Changes proposed in this Pull Request

Remove the requirement to collect WordAds owner information for US residents.
Clarify requirements for a valid PayPal email

Testing instructions

Apply Phab Diff D70725-code — This removes the server side requirements
In Calypso go to WordAds settings
The only field under Payment Information should be the users PayPal email
Save and update settings.
Verify no errors, and settings saved correctly

github-actions · 2021-11-29T18:51:28Z

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

matticbot · 2021-11-29T18:51:53Z

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

Sections (~1460 bytes removed 📉 [gzipped])

name  parsed_size           gzip_size
earn      -6381 B  (-1.5%)    -1460 B  (-1.2%)

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

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.

a8ci18n · 2021-11-29T18:56:29Z

client/my-sites/earn/ads/form-settings.jsx

-						<FormSectionHeading>{ translate( 'Site Owner Information' ) }</FormSectionHeading>
-						{ this.siteOwnerOptions() }
-						{ this.state.us_checked ? this.taxOptions() : null }
+						<FormSectionHeading>{ translate( 'Payment Information' ) }</FormSectionHeading>


ℹ️ String reuse speeds up translation and improves consistency. The following string might make a good alternative and has already been translated 22 times:
translate( 'Stored Payment Information' ) ES Score: 6
See 1 additional suggestions in the PR translation status page

tyxla

This is a great cleanup, thank you ❤️

One thing that came up in testing for me though: when attempting to change the settings for an existing site, I got this error from the endpoint:

Looking at the endpoint code, it seems that this is caused by the absence of the who_owns field.

I've also added some suggestions for further cleanup in this area, if you're interested.

tyxla · 2021-11-30T11:52:44Z

client/my-sites/earn/ads/form-settings.jsx

 import FormSettingExplanation from 'calypso/components/forms/form-setting-explanation';
 import FormTextInput from 'calypso/components/forms/form-text-input';
 import FormTextarea from 'calypso/components/forms/form-textarea';
-import StateSelector from 'calypso/components/forms/us-state-selector';


Besides the use in devdocs, this is the last usage of the StateSelector component. Would you like to remove it, too, as part of this PR?

It might be a useful component for someone else to use? If it helps to reduce the build though, we can remove it. Would it be better to do it on a separate PR for visibility and to make it easier to revert later if someone wants the component?

I agree it could be useful, but there's also a chance it may be unused forever 😉

It's easy enough to dig out of history anyway.

A separate PR to clean it up would be ace! Let me know if you need any help or a review.

tyxla · 2021-11-30T11:55:53Z

client/my-sites/earn/ads/form-settings.jsx

-			addr1: '',
-			addr2: '',
-			city: '',
-			name: '',
 			optimized_ads: false,


There's also an unused additionalAdsOption method - would you like to clean it up further? If we remove it, it looks like we can get rid of the optimized_ads setting default value as well, since it looks like it's not used.

Thanks for noticing this. I have a separate follow-up ticket to investigate where we are using optimized_ads and whether we should remove it, or make the option available again. For now, we'll leave the default.

Thanks! Let me know if I can aid with reviews or anything there.

client/my-sites/earn/ads/form-settings.jsx

tyxla · 2021-11-30T12:06:10Z

client/my-sites/earn/ads/form-settings.jsx

-				<FormFieldset>
-					<FormLabel htmlFor="who_owns">{ translate( 'Who owns this site?' ) }</FormLabel>
-					<FormSelect
-						name="who_owns"


Should we keep this field? Seems like its absence is preventing me from saving the settings.

Oops, I forgot to link to the WPCOM patch that will remove this requirement: D70725-code

Once that is applied these fields will no longer be required server-side.

Works well with the patch! 🚀

tyxla

Code LGTM and this works well 🚢

a8ci18n · 2021-12-01T18:18:51Z

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

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

a8ci18n · 2021-12-09T10:28:51Z

Translation for this Pull Request has now been finished.

…58600) * Update UX to stop asking for owner data * Split text onto multiple lines

a8ci18n reviewed Nov 29, 2021

View reviewed changes

Update UX to stop asking for owner data

05a676e

cphilleo force-pushed the update/remove-wordads-owner-data branch from 3e5d67a to 05a676e Compare November 29, 2021 19:27

cphilleo changed the title ~~Stop collecting WordAds owner information for US residents~~ WordAds: Stop collecting WordAds owner information for US residents Nov 30, 2021

cphilleo self-assigned this Nov 30, 2021

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

cphilleo requested a review from tyxla November 30, 2021 01:35

tyxla requested changes Nov 30, 2021

View reviewed changes

Split text onto multiple lines

6d99e3a

cphilleo requested a review from tyxla November 30, 2021 17:49

tyxla approved these changes Dec 1, 2021

View reviewed changes

cphilleo merged commit a02ac36 into trunk Dec 1, 2021

cphilleo deleted the update/remove-wordads-owner-data branch December 1, 2021 17:46

github-actions bot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Dec 1, 2021

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

WordAds: Stop collecting WordAds owner information for US residents (#…

f3a5497

…58600) * Update UX to stop asking for owner data * Split text onto multiple lines

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WordAds: Stop collecting WordAds owner information for US residents #58600

WordAds: Stop collecting WordAds owner information for US residents #58600

cphilleo commented Nov 29, 2021 •

edited

github-actions bot commented Nov 29, 2021 •

edited by matticbot

matticbot commented Nov 29, 2021 •

edited

a8ci18n Nov 29, 2021

tyxla left a comment •

edited

tyxla Nov 30, 2021

cphilleo Nov 30, 2021

tyxla Dec 1, 2021

tyxla Nov 30, 2021

cphilleo Nov 30, 2021

tyxla Dec 1, 2021

tyxla Nov 30, 2021

cphilleo Nov 30, 2021

tyxla Dec 1, 2021

tyxla left a comment

a8ci18n commented Dec 1, 2021 •

edited

a8ci18n commented Dec 9, 2021

WordAds: Stop collecting WordAds owner information for US residents #58600

WordAds: Stop collecting WordAds owner information for US residents #58600

Conversation

cphilleo commented Nov 29, 2021 • edited

Changes proposed in this Pull Request

Testing instructions

github-actions bot commented Nov 29, 2021 • edited by matticbot

matticbot commented Nov 29, 2021 • edited

Choose a reason for hiding this comment

tyxla left a comment • edited

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

Choose a reason for hiding this comment

tyxla left a comment

Choose a reason for hiding this comment

a8ci18n commented Dec 1, 2021 • edited

a8ci18n commented Dec 9, 2021

cphilleo commented Nov 29, 2021 •

edited

github-actions bot commented Nov 29, 2021 •

edited by matticbot

matticbot commented Nov 29, 2021 •

edited

tyxla left a comment •

edited

a8ci18n commented Dec 1, 2021 •

edited