This repository has been archived by the owner on Feb 5, 2024. It is now read-only.
Fix determination of worksheet ID for new sheets #48
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Tabletop.loadSheets uses this logic to determine each worksheet's ID from the worksheets feed (https://spreadsheets.google.com/feeds/worksheets/{{SPREADSHEET_KEY}}/public/basic?alt=json):
For old sheets,
data.feed.entry.links
has 4 entries:and the code correctly extracts
od6
from thehref
property of the lastlink
element.For new sheets, however,
data.feed.entry.links
has 5 entries:So, the code will incorrectly grab
csv
from the end of the fourth link element, instead ofod6
from the end of the last link element.While this method of extracting the worksheet ID seems to be too brittle in general and might be worth reconsidering, we can make the existing method work for both old and new sheets by inspecting the last link element instead of the fourth:
Without this fix, we get the error "Cannot read property 'title' of undefined" in Tabletop.Model as a result of no data being retrieved from an incorrectly constructed sheet list feed URL (https://spreadsheets.google.com/feeds/list/{{SPREADSHEET_KEY}}/csv/public/values?alt=json).
Note that this pull request also contains the change for #46