Get event results #165
Get event results #165
Conversation
|
I can also add a |
src/endpoints/getResults.ts
Outdated
| @@ -6,19 +6,35 @@ import { popSlashSource } from '../utils/parsing' | |||
| import { HLTVConfig } from '../config' | |||
| import { fetchPage, toArray, getMatchFormatAndMap } from '../utils/mappers' | |||
|
|
|||
| export const getResults = (config: HLTVConfig) => async ({ pages = 1 } = {}): Promise< | |||
| export const getResults = (config: HLTVConfig) => async ({ pages = 1, eventId = 0 } = {}): Promise< | |||
There was a problem hiding this comment.
Why is the default to 0 needed here?
There was a problem hiding this comment.
Yeah it's not... I'll remove it.
There was a problem hiding this comment.
Did you mean I should set it to undefined?
There was a problem hiding this comment.
I cheated and looked at how emha did it for teamId in his PR and made the same changes :)
I'm still new to typescript and didn't know how to remove the default value...
src/endpoints/getResults.ts
Outdated
| @@ -48,11 +64,21 @@ export const getResults = (config: HLTVConfig) => async ({ pages = 1 } = {}): Pr | |||
| } | |||
|
|
|||
| const event: Event = { | |||
| name: matchEl.find('.event-logo').attr('alt'), | |||
| id: Number(popSlashSource(matchEl.find('.event-logo'))!.split('.')[0]) | |||
| name: eventId ? eventName : matchEl.find('.event-logo').attr('alt'), | |||
There was a problem hiding this comment.
Why not just put the implementation of eventName here? Instead of:
let eventName: string
if(eventId) {
eventName = $('.eventname').text()
}
name: eventId ? eventName : matchEl.find('.event-logo').attr('alt')
just
name: eventId ? $('.eventname').text() : matchEl.find('.event-logo').attr('alt')
There was a problem hiding this comment.
I didn't want it inside the for loop. This way if you filter by eventid it is resolved only once.
There was a problem hiding this comment.
OK, so I was on my phone and missed, that I had it insode the for loop. I wanted to set it outside, but for the events it will only get one page, so setting it outside the loop is irrelevant :P
I did it as you suggested.
|
I've left some comments, but otherwise looks good! |
Simirally to filtering by
teamId, I added a parameter for events. I also changed how the Event data is filled and how the date-time is found, because the results page of events has a bit different layoutI also reset the
pagesparameter to 1 if theeventIdparameter is passed. That's because all the results for events are show on one page. For example: event=3047 has 220 results...Maybe they have a limit of 500 or something, but I didn't find any events that have that many results :)
This PR is related to issue #124.
I did a new branch from
master, so theteamIdparameter is not included... But it should not be difficult to merge both.