Skip to content
This repository has been archived by the owner on Feb 8, 2018. It is now read-only.

Present claimed packages as projects #4398

Merged
merged 5 commits into from
Apr 27, 2017

Conversation

chadwhitacre
Copy link
Contributor

@chadwhitacre chadwhitacre commented Apr 4, 2017

Part of #4305, follows on #4397.

Todo

@chadwhitacre chadwhitacre changed the base branch from master to project/claim-packages-finish April 4, 2017 20:09
@chadwhitacre
Copy link
Contributor Author

screen shot 2017-04-04 at 4 06 22 pm

@chadwhitacre
Copy link
Contributor Author

3347ae3 😍

screen shot 2017-04-04 at 4 16 00 pm

@chadwhitacre
Copy link
Contributor Author

Yesterday I started down a path of using Aspen typecasters to DRY up in a more general way. PR forthcoming against master for typecasting implementation, blocking this on that.

@chadwhitacre
Copy link
Contributor Author

chadwhitacre commented Apr 10, 2017

Alright, so we're looking at either implementing proper internal redirects, or serving package-projects on the same paths as regular projects after all.

The former is proving tricky (we either greatly complicate the website.algorithm, or we figure out how to rerun the entire algorithm). The latter should be fairly straightforward, though I guess that means further crowding our global project namespace w/ all of the npm namespace, putting more pressure on #4246.

@chadwhitacre
Copy link
Contributor Author

I guess that means we need to try to name projects after the underlying package on-claim, falling back as with usernames.

@chadwhitacre chadwhitacre mentioned this pull request Apr 10, 2017
15 tasks
@chadwhitacre
Copy link
Contributor Author

Bumped remaining todos to #4404. Ready for review!

@chadwhitacre
Copy link
Contributor Author

chadwhitacre commented Apr 11, 2017

c34da4f or thereabouts breaks www/%team/charts.json.spt. Write a test for that and review other cases as well.

pid-89470 thread-140735094956800 (MainThread) Traceback (most recent call last):
pid-89470 thread-140735094956800 (MainThread)   File "/Users/whit537/personal/gratipay/gratipay.com/env/lib/python2.7/site-packages/algorithm.py", line 288, in run
pid-89470 thread-140735094956800 (MainThread)     new_state = function(**deps.as_kwargs)
pid-89470 thread-140735094956800 (MainThread)   File "/Users/whit537/personal/gratipay/gratipay.com/env/lib/python2.7/site-packages/aspen/algorithms/website.py", line 115, in get_response_for_resource
pid-89470 thread-140735094956800 (MainThread)     return {'response': resource.respond(state)}
pid-89470 thread-140735094956800 (MainThread)   File "/Users/whit537/personal/gratipay/gratipay.com/env/lib/python2.7/site-packages/aspen/http/resource.py", line 59, in respond
pid-89470 thread-140735094956800 (MainThread)     content_type, body = super(Dynamic, self).respond(accept, state)
pid-89470 thread-140735094956800 (MainThread)   File "/Users/whit537/personal/gratipay/gratipay.com/env/lib/python2.7/site-packages/aspen/simplates/__init__.py", line 169, in respond
pid-89470 thread-140735094956800 (MainThread)     exec(self.pages[1], context)
pid-89470 thread-140735094956800 (MainThread)   File "/Users/whit537/personal/gratipay/gratipay.com/www/%team/charts.json.spt", line 48, in <module>
pid-89470 thread-140735094956800 (MainThread)     """, (slug,), back_as=dict)
pid-89470 thread-140735094956800 (MainThread)   File "/Users/whit537/personal/gratipay/gratipay.com/env/lib/python2.7/site-packages/postgres/__init__.py", line 548, in all
pid-89470 thread-140735094956800 (MainThread)     return cursor.all(sql, parameters)
pid-89470 thread-140735094956800 (MainThread)   File "/Users/whit537/personal/gratipay/gratipay.com/env/lib/python2.7/site-packages/postgres/cursors.py", line 145, in all
pid-89470 thread-140735094956800 (MainThread)     self.execute(sql, parameters)
pid-89470 thread-140735094956800 (MainThread)   File "/Users/whit537/personal/gratipay/gratipay.com/env/lib/python2.7/site-packages/psycopg2/extras.py", line 223, in execute
pid-89470 thread-140735094956800 (MainThread)     return super(RealDictCursor, self).execute(query, vars)
pid-89470 thread-140735094956800 (MainThread) ProgrammingError: can't adapt type 'Team'

@@ -87,3 +87,6 @@ def __str__(self):
return "Negative balance not allowed in this context."

class NotWhitelisted(Exception): pass


class OutOfOptions(Exception): pass
Copy link
Contributor

Choose a reason for hiding this comment

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

A clearer name or a docstring would help this Exception class.

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 in 3712318.

@@ -1,6 +1,10 @@
# -*- coding: utf-8 -*-
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this whole change be merged with master so that changes in the previous PR are reflected here? It should make it easier to review.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hopefully da0c3da helps?


if team is None:
# Try to redirect to a Participant.
from gratipay.models.participant import Participant # avoid circular import
Copy link
Contributor

Choose a reason for hiding this comment

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

Open a new GH issue to refactor this so you don't have to lazy-import. Circular dependencies are a hassle.

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: #4421.

@chadwhitacre
Copy link
Contributor Author

Rebased, was 89202f8.

@chadwhitacre
Copy link
Contributor Author

Rebasing, determined from #4397 (comment) that 757b498 and previous can be pushed off of here. The work starts at 8151f86.

@chadwhitacre
Copy link
Contributor Author

Squashed to ease rebase, was 89202f8.

@chadwhitacre
Copy link
Contributor Author

Not sure what I was thinking. Conflict resolution is effectively the same. Lessee here ...

@chadwhitacre
Copy link
Contributor Author

Okay! Now to address review and todo ...

Since alice is a user we were getting redirects to /~alice.
@chadwhitacre
Copy link
Contributor Author

Alright @dowski, another one on deck!

Copy link
Contributor

@dowski dowski left a comment

Choose a reason for hiding this comment

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

One minor thing, but overall looks good.

return team
break
else:
raise OutOfOptions()
Copy link
Contributor

Choose a reason for hiding this comment

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

This code will never run, unless UUID4 is broken. I'd put my money on UUID4 and remove the else and the exception.

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 in 132eceb.

@chadwhitacre chadwhitacre changed the base branch from project/claim-packages-finish to project/claim-packages April 26, 2017 08:51
@dowski dowski merged commit f50cf4e into project/claim-packages Apr 27, 2017
@chadwhitacre chadwhitacre deleted the project/claim-packages-view branch April 27, 2017 08:00
chadwhitacre added a commit that referenced this pull request Apr 28, 2017
chadwhitacre added a commit that referenced this pull request Apr 28, 2017
chadwhitacre added a commit that referenced this pull request May 5, 2017
chadwhitacre added a commit that referenced this pull request May 9, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants