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

Team creation #3412

Merged
merged 17 commits into from
May 14, 2015
Merged

Team creation #3412

merged 17 commits into from
May 14, 2015

Conversation

chadwhitacre
Copy link
Contributor

For #3399. cc: @rohitpaulk

@chadwhitacre chadwhitacre added this to the Pivot milestone May 13, 2015
@chadwhitacre chadwhitacre mentioned this pull request May 13, 2015
@rohitpaulk rohitpaulk force-pushed the team-creation branch 2 times, most recently from 24f040a to b4bf43d Compare May 13, 2015 12:21
@rohitpaulk
Copy link
Contributor

@whit537 - I feel like we're approaching this the wrong way. We're going to deliver a bad user experience if we attempt to get this done before tomorrow. If only a handful of teams (including us) are going to take part in this payday, why not collect the question/answers from them (via email) manually? Shouldn't we focus on getting a few 'beta' users and processing payments for them first?

I'm suggesting that we let this (building an application process) wait, and focus on how we're going to run Payday this Thursday.

@chadwhitacre
Copy link
Contributor Author

I definitely agree that our focus is on getting payday run this Thursday with a few beta users. If we don't widely advertise the /new page or link it in the nav then only those users who are paying attention to the relevant tickets here on GitHub will know about it. That'll be effectively the same as collecting the information via email and entering it in the db manually. I think it's okay to have a rough user experience here since we're not broadcasting this capability widely yet.

@rohitpaulk
Copy link
Contributor

I think it's okay to have a rough user experience here since we're not broadcasting this capability widely yet.

Ah, okay :)

@chadwhitacre chadwhitacre mentioned this pull request May 13, 2015
@chadwhitacre
Copy link
Contributor Author

I've been thinking about i18n for this. I don't think we need a full multi-language implementation like we've got for statements, because a team is going to have one primary language they work in. They can just answer the questions in whatever language they want (though that'll pose an interesting challenge in terms of our ability to review new teams). I think it would be good to have a "primary language" field for Teams (which should be pretty easy to implement), but beyond that I don't think we need to worry about i18n.

@chadwhitacre
Copy link
Contributor Author

More important is that the form needs to include a checkbox to accept the terms of service.

@rohitpaulk rohitpaulk force-pushed the team-creation branch 2 times, most recently from 60cfdd8 to 6f06e26 Compare May 13, 2015 18:48
@rohitpaulk
Copy link
Contributor

Rebased on master.

@rohitpaulk
Copy link
Contributor

Not the best UX, but it works. Tagging as ready for review...

@chadwhitacre
Copy link
Contributor Author

Awesome! I'm working on terms (#3408), and I'll look at this next.

@chadwhitacre
Copy link
Contributor Author

We want to require some things of participants before we allow them to register:

  • a working payout route
  • a verified email
  • really, identity verification, but that's too much for now :-(

@chadwhitacre
Copy link
Contributor Author

Alright, I'm satisfied with this as far as it goes. We shouldn't merge this until #3408 lands, because the whole point is to have people accept our new terms. Also, anyone using this will need their tips migrated or it's all for naught. Migrating tips depends on the schema that's to be built on #3414. Therefore, I think we need to hold tight with this PR for now. I guess we may as well implement migration on this PR since we're blocked on it.

@chadwhitacre
Copy link
Contributor Author

The migration experience should be that if a user creates a team, and that user has select count(*) from current_tips > 0, then we should add a notice to the /new form that we'll migrate their tips to the new team. And then we should migrate their tips. :-)

@chadwhitacre
Copy link
Contributor Author

And then we should migrate their tips. :-)

We can do that independently of this PR. Unblocking on migration. Still blocked on #3408.

@rohitpaulk
Copy link
Contributor

@whit537 - Who'd be the owner of the Gratipay team? whit537 or Gratipay? What happens if other teams apply with their existing 'team account'? Like sudoroom for example, should they log in with sudoroom to apply for a team, or should the owner use their personal account?

@chadwhitacre
Copy link
Contributor Author

The ~Gratipay user will be the owner of the Gratipay team. Sudo Room should use the ~sudoroom user account to apply for the sudoroom team.

rohitpaulk and others added 11 commits May 14, 2015 09:32
Python lesson time! :D

Python dicts are mutable, which means that this will modify foo:

>>> foo = {1: 2}
>>> bar = foo
>>> bar[1] = 3
>>> foo
{1: 3}

You can tell that foo and bar are the same identical object by comparing
their memory address:

>>> id(foo) == id(bar)
True

An easy way around this is to make a shallow copy using the dict
constructor (shallow means non-recursive):

>>> foo = {1: 2}
>>> bar = dict(foo)
>>> bar[1] = 3
>>> foo
{1: 2}
>>> id(foo) == id(bar)
False
 - must have a verified email
 - must have a payout route
 - can't be stub, closed, or suspicious
We don't need to migrate when the user applies. We need to migrate
before we run payday. Users can apply for a team and we can migrate
later.
@chadwhitacre
Copy link
Contributor Author

Rebased on master post-#3408.

Restructured a bit to fit this.
chadwhitacre added a commit that referenced this pull request May 14, 2015
@chadwhitacre chadwhitacre merged commit b7cf46e into master May 14, 2015
@chadwhitacre chadwhitacre deleted the team-creation branch May 14, 2015 13:44
@webmaven
Copy link
Contributor

Error messages should link to the form that needs to be completed (in my case, I hadn't verified my email address on the settings form, but I suspect that filling out the identity form also prevented a similar error).

@webmaven
Copy link
Contributor

Ah, and now I need to attach a bank account (also still not linking to the appropriate form).

May I suggest that if several requirements are not met, the error message list all of them, rather than making us play whack-a-mole?

@chadwhitacre
Copy link
Contributor Author

@webmaven Yeah, we cut UX corners in the interest of time, sorry. :-(

@chadwhitacre
Copy link
Contributor Author

Reticketed as #3422.

@rohitpaulk rohitpaulk mentioned this pull request May 14, 2015
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