Skip to content
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.

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

Wordads instant: Family safe please #7272

Merged
merged 10 commits into from
Aug 17, 2016
Merged

Wordads instant: Family safe please #7272

merged 10 commits into from
Aug 17, 2016

Conversation

artpi
Copy link
Contributor

@artpi artpi commented Aug 4, 2016

Fixes #723, goes with D2501-code

  • Introduces wordads status
  • blocks mature/spam/etc sites from requesting wordads instant activation

Visuals

zrzut ekranu 2016-08-08 o 18 29 22

zrzut ekranu 2016-08-11 o 11 34 03

zrzut ekranu 2016-08-11 o 11 34 19

zrzut ekranu 2016-08-11 o 11 34 57

Testing

Half-flow

  1. Go to http://calypso.localhost:3000/ads/settings/$site
  2. Paste in Redux dispatcher tool:
{
type: 'WORDADS_STATUS_REQUEST_SUCCESS',
siteId: $siteId,
status: {unsafe: false | other | spam | mature }
}

Fulll flow

Test live: https://calypso.live/?branch=update/wordads-flagged

CC @rralian @dbspringer @lamosty @egill

@artpi artpi added this to the Plans: Maintenance milestone Aug 4, 2016
@artpi artpi self-assigned this Aug 4, 2016
@artpi artpi force-pushed the update/wordads-flagged branch 2 times, most recently from b5c8dc2 to b7a2dfb Compare August 11, 2016 08:13
@artpi artpi added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. and removed [Status] In Progress labels Aug 11, 2016
* Get WordAds Status of a site.
*
* @param {int} siteId The site ID
* @returns {XMLHttpRequest} The XHR instance
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a promise object.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@gwwar
Copy link
Contributor

gwwar commented Aug 11, 2016

Running this sandboxed, I do see the notice properly for a private site. It does take a bit of time to populate, so maybe we could have a separate PR to add a placeholder or loading indicator, to let the user know that we're checking their eligibility status.

However I'm running into trouble when trying to approve newly created sites:
screen shot 2016-08-11 at 1 30 39 pm

Anyone else seeing this?

@artpi
Copy link
Contributor Author

artpi commented Aug 12, 2016

However I'm running into trouble when trying to approve newly created sites:

Thank you, there was an issue in D2501-code. Fixed now.

URL: { type: 'string' },
approved: { type: 'boolean' },
active: { type: 'boolean' },
unsafe: wordadsUnsafeValues,
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe this should be:

unsafe: { enum: wordadsUnsafeValues }

Copy link
Contributor

Choose a reason for hiding this comment

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

You can test if the schema works, by refreshing the page. You'll see a validation error, if the schema doesn't match:
screen shot 2016-08-12 at 12 44 34 pm

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, thanks!

@gwwar
Copy link
Contributor

gwwar commented Aug 12, 2016

👍 Verified that I can approve sites with the new patch. The Calypso PR should be good to go once the schema is fixed.

@artpi artpi 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. [Status] Ready to Merge labels Aug 15, 2016
@artpi
Copy link
Contributor Author

artpi commented Aug 15, 2016

Once patch is merged, I'll merge this

@dbspringer
Copy link
Member

Giving my 👍 on this too.

artpi added 6 commits August 17, 2016 21:28
Redux site stickers

Add missing bracket after rebase

Add tests to stickers reducer tree

Fix notice

Modify stickers to wordads status
@artpi artpi force-pushed the update/wordads-flagged branch from 060d2a4 to db6f819 Compare August 17, 2016 19:33
@artpi artpi merged commit 5d494ed into master Aug 17, 2016
@artpi artpi deleted the update/wordads-flagged branch August 17, 2016 20:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants