FilterService: Pass Date() instead of numbers to the map.

PokemonGoers / Catch-em-all

Now that we have tons of data about Pokemon (what they are, where they are, what’s their relationship, what they can transform into, which attacks they can perform, aso) we want to integrate it all into a comprehensive website. This website should contain sections about each Pokemon and its details. Additionally, the website should register the user’s location and tell the user how close is that the predicted pokemon to him/her. Additionally you will be incorporating the apps that were created by project B,C and D into the website. Your group will need to create automated builds and testing for this apps and use continuous integration to pull in new changes in the code repositories. Apps from projects B-D should be packaged and made available on NPM. Ideally when you completed these tasks the webapp component would integrate the apps by “requiring’ them. Here is a possible user story: when a user opens the website or the app the current location of the user will be shown. Additionally, the website/app will show automatically where the pokemons that are currently active are and where the pokemons that we predict to active in the nearest future (i.e. within half a day) will be located (all of this will be available from the app developed in project D). Hopefully, the website will be somewhat crowded by that data. Then, there needs to be a menu bar or something available (e.g. above the map or on the right side to it) that will list currently active or predicted pokemons. Clicking on one of them will make other pokemons on the map disappear, except of this clicked one. Separate web pages would allow the search and presentation of individual Pokemons and the information we gathered about them, including third party data (project A) and twitter analysis (project C)

9 stars 7 forks source link

FilterService: Pass Date() instead of numbers to the map. #136

Closed WoH closed 7 years ago

WoH commented 7 years ago

Title should say it all.

I don't care who implements this, but do it asap.

johartl commented 7 years ago

Why do you want to pass a date?

AlexanderLill commented 7 years ago

@johartl @WoH Date does not work...:

http://pokedata.c4e3f8c7.svc.dockerapp.io:65014/api/pokemon/sighting/ts/Sat,%2022%20Oct%202016%2014:22:59%20GMT/range/-5183998m 500 (Internal Server Error)

They say in the API documentation that they want "anything that works for new Date()...

WoH commented 7 years ago

Poke-Map-1 expects a Date. I don't care how they call the PokeData API and it's irrelevant.

AlexanderLill commented 7 years ago

Which map do we currently use in develop? I tried it with a date, it doesnt work.

See #137 for the PR

Btw, shouldnt both Map teams expect the same format?

johartl commented 7 years ago

Why would we use a Date?? There are so many things wrong about this.

https://github.com/PokemonGoers/Catch-em-all/issues/24 clearly states that we are using time in seconds as parameters. This API hasn't changed for over a month and both teams agreed to adhere to this interface. If we start passing Dates now this will break things for the PokeMap-2 team.
If we use a Date this means that sightingsSince and predictionsUntilwill be an absolute point in time. As time passes by, the predictionsUntil Date will eventually lie in the past and no predictions are shown anymore. On the contrary, the time interval between sightingsSince and the current point in time will increase and we will no longer show sightings of "the last 5 minutes" but actually a lot more than that. This must be extremely frustrating for the user. Our filter tab is already using relative times (1 week, 5 mintutes, and so on) and the PokeMap interface should reflect exactly the same logic. The only way around this would be to re-apply the filter every now and then which will cause unnecessary requests and reloads of the map.

So if passing numbers to the map is causing problems, then this is an issue the @PokemonGoers/pokemap-1 team should take care of.

WoH commented 7 years ago

It does not matter if time passes on in the ap or on the map. I talked to them and we agreed to use Date(). My bad, I didn't update the closed Issue where it is specified.

johartl commented 7 years ago

Why wouldn't it matter? The PokeMap should just do the transformation on their own. This prevents all mentioned problems and is basically just one line of code on their side.

WoH commented 7 years ago

It's literally just the same amount of code for us aswell. And we don't have to pass numbers but open a useful interface. And your "solution" does in fact not solve any issues, it just moves the complexity to the map team.

And a Date() is the most useful method to define time in JS. I'd rather have a useful interface for anyone to use than numbers.

sacdallago commented 7 years ago

@WoH I agree with @johartl on this one. I understand thought why you want to represent Dates as Dates. Can't you internally define it as Date but pass it on as timeInMs? Date objects are hard to handle and pass around, that's why most of the time they are downcast to UNIX time stamps. They (map teams) need to construct a Date from a timestamp, no biggie. If they had to read a Date object they'd have to do basically the same.

WoH commented 7 years ago

@sacdallago @johartl I took over when no one wanted to take care of that. I agreed to use Date(). Poke-Map-1 implemented the Map accordingly. I didn't communicate that properly, that was my bad. If we want to use numbers to represent the Time in seconds from now, it would be fair to implement the change and submit a PR over there or ask them very nicely to change their interface. Feel free to do that.

gqinami commented 7 years ago

We can do that, it is ok. Just let us know if this is your final decision, so we change from Date() to time in seconds

johartl commented 7 years ago

@gqinami

We can do that, it is ok. Just let us know if this is your final decision, so we change from Date() to time in seconds

Yes please stick to the original interface definition from #24 using the relative time in seconds.

johartl commented 7 years ago

Fixed by https://github.com/PokemonGoers/PokeMap-1/pull/41

sacdallago commented 7 years ago

Good.

AlexanderLill commented 7 years ago

I agree :)

On Oct 21, 2016 23:52, "Jochen Hartl" notifications@github.com wrote:

Why wouldn't it matter? The PokeMap should just do the transformation on their own. This prevents all mentioned problems and is basically just one line of code on their side.

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub https://github.com/PokemonGoers/Catch-em-all/issues/136#issuecomment-255474402, or mute the thread https://github.com/notifications/unsubscribe-auth/AKkaObQQ7_RaK4W3boLQlePeJ1jgAMsqks5q2TQ1gaJpZM4KdCqQ .