fix google docs for xlsx
We use a very old version of the google docs api, and according to the api docs (https://developers.google.com/google-apps/documents-list/), when selecting an export format to download a spreadsheet, it should not matter whether you specify 'xls' or 'xlsx', you should get the same file. However, in manual testing, this is not true. For old files, there are slight differences between the binary files that are downloaded between the two specified file extensions, and for new files, specifying 'xls' returns an error rather than a file, while 'xlsx' still works. One option would be to switch all spreadsheet downloads from google docs. Since the binaries were slightly different on older docs, in order to prevent changing old behavior (and because this whole api has to be totally replaced in the next few months), I decided to implement this as a fallback exception, where it still tries 'xls' first but if that option fails, then it falls back to trying 'xlsx'. fixes CNVS-16689 test plan: - authenticate a user with google docs - create a new spreadsheet in google drive - create an assignment that accepts google docs submissions - go to submit the assignment, and choose the spreadsheet - submit the assignment - it should submit successfully Change-Id: Ic93ff6267de33106fb8e3bb3bd23d14c90f3563c Reviewed-on: https://gerrit.instructure.com/48014 Reviewed-by: Brad Horrocks <bhorrocks@instructure.com> Tested-by: Jenkins QA-Review: Adam Stone <astone@instructure.com> Product-Review: Simon Williams <simon@instructure.com>
This commit is contained in:
parent
2ad51bc3c1
commit
83cd3adfc6
|
@ -57,14 +57,27 @@ module GoogleDocs
|
|||
access_token = retrieve_access_token
|
||||
entry = fetch_list(access_token).entries.map { |e| GoogleDocs::Entry.new(e) }.find { |e| e.document_id == document_id }
|
||||
if entry
|
||||
response = access_token.get(entry.download_url)
|
||||
response = access_token.get(response['Location']) if response.is_a?(Net::HTTPFound)
|
||||
response = fetch_entry(access_token, entry)
|
||||
|
||||
# some new google spreadsheets will not download as plain 'xls', so
|
||||
# retry the download as 'xlsx'
|
||||
if response.is_a?(Net::HTTPBadRequest) && entry.extension == 'xls'
|
||||
entry.reset_extension_as_xlsx
|
||||
response = fetch_entry(access_token, entry)
|
||||
end
|
||||
|
||||
[response, entry.display_name, entry.extension]
|
||||
else
|
||||
[nil, nil, nil]
|
||||
end
|
||||
end
|
||||
|
||||
def fetch_entry(access_token, entry)
|
||||
response = access_token.get(entry.download_url)
|
||||
response = access_token.get(response['Location']) if response.is_a?(Net::HTTPFound)
|
||||
response
|
||||
end
|
||||
|
||||
def fetch_list(access_token)
|
||||
response = access_token.get('https://docs.google.com/feeds/documents/private/full')
|
||||
Atom::Feed.load_feed(response.body)
|
||||
|
|
|
@ -95,6 +95,10 @@ module GoogleDocs
|
|||
}
|
||||
end
|
||||
|
||||
def reset_extension_as_xlsx
|
||||
@extension = 'xlsx'
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
def set_document_id_from(entry)
|
||||
|
|
|
@ -240,6 +240,30 @@ describe GoogleDocs::Connection do
|
|||
doc_array = google_docs.download(doc_id)
|
||||
doc_array.should == [nil, nil, nil]
|
||||
end
|
||||
|
||||
it "attempts as 'xlsx' if 'xls' fails" do
|
||||
doc_id = 'spreadsheet:0AiN8C_VHrPxkdEF6YmQyc3p2Qm02ODhJWGJnUmJYY2c'
|
||||
src = 'https://docs.google.com/feeds/download/spreadsheets/Export?key=0AiN8C_VHrPxkdEF6YmQyc3p2Qm02ODhJWGJnUmJYY2c'
|
||||
xls_src = [src, "exportFormat=xls", "format=xls"].join('&')
|
||||
xlsx_src = [src, "exportFormat=xlsx", "format=xlsx"].join('&')
|
||||
|
||||
access_token = mock_access_token
|
||||
|
||||
document_response = mock()
|
||||
access_token.expects(:get).with(xlsx_src).returns(document_response)
|
||||
bad_req = Net::HTTPBadRequest.new(1.0, 400, "Bad Request")
|
||||
access_token.expects(:get).with(xls_src).returns(bad_req)
|
||||
|
||||
fetch_list_response = mock()
|
||||
fetch_list_response.expects(:body).returns(xml_doc_list_many)
|
||||
access_token.expects(:get).with(xml_schema_id).returns(fetch_list_response)
|
||||
|
||||
google_docs = GoogleDocs::Connection.new(token, secret)
|
||||
doc_array = google_docs.download(doc_id)
|
||||
doc_array[0].should == document_response
|
||||
doc_array[1].should == 'Sprint Teams'
|
||||
doc_array[2].should == 'xlsx'
|
||||
end
|
||||
end
|
||||
|
||||
describe "#create_doc" do
|
||||
|
|
|
@ -50,6 +50,13 @@ describe GoogleDocs::Entry do
|
|||
entry = GoogleDocs::Entry.new(entry_feed)
|
||||
entry.extension.should == "doc"
|
||||
end
|
||||
|
||||
it "can be reset/forced to 'xlsx'" do
|
||||
entry = GoogleDocs::Entry.new(entry_feed)
|
||||
entry.extension.should == "doc"
|
||||
entry.reset_extension_as_xlsx
|
||||
entry.extension.should == "xlsx"
|
||||
end
|
||||
end
|
||||
|
||||
describe '#download_url' do
|
||||
|
|
Loading…
Reference in New Issue