search

alan-turing-institute / WimbledonPlanner

Project planning for REG
MIT License
0 stars 0 forks source link

Add GitHub emoji preferences table to web app #43

Closed edwardchalstrey1 closed 5 years ago

edwardchalstrey1 commented 5 years ago

@jack89roberts opening the PR now, but will refactor before merge ready

TODO:

  1. Add support for GitHub issues where the emojis are not on the top comment e.g. Learning Machines #428 :ballot_box_with_check:
  2. Investigate speeding up emoji extraction with a single GraphQL query for the list of issues
  3. Make table prettier/ more readable. :ballot_box_with_check:
  4. Remove team members with no availability (because they left) - currently only removes those with no emojis :ballot_box_with_check:
jack89roberts commented 5 years ago

Looking good and runs for me, thanks! Few quick thoughts:

edwardchalstrey1 commented 5 years ago

Looking good and runs for me, thanks! Few quick thoughts:

  • Could you add a white background colour to the table (at least the row/column headers), rather than it being transparent like it is now, so that when you scroll you don't see the content of the other cells behind the headers? You can stick background-color: white in the relevant CSS blocks. Might be nice to play with adding bold text for headers, and (centre?) justification of everything to make the table a bit prettier too.
  • Can all the functions that use/create an fc object in preferences_availability.py optionally be passed a previously created fc instance? E.g. def get_all_preferences_table(fc=None):, then if fc is None make a new one, else use the given one. Then in the app the same fc object can be used for everything (in the current version of app.py the harvest api ends up being queried for the data twice).
  • Could you add a link to the new /preferences end-point in the string returned by the home function in app.py (so it appears as an option on the "home page")?

Not sure where to get the fc object from in app.py where get_all_preferences_table() is called on line 80? It can now be passed one though: get_all_preferences_table(fc=fc)

Do you think we should include the following info alongside the project titles:

  1. GitHub issue number (or better still a link)
  2. The dates between which the mean resource requirement and person availability was calculated
jack89roberts commented 5 years ago

The fc object is hidden inside the Visualise instance - you should be able to pass in vis.fc (if you move the preference table stuff after vis is made).

Would be nice to add links to the projects - on the whiteboard I do that by putting the project names in a <a href=issue_url>project_name</a>.

Having a page title including the date range would be nice too 🙂

jack89roberts commented 5 years ago

I noticed in the query string we have users(first:20) which I assume means we're only getting the first 20 emojis back. That's probably safe for now but won't be future-proof as the team grows.

Could we look at either:

Or

In both cases would need to check how much the API will return in one query and how we can split a request across multiple queries if necessary.

jack89roberts commented 5 years ago

For this to be compatible with the dev/27-database branch (which I hope to make master soon) these changes will be needed:

edwardchalstrey1 commented 5 years ago

For this to be compatible with the dev/27-database branch (which I hope to make master soon) these changes will be needed:

  • from wimbledon.vis.Visualise import DataHandlers is replaced by from wimbledon import Wimbledon
  • fc names should be changed to wim
  • peopledf = 1 - fc.people_totals can be replaced with wim.people_free_capacity
  • fc.projects['GitHub'] is now wim.projects['github']
  • fc = DataHandlers.Forecast() should now be wim = Wimbledon(update_db=True, with_tracked_time=False)

Let me know when you have merged your branch with master, then I can rebase this branch on master and test it's working after making those edits