Subscriptions api #3909

aandis · 2016-01-30T14:02:26Z

Still WIP.

Added GET to show all subscriptions of a user along with filtering on team slug.
References #3460

aandis · 2016-01-30T14:09:21Z

One thing we had on the old tips.json was the ability to bulk update a bunch of tips in one POST. The Khan Academy setup used that, e.g.

payment_instruction.json doesn't support batch subscription creation and it also exposes some extra information not relevant to this endpoint. So, I thought I'd leave it alone and ditch the redirection thing and add all functionality directly to this.

aandis · 2016-01-30T14:10:27Z

So if that sounds good @rohitpaulk , @whit537, I'll go ahead and add POST to this to support batch updates.

aandis · 2016-01-30T14:12:56Z

Would appreciate a quick code review as well. :)

rohitpaulk · 2016-01-30T14:59:04Z

www/~/%username/subscriptions.json.spt

+    raise Response(403, _("Please sign in first."))
+
+else:
+    participant = user.participant


Use get_participant here to make sure the user exists, and that the authenticated user has the necessary permissions to access the endpoint. The current way this is implemented, looks like hitting www/~/aandis/subscriptions.json.spt is not going to give me (logged in as rohitpaulk) a 403/404, it is going to return my own subscriptions.

Oh yeah. I was yet to change that.

aandis · 2016-01-30T17:50:29Z

@rohitpaulk POST still doesn't work for me. Do you have any idea why? See #3460 (comment)

aandis · 2016-01-31T12:04:42Z

Added changes discussed. Yet to find a way around #3460 (comment)

aandis · 2016-02-01T07:06:03Z

Alright. Fixed #3460 (comment). 💃 Ended up modifying this after digging for a couple of hours. Looks correct to me. Not really sure if this will happen on prod too. If it does, we should fix this in aspen.
Anyway, I'll continue with POST later today! :)

aandis · 2016-02-01T20:06:20Z

POST added. 💃 Review needed.

btw Fixing #3460 (comment) made github and other provider's sign up redirects to stop working. I am sensing it's an ssl thing. But it's way easier for development compared to setting up ssl. 😉

techtonik · 2016-02-01T20:19:10Z

What users are subscribed to?

aandis · 2016-02-01T20:25:03Z

Giving to teams @techtonik. See #3460 for details. :)

chadwhitacre · 2016-02-01T20:29:49Z

POST added. 💃

!m @aandis :)

chadwhitacre · 2016-02-01T20:30:09Z

@aandis First thing I notice is that we'll need some tests for this endpoint.

aandis · 2016-02-01T20:36:58Z

Yep. That's next. Figured I should first review then write tests. :)

techtonik · 2016-02-01T20:43:10Z

That sounds strange. When I subscribe to something (especially for something), I expect to receive something. If I have a lot of subscribers, I automatically feel obliged to do something in exchange.

If we are still for "giving with no strings attached" then maybe there is a better term? Facebook has "liking", newspapers and lists have "subscribing", and when we send money the closest term is "supporting", but if we want more involvement and more positive value on the receiver's end, how do we call it? "links" to the causes, so we "link" people with "projects" and use "routes" to send them gratitude in the emotionless form of money that can be used to do something good for them. =)

chadwhitacre · 2016-02-01T20:43:20Z

@aandis Gotcha.

chadwhitacre · 2016-02-01T20:48:32Z

@techtonik You raise a good point: over in gratipay/inside.gratipay.com#117 we decided to go with "payment instructions" instead of "subscriptions" (gratipay/inside.gratipay.com#117 (comment) notwithstanding). Presumably #3460 predates that decision.

@aandis Let's go ahead and switch from "subscription" to "payment instruction" here. That'll line up with the database table and set_payment_instruction method name, as well.

chadwhitacre · 2016-02-01T20:50:42Z

You raise a good point

(Though we are no longer "for 'giving with no strings attached,'" post-gratipay/inside.gratipay.com#180 and pre-gratipay/inside.gratipay.com#423 .)

chadwhitacre · 2016-02-01T21:31:03Z

www/~/%username/subscriptions.json.spt

+
+    slug = request.qs.get("team_slug", "")
+
+    if slug: # Filter subscription info only for this team from the list.


How is this intended to related to the existing %team/payment-instruction.json.spt API? Looks like it at least partially duplicates the functionality over there. Why?

Only thing similar between them as of now is GET with team filter. But team/payment-instruction.json.spt returns a different response which isn't really relevant to this api. This endpoint offers an additional GET without a team filter to show all subscriptions of a user. Per #3909 (comment), we should maintain consistency in the api responses. Hence everything is implemented right here.

aandis · 2016-02-02T14:06:57Z

Let's go ahead and switch from "subscription" to "payment instruction"

Alright, will do. :)

chadwhitacre · 2016-02-05T19:58:51Z

@aandis Yeah that's most of it. We'd need some tests to land a PR in Aspen, and Aspen is undergoing big changes right now so you can decide if you want to spend time on that. :) The workaround would be to do hxt('PATCH', ...).

aandis · 2016-02-05T20:06:57Z

Cool. I'll look into it. Thanks :)

aandis · 2016-02-06T14:48:40Z

Readme updated. 💃

chadwhitacre · 2016-02-06T21:33:18Z

@aandis Nice! Can you rebase this PR branch onto the latest master? Seems we have conflicts.

aandis · 2016-02-07T08:59:28Z

👍

chadwhitacre · 2016-02-09T16:27:36Z

README looks fine. I hard-wrapped the new paragraphs and added a missing space in 63c74dd.

I think this is ready to go!

Except that ... @rohitpaulk Didn't you say somewhere that you want to move to /api/v1/ style endpoints? Was that a blocker for this PR?

rohitpaulk · 2016-02-09T17:41:46Z

Was that a blocker for this PR?

Not a blocker

Subscriptions api

chadwhitacre · 2016-02-09T20:04:23Z

!m @aandis

aandis · 2016-02-09T20:23:15Z

@whit537 If you have deployed this, can you confirm that POST to this endpoint works? I think I am still seeing signs of #3460 (comment) on production.

chadwhitacre · 2016-02-09T20:34:44Z

@aandis It's working for me (and I found a good example of #3876 ;).

chadwhitacre · 2016-02-09T20:36:28Z

(The second payment instruction is the one that changes in those screenshots—I guess including 0.00 tips is okay, so we can use this for the "Canceled" tab on the Giving page if we want. The first one to sudo room demonstrates the runaway dues.)

aandis · 2016-02-09T20:50:22Z

Heh okay! I see now. All this time I had been hitting /~/aandis/payment-instructions.json instead of /~aandis/payment-instructions.json. The former redirects and becomes a GET.

aandis · 2016-02-09T20:52:09Z

Learnt about http redirect methods in depth while overcoming this though. So all good in the end I guess. ;)

aandis · 2016-02-09T20:55:46Z

The doc instructs to hit /aandis/payment-instructions.json which also doesn't work. Should change that.

aandis · 2016-02-09T21:03:35Z

Ideally though, all these routes should work with POST too because a GET to any of them will give you identical response.

chadwhitacre · 2016-02-09T21:10:20Z

All this time I had been hitting /~/aandis/payment-instructions.json

Oh no! 🙀 Glad you figured it out. :)

The doc instructs to hit /aandis/payment-instructions.json which also doesn't work. Should change that.

#3916, ya?

aandis added the Review label Jan 30, 2016

rohitpaulk reviewed Jan 30, 2016
View reviewed changes

aandis force-pushed the subscriptions-api branch from a97c089 to 2a35603 Compare January 31, 2016 11:55

aandis added Review and removed Review labels Feb 1, 2016

chadwhitacre reviewed Feb 1, 2016
View reviewed changes

chadwhitacre mentioned this pull request Feb 1, 2016

Radar 44 gratipay/inside.gratipay.com#487

Closed

aandis force-pushed the subscriptions-api branch 2 times, most recently from 8db0f72 to eebf8f8 Compare February 3, 2016 18:19

aandis mentioned this pull request Feb 5, 2016

Use only one api for getting and setting payment instructions. #3913

Closed

aandis force-pushed the subscriptions-api branch from 0b3c069 to d2d848a Compare February 7, 2016 08:15

aandis added 5 commits February 7, 2016 14:24

Added simple GET without params.

6c8f011

Added GET with team filter.

e594dd1

Added POST

58a3122

Added tests

e5f7c3e

Update readme.

7cce577

aandis force-pushed the subscriptions-api branch from d2d848a to 7cce577 Compare February 7, 2016 08:56

chadwhitacre mentioned this pull request Feb 9, 2016

Radar 45 gratipay/inside.gratipay.com#494

Closed

Hard-wrap and insert a space

63c74dd

chadwhitacre added a commit that referenced this pull request Feb 9, 2016

Merge pull request #3909 from gratipay/subscriptions-api

ca9c608

Subscriptions api

chadwhitacre merged commit ca9c608 into master Feb 9, 2016

chadwhitacre deleted the subscriptions-api branch February 9, 2016 19:53

chadwhitacre mentioned this pull request Feb 9, 2016

tweak URL for payment-instructions.json in README #3916

Merged


		slug = request.qs.get("team_slug", "")

		if slug: # Filter subscription info only for this team from the list.

Subscriptions api #3909

Subscriptions api #3909

Conversation

aandis commented Jan 30, 2016

aandis commented Jan 30, 2016

aandis commented Jan 30, 2016

aandis commented Jan 30, 2016

rohitpaulk Jan 30, 2016

Choose a reason for hiding this comment

aandis Jan 30, 2016

Choose a reason for hiding this comment

aandis commented Jan 30, 2016

aandis commented Jan 31, 2016

aandis commented Feb 1, 2016

aandis commented Feb 1, 2016

techtonik commented Feb 1, 2016

aandis commented Feb 1, 2016

chadwhitacre commented Feb 1, 2016

chadwhitacre commented Feb 1, 2016

aandis commented Feb 1, 2016

techtonik commented Feb 1, 2016

chadwhitacre commented Feb 1, 2016

chadwhitacre commented Feb 1, 2016

chadwhitacre commented Feb 1, 2016

chadwhitacre Feb 1, 2016

Choose a reason for hiding this comment

aandis Feb 2, 2016

Choose a reason for hiding this comment

aandis commented Feb 2, 2016

chadwhitacre commented Feb 5, 2016

aandis commented Feb 5, 2016

aandis commented Feb 6, 2016

chadwhitacre commented Feb 6, 2016

aandis commented Feb 7, 2016

chadwhitacre commented Feb 9, 2016

rohitpaulk commented Feb 9, 2016

chadwhitacre commented Feb 9, 2016

aandis commented Feb 9, 2016

chadwhitacre commented Feb 9, 2016

chadwhitacre commented Feb 9, 2016

aandis commented Feb 9, 2016

aandis commented Feb 9, 2016

aandis commented Feb 9, 2016

aandis commented Feb 9, 2016

chadwhitacre commented Feb 9, 2016