-
Notifications
You must be signed in to change notification settings - Fork 7
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
Apple Wallet: Implement support for the lastUpdated tag (#75) #86
Changes from all commits
c5cb32f
5ebe1f1
1f0fbb6
2c76b01
49a57ef
690de1a
3435901
6a7fb57
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -65,12 +65,33 @@ export default function handler(req: NextApiRequest, res: NextApiResponse) { | |
error: 'Internal Server Error: ' + latest_updates_result.error.message | ||
}) | ||
} else { | ||
// Return matching passes (serial numbers) and their modification time | ||
// Convert from ISO string to Date | ||
const latestUpdateDate: Date = new Date(latest_updates_result.data['time']) | ||
res.status(200).json({ | ||
serialNumbers: serialNumbers, | ||
lastUpdated: String(Math.round(latestUpdateDate.getTime() / 1000)) | ||
}) | ||
console.log('latestUpdateDate:', latestUpdateDate) | ||
|
||
if (!passesUpdatedSince) { | ||
// The passes on this device have not been updated previously, so return all passes. | ||
res.status(200).json({ | ||
serialNumbers: serialNumbers, | ||
lastUpdated: String(Math.round(latestUpdateDate.getTime() / 1000)) | ||
}) | ||
} else { | ||
// The passes on this device have been updated previously, so only return passes that | ||
// were updated before the most recent Nation3 update in the `latest_updates` database table. | ||
|
||
// Convert from epoch timestamp string to Date | ||
const passesUpdatedSinceDate: Date = new Date(Number(passesUpdatedSince) * 1000) | ||
console.log('passesUpdatedSinceDate:', passesUpdatedSinceDate) | ||
|
||
if (passesUpdatedSinceDate.getTime() < latestUpdateDate.getTime()) { | ||
res.status(200).json({ | ||
serialNumbers: serialNumbers, | ||
lastUpdated: String(Math.round(latestUpdateDate.getTime() / 1000)) | ||
}) | ||
} else { | ||
res.status(204).end() | ||
} | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll give the same feedback as ever, for me there are too many branches of code in here, and too many different return paths that make it tricky for the reader to follow what the code is doing. I would pull out some methods with names making it clear on the purpose and havea simple helper for the / 1000 and * 1000 code to avoid tripping up. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @johnmark13 Good feedback, thank you. Will fix 😀 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @johnmark13 Just to get the functional solution deployed, I'm merging this PR. And I added your refactoring suggestions to #76. I'm working on those now, in a separate branch/PR. |
||
} | ||
}) | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Up above you use
.single
and.limit(1)
I do not think you need both, single will blow up if there is not 1 result, and will only return 1 result.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@johnmark13 Yes, I agree this seems redundant. However, without including
.limit(1)
, it fails:I followed the code example in the documentation at https://supabase.com/docs/reference/javascript/single