Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New EDD_API Class #857

Closed
pippinsplugins opened this Issue Feb 9, 2013 · 50 comments

Comments

Projects
None yet
6 participants
@pippinsplugins
Copy link
Member

commented Feb 9, 2013

We're going to introduce a new API that makes it possible to retrieve sales data, earnings, product information, etc in a JSON/XML format.

One of the many uses for this API will be mobile sales tracking apps, or mobile store UIs.

Initially, we're looking at incorporating the following features:

  1. Retrieve sale status for all products
  2. Retrieve earning stats for all products
  3. Retrieve 1 and 2 for individual products
  4. Retrieve sales for current day
  5. Retrieve earnings for current day
  6. Retrieve earnings / sales for any date / date range
  7. Retrieve status on customers (number / emails)
  8. Retrieve product information (name, description, price, etc)
@pippinsplugins

This comment has been minimized.

Copy link
Member Author

commented Feb 9, 2013

Initially, the API will only provide GET methods, no methods for PUT.

@pippinsplugins

This comment has been minimized.

Copy link
Member Author

commented Feb 9, 2013

Authentication (for GET methods) will be based on API key / email address.

@evertiro

This comment has been minimized.

Copy link
Member

commented Feb 9, 2013

Started working on the base class for the API. I'll keep everyone updated as development continues.

@evertiro

This comment has been minimized.

Copy link
Member

commented Feb 9, 2013

Any thoughts on where the API should be accessible?

@pippinsplugins

This comment has been minimized.

Copy link
Member Author

commented Feb 9, 2013

It should be front and back, at least for the GET methods.

@evertiro

This comment has been minimized.

Copy link
Member

commented Feb 9, 2013

You misunderstood the question. I meant in regards to URL structure

@pippinsplugins

This comment has been minimized.

Copy link
Member Author

commented Feb 9, 2013

Whoops! I would say register a new URL endpoint for "edd-api".

@evertiro

This comment has been minimized.

Copy link
Member

commented Feb 9, 2013

So we want to use edd-api. Ok! That's all I needed.

@evertiro

This comment has been minimized.

Copy link
Member

commented Feb 14, 2013

Just to keep everyone up to date, the API initialization functions have been completed, and I'm about 50% done the key generation and management.

Additionally, the API is being designed as a class to make extension easy on anyone who want to...

@pippinsplugins

This comment has been minimized.

Copy link
Member Author

commented Feb 14, 2013

Fantastic, thanks for the update!

@Geczy

This comment has been minimized.

Copy link
Contributor

commented Feb 14, 2013

Why not just use authentication with WP shop admin credentials ?

@pippinsplugins

This comment has been minimized.

Copy link
Member Author

commented Feb 14, 2013

Because that doesn't work nearly as well for things like mobile apps.

It's better to enter an API key in a mobile app than it is a username and password.

It is also more secure because if the API key gets stolen, then only thing the perpetrator can do is read data, whereas with an admin user account they can do anything.

@Geczy

This comment has been minimized.

Copy link
Contributor

commented Feb 14, 2013

Good point ! 👍

It's not the combo of api key + email is it? Just api I hope?

@pippinsplugins

This comment has been minimized.

Copy link
Member Author

commented Feb 14, 2013

It might be API + email, but not sure. @ghost1227 ?

@evertiro

This comment has been minimized.

Copy link
Member

commented Feb 14, 2013

What's not the combo of api key and email? The access credentials for the API? If so, the answer is yes, unless someone gives me a better idea. As of now, it's set up such that you would pass parameters &username=&key= in order to authenticate.

@Geczy

This comment has been minimized.

Copy link
Contributor

commented Feb 14, 2013

Well IMO, the better idea is to just use key=, without username.

@Geczy

This comment has been minimized.

Copy link
Contributor

commented Feb 14, 2013

Do you have this on Github anywhere? Can we see what you have currently?

@pippinsplugins

This comment has been minimized.

Copy link
Member Author

commented Feb 14, 2013

I have no issues with the username. The only argument for not including it I can see is it's simpler, but I don't think that's a good enough argument in this case.

@Geczy

This comment has been minimized.

Copy link
Contributor

commented Feb 14, 2013

What's the argument to keep username? It seems useless.

@pippinsplugins

This comment has been minimized.

Copy link
Member Author

commented Feb 14, 2013

More secure.

@Geczy

This comment has been minimized.

Copy link
Contributor

commented Feb 14, 2013

How?

If the api key is compromised, it's logical to assume so is the username.

@pippinsplugins

This comment has been minimized.

Copy link
Member Author

commented Feb 14, 2013

Not necessarily. Someone could get hold of the API key but not know the username. While a lot of the time they would have both, it's very safe to assume they could also have only gotten one. It's an extra level that can only help.

@Geczy

This comment has been minimized.

Copy link
Contributor

commented Feb 14, 2013

Seems like a false sense of security to me :/

Most smaller APIs I know of only require an API key to function, eg rottentomatoes or something

@Geczy

This comment has been minimized.

Copy link
Contributor

commented Feb 14, 2013

I think what'd be safer is to track API key usage in the backend.

@pippinsplugins

This comment has been minimized.

Copy link
Member Author

commented Feb 14, 2013

No, it's not a false sense of security at all. It may not always make much difference, but there will absolutely be cases where it does. There is zero benefit to not using the username.

@pippinsplugins

This comment has been minimized.

Copy link
Member Author

commented Feb 14, 2013

Tracking key usage would be great. I'd say we setup another view for the Logs page that shows API access usage.

@evertiro

This comment has been minimized.

Copy link
Member

commented Feb 14, 2013

That sounds like a good idea, I'll add it to the list!

In response to the argument against usernames, it's technically possible (however unlikely) to brute-force a key. Managing to brute-force both key and username is almost impossible without knowing the username.

@Geczy

This comment has been minimized.

Copy link
Contributor

commented Feb 14, 2013

I would rather see an 'api secret' than username, then. Something not personally identifiable on first glance if compromised.

@Geczy

This comment has been minimized.

Copy link
Contributor

commented Feb 14, 2013

@pippinsplugins

This comment has been minimized.

Copy link
Member Author

commented Feb 14, 2013

For an API secret, is that really just a separate API key? Similar to how Stripe works?

pippinsplugins added a commit that referenced this issue Feb 24, 2013

pippinsplugins added a commit that referenced this issue Feb 24, 2013

pippinsplugins added a commit that referenced this issue Mar 6, 2013

@pippinsplugins

This comment has been minimized.

Copy link
Member Author

commented Mar 6, 2013

Since it's very possible for a store to have hundreds of products, retrieving all products (or customers) wasn't a good idea. I've added a couple of helper functions and pagination for all queries.

You can now control the number of results shown per page by adding &page=# to the query. The number of items shown per page is controlled with &number=#.

pippinsplugins added a commit that referenced this issue Mar 6, 2013

@pippinsplugins

This comment has been minimized.

Copy link
Member Author

commented Mar 6, 2013

Need to read through this before we consider this finished: http://info.apigee.com/Portals/62317/docs/web%20api.pdf

pippinsplugins added a commit that referenced this issue Mar 8, 2013

@pippinsplugins

This comment has been minimized.

Copy link
Member Author

commented Mar 8, 2013

I've just pushed up an update to simply things a little.

Instead of doing this:

http://localhost/wordpress/edd-api/?user=pippin@pippinsplugins.com&key=7ede50bc7b8264b15bad22475bf2a0ce&query=customers

One will simply do this:

http://localhost/wordpress/edd-api/customers?user=pippin@pippinsplugins.com&key=7ede50bc7b8264b15bad22475bf2a0ce

Instead of explicitly passing a query parameter, we simply use the value of edd-api to set the query mode. This makes it simpler and more intuitive.

In the same way that we would expect a site to use /customers, /products, etc, the API now does the same this:

  • /edd-api/customers
  • /edd-api/products
  • /edd-api/sales
  • /edd-api/stats

pippinsplugins added a commit that referenced this issue Mar 8, 2013

pippinsplugins added a commit that referenced this issue Mar 8, 2013

pippinsplugins added a commit that referenced this issue Mar 8, 2013

pippinsplugins added a commit that referenced this issue Mar 8, 2013

pippinsplugins added a commit that referenced this issue Mar 8, 2013

@sunnyratilal

This comment has been minimized.

Copy link
Member

commented Mar 8, 2013

After the rewrite endpoint is added, flush_rewrite_rules() should be added because when I went in and enabled the API keys to be generated from EDD settings and then generated one, I tried accessing the edd-api and it resulted in 404. Had to go into Settings > Permalinks and hit Save Permalinks.

pippinsplugins added a commit that referenced this issue Mar 8, 2013

@pippinsplugins

This comment has been minimized.

Copy link
Member Author

commented Mar 8, 2013

@sunnyratilal Already noted #969

@sunnyratilal

This comment has been minimized.

Copy link
Member

commented Mar 8, 2013

Whoops, didn't see that! :)

On Friday, 8 March 2013, Pippin Williamson wrote:

@sunnyratilal https://github.com/sunnyratilal Already noted #969https://github.com/pippinsplugins/Easy-Digital-Downloads/issues/969


Reply to this email directly or view it on GitHubhttps://github.com/pippinsplugins/Easy-Digital-Downloads/issues/857#issuecomment-14642955
.

@pippinsplugins

This comment has been minimized.

Copy link
Member Author

commented Mar 8, 2013

We should add also "predefined dates" to the stats query:

  • last_year
  • this_year
  • this_month
  • last_month
  • this_week
  • last_month

pippinsplugins added a commit that referenced this issue Mar 8, 2013

pippinsplugins added a commit that referenced this issue Mar 8, 2013

@pippinsplugins

This comment has been minimized.

Copy link
Member Author

commented Mar 9, 2013

The numerical index needs to be removed from customers:

 "0": {
    "info": {
        "id": -1,
        "username": "Guest",
        "display_name": "Guest",
        "first_name": "Guest",
        "last_name": "Guest",
        "email": "johnson@test.com"
    },
    "stats": {
        "total_purchases": 2,
        "total_spent": "20",
        "total_downloads": 0
    }
},
"1": {
    "info": {
        "id": -1,
        "username": "Guest",
        "display_name": "Guest",
        "first_name": "Guest",
        "last_name": "Guest",
        "email": "asda@test.com"
    },
    "stats": {
        "total_purchases": 0,
        "total_spent": "0",
        "total_downloads": 0
    }
},
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.