Apple Wallet: Implement support for the lastUpdated tag (#75) #86
Conversation
When the passesUpdatedSince parameter is not included, expect all serial numbers for the device to be returned.
|
@aahna-ashina is attempting to deploy a commit to the Nation3 Team on Vercel. A member of the Team first needs to authorize it. |
aahna-ashina
changed the title
Codecov Report
@@ Coverage Diff @@
## main #86 +/- ##
=======================================
Coverage 80.69% 80.69%
=======================================
Files 5 5
Lines 202 202
Branches 22 22
=======================================
Hits 163 163
Misses 38 38
Partials 1 1 Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
…tedSince in the future
Expect HTTP 204 No Content when the passesUpdatedSince value is equal to or greather than the timestamp of the most recent update in the `latest_updates` database table.
| } else { | ||
| res.status(204).end() | ||
| } | ||
| } |
There was a problem hiding this comment.
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.
@johnmark13 Good feedback, thank you. Will fix 😀
There was a problem hiding this comment.
@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.
| @@ -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 | |||
There was a problem hiding this comment.
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.
@johnmark13 Yes, I agree this seems redundant. However, without including .limit(1), it fails:
'Results contain 12 rows, application/vnd.pgrst.object+json requires 1 row'
I followed the code example in the documentation at https://supabase.com/docs/reference/javascript/single
Updates code to match the Response Codes at https://developer.apple.com/documentation/walletpasses/get_the_list_of_updatable_passes
This prevents unnecessary downloads of unchanged data.
Dework Task
https://app.dework.xyz/nation3/passport-services-p?taskId=b479bb3a-cf2b-4322-b3fe-0eb720da8ad7
Related GitHub Issue
closes #75
How Has This Been Tested?
npm run cy:headless/api/pushLastUpdate) while observing Vercel logs, ensuring that no error message is received from Apple.Integration Tests
Are There Admin Tasks?