From 585f16e83f38bdec094d7f514ae069b62ca717a1 Mon Sep 17 00:00:00 2001 From: Chad Whitacre Date: Mon, 12 Jun 2017 10:53:15 -0400 Subject: [PATCH] Tie into review process better --- emails/approved-1.0.spt | 2 +- emails/identity-viewed.spt | 2 +- emails/project-review.spt | 27 +++--- emails/rejected-1.0.spt | 2 +- emails/team-approved.spt | 2 +- emails/team-rejected.spt | 2 +- gratipay/application.py | 4 +- gratipay/models/participant/packages.py | 2 +- gratipay/models/team/__init__.py | 5 -- ...view_repo.py => project_review_process.py} | 44 ++++++++-- tests/py/test_email.py | 2 +- tests/py/test_project_review_process.py | 86 +++++++++++++++++++ tests/py/test_project_review_repo.py | 58 ------------- tests/py/test_teams.py | 2 +- www/teams/create.json.spt | 18 +--- 15 files changed, 150 insertions(+), 108 deletions(-) rename gratipay/{project_review_repo.py => project_review_process.py} (58%) create mode 100644 tests/py/test_project_review_process.py delete mode 100644 tests/py/test_project_review_repo.py diff --git a/emails/approved-1.0.spt b/emails/approved-1.0.spt index 596a628495..7e3b897129 100644 --- a/emails/approved-1.0.spt +++ b/emails/approved-1.0.spt @@ -1,4 +1,4 @@ -{{ _("1.0 Payout: Approved!") }} +{{ _("1.0 payout approved!") }} [---] text/html {{ _("We've approved your application for a payout of your Gratipay 1.0 balance. Watch for that during our next weekly payment cycle.") }}
diff --git a/emails/identity-viewed.spt b/emails/identity-viewed.spt index 43dae3dae7..c83cac7d0a 100644 --- a/emails/identity-viewed.spt +++ b/emails/identity-viewed.spt @@ -1,4 +1,4 @@ -{{ _("Identity Viewed") }} +{{ _("Identity viewed") }} [---] text/html {{ _( "This is a transactional email to let you know that {a_viewer}{viewer}{_a} viewed your identity information for {a_country}{country_name}{_a} on Gratipay." , viewer=viewer diff --git a/emails/project-review.spt b/emails/project-review.spt index 8424918b74..c298a24346 100644 --- a/emails/project-review.spt +++ b/emails/project-review.spt @@ -1,20 +1,19 @@ {{ _("We have your application!") }} [---] text/html -{{ _("Thanks for your project application for {0}! Your public project page is: " - "{1} " - "and your public review ticket is: " - "{2}. " - "You can watch and participate in our review process there. We will send a notification to {3} when we finish our review. " - "Thanks for applying!", - team_name, team_url, review_url, email) }} +{{ _( "Thanks for applying to use Gratipay! The next step is for us to review your application, " + "which happens on {a}this public GitHub issue{_a}. You can watch and participate in our " + "review there, and we will email you again when we finish (usually in about a week). " + "Thanks again!" + , a=''.format(review_url)|safe + , _a=''|safe + ) }} [---] text/plain -{{ _("Thanks for your project application for {0}! Your public project page is: " - "{1} " - "and your public review ticket is: " - "{2}. " - "You can watch and participate in our review process there. We will send a notification to {3} when we finish our review. " - "Thanks for applying!", - team_name, team_url, review_url, email) }} +{{ _( "Thanks for applying to use Gratipay! The next step is for us to review your application, " + "which happens on this public GitHub issue:") }} +{{ review_url }} + +{{ _( "You can watch and participate in our review there, and we will email you again when we " + "finish (usually in about a week). Thanks again!") }} diff --git a/emails/rejected-1.0.spt b/emails/rejected-1.0.spt index d235a5dd02..20a0d4e6a6 100644 --- a/emails/rejected-1.0.spt +++ b/emails/rejected-1.0.spt @@ -1,4 +1,4 @@ -{{ _("1.0 Payout: Rejected") }} +{{ _("1.0 payout rejected") }} [---] text/html {{ _("We've rejected your application for a payout of your Gratipay 1.0 balance, because we couldn't determine what product or service you were offering for which people were paying you on Gratipay. Please reply if you would like to discuss this further.") }} [---] text/plain diff --git a/emails/team-approved.spt b/emails/team-approved.spt index afbc2e1e82..718082d32d 100644 --- a/emails/team-approved.spt +++ b/emails/team-approved.spt @@ -1,4 +1,4 @@ -{{ _("Project Application Approved!") }} +{{ _("Project application approved!") }} [---] text/html {{ _( "We've approved your application for the '{team}' project on Gratipay. For details, please refer to {a}our review ticket{_a}." , team=team_name diff --git a/emails/team-rejected.spt b/emails/team-rejected.spt index 3aacdf3605..2865adb133 100644 --- a/emails/team-rejected.spt +++ b/emails/team-rejected.spt @@ -1,4 +1,4 @@ -{{ _("Project Application Rejected") }} +{{ _("Project application rejected") }} [---] text/html {{ _( "We've rejected your application for the '{team}' project on Gratipay. For details, please refer to {a}our review ticket{_a}." , team=team_name diff --git a/gratipay/application.py b/gratipay/application.py index 234f23c22c..1dc012f865 100644 --- a/gratipay/application.py +++ b/gratipay/application.py @@ -7,7 +7,7 @@ from .cron import Cron from .models import GratipayDB from .payday_runner import PaydayRunner -from .project_review_repo import ProjectReviewRepo +from .project_review_process import ProjectReviewProcess from .website import Website @@ -46,7 +46,7 @@ def __init__(self): self.install_periodic_jobs(website, env, db) self.website = website self.payday_runner = PaydayRunner(self) - self.project_review_repo = ProjectReviewRepo(env) + self.project_review_process = ProjectReviewProcess(env, db, self.email_queue) def install_periodic_jobs(self, website, env, db): diff --git a/gratipay/models/participant/packages.py b/gratipay/models/participant/packages.py index 792290cb63..93ab6fcc81 100644 --- a/gratipay/models/participant/packages.py +++ b/gratipay/models/participant/packages.py @@ -112,7 +112,7 @@ def finish_package_claims(self, cursor, nonce, *packages): team = package.get_or_create_linked_team(cursor, self) teams.append(team) team_ids.append(team.id) - review_url = self.app.project_review_repo.create_issue(*teams) + review_url = self.app.project_review_process.start(*teams) cursor.run('DELETE FROM claims WHERE nonce=%s', (nonce,)) cursor.run('UPDATE teams SET review_url=%s WHERE id=ANY(%s)', (review_url, team_ids,)) diff --git a/gratipay/models/team/__init__.py b/gratipay/models/team/__init__.py index e93abc7f27..8e1f1aab77 100644 --- a/gratipay/models/team/__init__.py +++ b/gratipay/models/team/__init__.py @@ -249,11 +249,6 @@ def get_upcoming_payment(self): """, {'team_id': self.id, 'mcharge': MINIMUM_CHARGE}) - def set_review_url(self, review_url): - self.db.run("UPDATE teams SET review_url=%s WHERE id=%s", (review_url, self.id)) - self.set_attributes(review_url=review_url) - - def get_og_title(self): out = self.name receiving = self.receiving diff --git a/gratipay/project_review_repo.py b/gratipay/project_review_process.py similarity index 58% rename from gratipay/project_review_repo.py rename to gratipay/project_review_process.py index 50b07c0f2b..756d7d2688 100644 --- a/gratipay/project_review_repo.py +++ b/gratipay/project_review_process.py @@ -9,18 +9,29 @@ from aspen import log from gratipay.exceptions import NoTeams +from gratipay.models.participant import Participant -class ProjectReviewRepo(object): +class ProjectReviewProcess(object): - def __init__(self, env): + def __init__(self, env, db, email_queue): repo = env.project_review_repo auth = (env.project_review_username, env.project_review_token) + self.db = db + self.email_queue = email_queue self._poster = GitHubPoster(repo, auth) if repo else ConsolePoster() - def create_issue(self, *teams): - """Given team objects, POST to GitHub, and return the URL of the new issue. + def start(self, *teams): + """Given team objects, kick off a review process by: + + 1. creating an issue in our project review repo on GitHub, and + 2. sending an email notification to the owner of the team(s). + + It's a bug to pass in teams that don't all have the same owner. + + :return: the URL of the new review issue + """ if not teams: raise NoTeams() @@ -31,12 +42,31 @@ def create_issue(self, *teams): title = "{} and {}".format(*[t.name for t in teams]) else: title = "{} and {} other projects".format(teams[0].name, nteams-1) + body = [] + team_ids = [] + owner_usernames = set() for team in teams: + team_ids.append(team.id) + owner_usernames.add(team.owner) body.append('https://gratipay.com{}'.format(team.url_path)) + assert len(owner_usernames) == 1, owner_usernames body.extend(['', '(This application will remain open for at least a week.)']) data = json.dumps({'title': title, 'body': '\n'.join(body)}) - return self._poster.post(data) + review_url = self._poster.post(data) + + self.db.run("UPDATE teams SET review_url=%s WHERE id = ANY(%s)", (review_url, team_ids)) + [team.set_attributes(review_url=review_url) for team in teams] + + owner = Participant.from_username(owner_usernames.pop()) + self.email_queue.put( owner + , 'project-review' + , review_url=team.review_url + , include_unsubscribe=False + , _user_initiated=False + ) + + return review_url class GitHubPoster(object): @@ -49,6 +79,8 @@ def __init__(self, repo, auth): self.auth = auth def post(self, data): + """POST data to GitHub and return the issue URL. + """ out = '' try: r = requests.post(self.api_url, auth=self.auth, data=data) @@ -73,6 +105,8 @@ def __init__(self, fp=sys.stdout): self.fp = fp def post(self, data): + """POST data to nowhere and return a URL of lies. + """ p = lambda *a, **kw: print(*a, file=self.fp) p('-'*78,) p(pprint.pformat(json.loads(data))) diff --git a/tests/py/test_email.py b/tests/py/test_email.py index 51b8aa6e24..a964a86048 100644 --- a/tests/py/test_email.py +++ b/tests/py/test_email.py @@ -796,7 +796,7 @@ def start(self, address, *package_names): self.alice.start_email_verification(address, *packages) return self.alice.get_email(address).nonce - @mock.patch('gratipay.project_review_repo.ConsolePoster.post') + @mock.patch('gratipay.project_review_process.ConsolePoster.post') def check(self, *package_names): package_names, post = package_names[:-1], package_names[-1] post.return_value = 'some-github-url' diff --git a/tests/py/test_project_review_process.py b/tests/py/test_project_review_process.py new file mode 100644 index 0000000000..ddbd70a219 --- /dev/null +++ b/tests/py/test_project_review_process.py @@ -0,0 +1,86 @@ +# -*- coding: utf-8 -*- +from __future__ import absolute_import, division, print_function, unicode_literals + +from cStringIO import StringIO + +import mock +from gratipay.testing import T +from gratipay.testing.email import QueuedEmailHarness +from pytest import raises + +from gratipay.project_review_process import ConsolePoster, ProjectReviewProcess +from gratipay.exceptions import NoTeams + + +class ENV_GH(object): + project_review_repo = 'some/repo' + project_review_username = 'cheeseburger' + project_review_token = 'di3tc0ke' + + +class ENV(object): + project_review_repo = '' + project_review_username = '' + project_review_token = '' + + +class Tests(QueuedEmailHarness): + + def setUp(self): + QueuedEmailHarness.setUp(self) + self.project_review_process = ProjectReviewProcess(ENV, self.db, self.app.email_queue) + + + def test_console_poster_posts_to_fp(self): + fp = StringIO() + poster = ConsolePoster(fp) + poster.post('{"blah": "blah blah"}') + fp.seek(0) + assert fp.read() == '''\ +------------------------------------------------------------------------------ +{u'blah': u'blah blah'} +------------------------------------------------------------------------------ +''' + + + @mock.patch('gratipay.project_review_process.requests.post') + def test_github_poster_attempts_to_post_to_github(self, post): + foo = self.make_team(name='foo') + bar = self.make_team(name='bar') + baz = self.make_team(name='baz') + + post.return_value = '' + + ProjectReviewProcess(ENV_GH, self.db, self.app.email_queue).start(foo, bar, baz) + + assert post.call_count == 1 + args, kwargs = post.mock_calls[0][1:] + assert args[0] == 'https://api.github.com/repos/some/repo/issues' + assert kwargs['data'] == ( + '{"body": "https://gratipay.com/foo/\\nhttps://gratipay.com/bar/\\n' + 'https://gratipay.com/baz/\\n\\n(This application will remain open ' + 'for at least a week.)", "title": "foo and 2 other projects"}') + assert kwargs['auth'] == ('cheeseburger', 'di3tc0ke') + + + def test_team_objects_get_review_url(self): + foo = self.make_team(name='foo') + assert foo.review_url is None + self.project_review_process.start(foo) + assert foo.review_url == T('foo').review_url == 'some-github-issue' + + + def test_owner_gets_an_email_notification(self): + foo = self.make_team(name='foo') + self.project_review_process.start(foo) + assert self.get_last_email()['subject'] == 'We have your application!' + + + def test_no_teams_raises(self): + raises(NoTeams, self.project_review_process.start) + + + def test_multiple_owners_raises(self): + foo = self.make_team(name='foo') + bar = self.make_team(name='bar', owner='crusher') + raises(AssertionError, self.project_review_process.start, foo, bar) diff --git a/tests/py/test_project_review_repo.py b/tests/py/test_project_review_repo.py deleted file mode 100644 index cfc0e6b7b1..0000000000 --- a/tests/py/test_project_review_repo.py +++ /dev/null @@ -1,58 +0,0 @@ -# -*- coding: utf-8 -*- -from __future__ import absolute_import, division, print_function, unicode_literals - -from cStringIO import StringIO - -import mock -from gratipay.testing import Harness -from pytest import raises - -from gratipay.project_review_repo import ConsolePoster, ProjectReviewRepo -from gratipay.exceptions import NoTeams - - -class Tests(Harness): - - def test_console_poster_posts_to_fp(self): - fp = StringIO() - poster = ConsolePoster(fp) - poster.post('{"blah": "blah blah"}') - fp.seek(0) - assert fp.read() == '''\ ------------------------------------------------------------------------------- -{u'blah': u'blah blah'} ------------------------------------------------------------------------------- -''' - - - @mock.patch('gratipay.project_review_repo.requests.post') - def test_github_poster_attempts_to_post_to_github(self, post): - post.return_value = '' - - class HackEnv: - project_review_repo = 'some/repo' - project_review_username = 'cheeseburger' - project_review_token = 'di3tc0ke' - env = HackEnv() - - class HackTeam: - def __init__(self, name): - self.name = name - self.url_path = '/'+name - - repo = ProjectReviewRepo(env) - repo.create_issue(*map(HackTeam, ['foo', 'bar', 'baz'])) - - assert post.call_count == 1 - args, kwargs = post.mock_calls[0][1:] - assert args[0] == 'https://api.github.com/repos/some/repo/issues' - assert kwargs['data'] == ( - '{"body": "https://gratipay.com/foo\\nhttps://gratipay.com/bar\\n' - 'https://gratipay.com/baz\\n\\n(This application will remain open ' - 'for at least a week.)", "title": "foo and 2 other projects"}') - assert kwargs['auth'] == ('cheeseburger', 'di3tc0ke') - - - def test_no_teams_raises(self): - class Env: __getattr__ = lambda *a: '' - raises(NoTeams, ProjectReviewRepo(Env()).create_issue) diff --git a/tests/py/test_teams.py b/tests/py/test_teams.py index 5f1b0ce868..9315b2eed9 100644 --- a/tests/py/test_teams.py +++ b/tests/py/test_teams.py @@ -245,7 +245,7 @@ def test_application_email_sent_to_owner(self): last_email = self.get_last_email() self.app.email_queue.flush() assert last_email['to'] == 'alice ' - expected = "Thanks for your project application for" + expected = "Thanks for applying to use Gratipay!" assert expected in last_email['body_text'] def test_401_for_anon_creating_new_team(self): diff --git a/www/teams/create.json.spt b/www/teams/create.json.spt index 68406ab291..cab1c34ae6 100644 --- a/www/teams/create.json.spt +++ b/www/teams/create.json.spt @@ -85,22 +85,8 @@ if request.method == 'POST': team.save_image(image, large, small, image_type) - review_url = website.app.project_review_repo.create_issue(team) - team.set_review_url(review_url) - - team_url = website.env.base_url + '/{}/'.format(team.slug) - - owner = Participant.from_username(team.owner) - website.app.email_queue.put( owner - , 'project-review' - , team_name=team.name - , team_url=team_url - , review_url=team.review_url - , email=user.participant.email_address - , include_unsubscribe=False - , _user_initiated=False - ) - + review_url = website.app.project_review_process.start(team) + team_url = website.env.base_url + team.url_path [---] application/json via json_dump {'review_url': review_url, 'team_url': team_url}