Skip to content
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

Ajay map updates #266

Merged
merged 9 commits into from
Nov 18, 2020
Merged

Ajay map updates #266

merged 9 commits into from
Nov 18, 2020

Conversation

Ajay-Vishwanath
Copy link
Collaborator

@Ajay-Vishwanath Ajay-Vishwanath commented Nov 16, 2020

Checklist

  • Add description
  • Reference the open issue that the pull request addresses
  • Pass code quality checks
    • spin up docker docker-compose up -d --build
    • enter api container docker-compose exec api /bin/bash
    • run api tests make validate
    • exit container ctrl/command+D or exit
    • enter web container docker-compose exec web /bin/sh
    • run front-end tests npm run test or npx jest
    • lint npm run lint-fix
    • exit container as above
  • Request code review
    • Please allow 36 hours from opening a pull request before merging a pull request- even if it has already received an approving review.
  • Address comments on code and resolve requested changes
  • Merge own code

Description

Issue: #261 #257 #260

This pull request:

  • Adds a loading indicator on the map when pulling the pollution data
  • Pushes user to the appropriate map date after uploading their data
  • Adds a 'Download Raw Data' button which almost gets us to download the data (more on that below).

Next Steps

In order to get to Map UI MVP - there are 4 steps developers can take right now

  1. Issue Device Name Saved to the Database #259: Add 'Device Type' as a column to the Collection Table, have the frontend include that in the formData upon upload, and then pull that for the map display.

  2. Complete issue Links to Open Session Files on the Map Page #260 - I've gotten the 'Download Raw Data' div in the ControlPanel component to bring us to the API endpoint which contains the relevant files - but can't key into currentCollection.collection_files[0].file even though I can see the file parameter in the currentCollection.collection_files[0] object. That code is found lines 53-57 of the ControlPanel index.tsx file.

  3. Issue Initial Error from Collection Date API Request #258 - 'cancel' is being logged to the console every time we call the getCollections function in the map component. I don't know if we just need to remove that console.log(error) or if there's something not ideal happening.

  4. Bug that appeared during demo - after we changed dates, the API call and pollution data parsing were not cancelled and the pins still appeared on dates where there were no collections.

For future work to get to Map UI MVP - as a team we need to have discussions about:

  1. Part of issue Improve UX on map loading #257 - specifically about the second checkbox - pushing the user to the map immediately after clicking save. I don't think this will be ideal with what's happening under the hood - the collections for the specific dates are called as soon as the component is mounted, and if we push to the map page without the newest collection being successfully saved yet, I can see it not coming up. I think there should be some loading indicator on the upload page after clicking save that we could design out.

  2. Issue Display and color code readings on the map #237 - developers can implement the 'air quality this session/today' display that appears on the wireframe once we figure out how we are going to calculate that. Currently we pull just collection pollution data one session at a time (was too slow and erroring out when we pulled all sessions from a day) - so we will have to think of something to get daily aggregate data and then processing that. Maybe upon save - the backend ends up parsing the files and saving an average AQ index per Collection instance, and then the frontend can average out the daily Collection AQ indexes after it retrieves the Collections set per day.

@TangoYankee
Copy link
Member

All the thoughts I have are already captured in the your next steps.

@Ajay-Vishwanath Ajay-Vishwanath merged commit f828f0c into master Nov 18, 2020
@Ajay-Vishwanath Ajay-Vishwanath deleted the Ajay-Map-Updates branch November 18, 2020 07:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants