-
Notifications
You must be signed in to change notification settings - Fork 2
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
Add /next-hour endpoint #57
Conversation
@@ -88,6 +111,14 @@ | |||
return res; | |||
} | |||
|
|||
if (url.pathname == '/next-hour') { | |||
for (const [key, value] of Object.entries(eventData)) { | |||
const upcomingEvents = value.eventSearch.edges.filter(isLessThanOneHourIntoFuture) |
Check notice
Code scanning / CodeQL
Semicolon insertion Note
the enclosing function
@@ -88,6 +111,14 @@ | |||
return res; | |||
} | |||
|
|||
if (url.pathname == '/next-hour') { | |||
for (const [key, value] of Object.entries(eventData)) { |
Check notice
Code scanning / CodeQL
Unused variable, import, function or class Note
Closing because it seems unnecessary to add an endpoint since we're hitting the API pretty infrequently and we can just do the filtering on the ruby/client side |
@ebanner I agree with your previous comment, but I do think that date-based filters would be a useful addition to the API. That said, I would recommend a slightly modified approach to what was included in this PR-- rather than a single endpoint that can be only for a particular use case (events within the next hour), what I'd prefer to see would look like a pair of query parameter filters:
With these:
This not only satisfies the original use case, but it also provides a pretty flexible utility filter that lets downstream consumers avoid doing at least some (if not most of) the date-based filtering themselves. |
In order to have 60 minute event reminders (TampaDevs/events-slack-push#2), this PR adds a
/next-hour
endpoint to show meetups happening in the next hour