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

Create a migrate tips function #3424

Merged
merged 4 commits into from
May 28, 2015
Merged

Create a migrate tips function #3424

merged 4 commits into from
May 28, 2015

Conversation

rohitpaulk
Copy link
Contributor

For #3399, includes #3420

@chadwhitacre chadwhitacre mentioned this pull request May 14, 2015
@rohitpaulk rohitpaulk changed the title Migrate tips Create a migrate tips function May 14, 2015
@chadwhitacre
Copy link
Contributor

!m @rohitpaulk

@chadwhitacre
Copy link
Contributor

Once #3415 settles down let's figure out how to merge #3399 (comment) and this.

@chadwhitacre
Copy link
Contributor

Rebased on master.

INSERT INTO subscriptions
(ctime, mtime, subscriber, team, amount, is_funded)
SELECT ctime
, mtime
Copy link
Contributor

Choose a reason for hiding this comment

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

I would leave this out and let mtime be set to now. No?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, I'm not sure. We use mtime to show when the tip was last modified in the UI (Last set). Do we want that to be the time when the user last set it, or the time when we migrated the tip?

@chadwhitacre
Copy link
Contributor

I suppose we want to run this when we approve a team for the first time.

@chadwhitacre
Copy link
Contributor

How does this compare with the SQL I used the other day?

#3399 (comment)

@rohitpaulk
Copy link
Contributor Author

How does this compare with the SQL I used the other day?

That excludes tips from suspicious/closed/unclaimed accounts and also checks for amount > 0. Makes sense to exclude those here too.

@rohitpaulk
Copy link
Contributor Author

I suppose we want to run this when we approve a team for the first time.

Should we also send the tippers (subscribers, now) a notification?

@chadwhitacre
Copy link
Contributor

@rohitpaulk Yes, definitely. Good call.

@chadwhitacre
Copy link
Contributor

We should go back and notify the new subscribers from last week as well.

@rohitpaulk
Copy link
Contributor Author

Rebased on master.

@rohitpaulk
Copy link
Contributor Author

@whit537 - I'm going to use this for #3486 (comment), can you have a look and merge it in?

@chadwhitacre
Copy link
Contributor

Yes, just reread it, looks good. Go ahead and merge once Travis runs green.

rohitpaulk added a commit that referenced this pull request May 28, 2015
Create a migrate tips function
@rohitpaulk rohitpaulk merged commit eea8ea7 into master May 28, 2015
@rohitpaulk rohitpaulk deleted the migrate-tips branch May 28, 2015 23:21
@rohitpaulk
Copy link
Contributor Author

Deploying...

@rohitpaulk
Copy link
Contributor Author

Deployed.

@chadwhitacre
Copy link
Contributor

Awesome. Now to call it for the users in question?

@rohitpaulk
Copy link
Contributor Author

I tested this on a backup, I can confirm that tips are migrated and an error is raised if the migration is attempted twice.

@rohitpaulk
Copy link
Contributor Author

Now to call it for the users in question?

Yep. Heading back to #3486...

@rohitpaulk rohitpaulk restored the migrate-tips branch May 29, 2015 01:45
@rohitpaulk rohitpaulk deleted the migrate-tips branch May 29, 2015 01:46
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.

2 participants