Edit claimed packages #4404
Edit claimed packages #4404
Conversation
chadwhitacre
changed the base branch from
master
to
project/claim-packages-view
chadwhitacre
force-pushed
the
project/claim-packages-edit
branch
from
cb5d10e
to
b789720
Compare
|
I'm pretty close on review url adaptations. Will pick up |
|
Hey, look what's on my profile page! :-)
|
|
After closing:
|
|
Broken image when 410 also affects non-package projects.
|
chadwhitacre
force-pushed
the
project/claim-packages-edit
branch
from
f70a898
to
17f256e
Compare
|
Review, Receiving: ✓
|
|
💃
|
|
Okay! Leaving off with this one, on to the next! |
chadwhitacre
force-pushed
the
project/claim-packages-edit
branch
from
f86148c
to
a10ca2e
Compare
gratipay/project_review_repo.py
Outdated
| """Given team objects, POST to GitHub, and return the URL of the new issue. | ||
| """ | ||
| if not teams: | ||
| return |
There was a problem hiding this comment.
Maybe log and/or raise an Exception, unless this is expected.
There was a problem hiding this comment.
Done in bcb5b11.
tests/py/test_teams.py
Outdated
| package = self.make_package(name='enterprise') | ||
| with self.db.get_cursor() as c: | ||
| team = package.create_linked_team(c, alice) | ||
| pytest.raises(AssertionError, team.update, field='foo') |
There was a problem hiding this comment.
Where is this testing homepage?
There was a problem hiding this comment.
Oops! Fixed in ff933e9. 😊
chadwhitacre
force-pushed
the
project/claim-packages-view
branch
3 times, most recently
from
ca01581
to
da0c3da
Compare
|
Half-way through rebasing, will pick up next time ... |
gratipay/models/participant/email.py
Outdated
| package_ids.append(package.id) | ||
| team = package.get_or_create_linked_team(cursor, self) |
There was a problem hiding this comment.
@dowski Related to #4397 (comment), it looks like this is where it becomes useful public API to return a team object from get_or_create_linked_team. You okay to bring that back here, to avoid an awkward package._load_team(cursor)?
There was a problem hiding this comment.
Done in 80d2655.
|
Rebased, was a10ca2e. |
chadwhitacre
force-pushed
the
project/claim-packages-edit
branch
from
a10ca2e
to
b21bda0
Compare
chadwhitacre
force-pushed
the
project/claim-packages-edit
branch
from
be18500
to
ff933e9
Compare
|
Actually, moved that to #4422 to keep this PR manageable (we target 400 max net lines changed per PR). |
dowski
changed the base branch from
project/claim-packages-view
to
project/claim-packages
| review_url = self.app.project_review_repo.create_issue(*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,)) |
There was a problem hiding this comment.
Is this stuff happening in a transaction? Maybe there's a test that verifies that or something - I'll look as I scroll down.
I didn't see one. Maybe you know? I can't really tell.
There was a problem hiding this comment.
If you're seeing cursor instead of self.db then yes, the expectation is that this is happening in a transaction. The transaction begins when the cursor is created, which happens in a context manager invocation. It will be rolled back if there's an exception, or committed when the context manager exits normally. Docs.
In terms of a test, I believe we're covered by the one added in eaa8699 under #4397, no?
Part of #4305, follows on #4398.
Todo
homepageto URL on npmjs.comproduct_or_serviceto npm descriptiondistributingdidn't check; let's make an 🐴 out of you and me