asimlqt / php-google-spreadsheet-client

A PHP library for accessing and manipulating Google Spreadsheets
Other
544 stars 152 forks source link

Added ability to access public spreadsheets. #102

Closed MarkMaldaba closed 8 years ago

MarkMaldaba commented 9 years ago

These are spreadsheets that have been published using the "Publish to the web" feature (not to be confused with those that have been made public using the "Public on the web" visibility option).

The ServiceRequest should be initialised with the sheet's key as its $accessToken, and $tokenType should be set to "basic". You can then use the PublicSpreadsheetService to access the sheet.

Example:

use Google\Spreadsheet\DefaultServiceRequest;
use Google\Spreadsheet\ServiceRequestFactory;

$serviceRequest = new DefaultServiceRequest($key, "basic");
ServiceRequestFactory::setInstance($serviceRequest);
$spreadsheetService = new Google\Spreadsheet\PublicSpreadsheetService();

// Get cells from first worksheet.  Not sure if non-numeric ids can be used here - the API docs specify a number should be used.
$cells = $spreadsheetService->getCellFeed(1);

I have exposed the $accessToken in the DefaultServiceRequest class, via a new getAccessToken() method, so that this can be accessed from the PublicSpreadsheetService class.

The new PublicSpreadsheetService class is based on SpreadsheetService, but does not implement the getSpreadsheetById() method, as this is not possible and makes no sense in the context of a public sheet. There seemed little point extending from the SpreadsheetService class, as the code is not sufficiently re-usable between the two contexts.

asimlqt commented 9 years ago

Hi Mark,

Thanks for the PR. I checked this out and unfortunately there's some issues with it, If you can resolve them then I can merge it in.

  1. Using the access token in DefaultServiceRequest is confusing. A lot of people will have trouble with using this class and raise issues. It will be much easier to just add a constructor to PublicSpreadsheetService which accepts the spreadsheet id or set it as the first parameter on all the methods in the class and bypass the DefaultServiceRequest altogether.
  2. If you can write docs (in README.md) as well that'll be really handy.
  3. The getSpreadsheets method is wrong, should be named getWorksheets and must return an instance of WorksheetFeed and not SpreadsheetFeed. (I've not tested the other 2 methods yet)
  4. Any unit tests will also be useful.

Thanks Asim

MarkMaldaba commented 9 years ago

Hi @asimlqt, thanks for your feedback! I will try and improve this PR to an acceptable standard.

Re: your points:

  1. I think I could probably do with some guidance for this point. I'm not sure what you are referring to when you say "A lot of people will have trouble with using this class and raise issues". Can you elaborate? I'm also not sure how we could bypass the DefaultServiceRequest class, as we can't do this without making a service request!
  2. I included some documentation as part of the commit message (everything up to and including the example code). Is this the kind of thing you mean? If so, this will be easy to copy into README.md, if that's the best place for it.
  3. If I've understood this correctly then this should be an easy fix.
  4. I will take a look, but I'm not sure if I have the expertise to write meaningful tests. Is this is a requirement for the PR to be accepted? If so, I may need some help.

-- Mark

asimlqt commented 9 years ago

Hi Mark,

re:1 - Yes you're right, it won't be that easy to bypass the DefaultServiceRequest class however using the spreadsheet id place of the access token in the constructor is confusing. Maybe we should update the class to support a "visibility" property and make the constructor params optional. I'll have a think on it and see what the best way of supporting it is.

MarkMaldaba commented 9 years ago

I agree it is a little confusing, but that might just be a variable naming issue. You have a think and let me know how you want me to proceed (or, if you have a strong idea and just want to dive in and make the changes yourself, then please go ahead!) :-)

I'll wait to hear back from you.

asimlqt commented 8 years ago

I have added accessing public spreadsheet support in v3.