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

Queries and routes for Projects via GraphQL #130

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

danvk
Copy link

@danvk danvk commented Jun 15, 2018

Hi @radekstepan, here's the work you requested in #129 (comment).

A couple things to note:

  • I've implemented this using GitHub's new GraphQL API.
  • The GraphQL API requires you to be logged in, even for queries on open source projects. You'll get unauthorized errors on projects until you log in.
  • Projects can contain either cards or issues, so you'll want to filter down to just the issues.
  • I haven't implemented any sort of pagination.
  • Projects due not have due dates like Milestones do.
  • There are two types of projects, org-level and repo-level. Org-level projects can contain issues from any repo in the organization. This means that burnchart's current approach of fetching all issues in a repo and then linking issue IDs to a milestone won't work. Fortunately GraphQL lets us do everything in a single query.
  • I've left some logging / debugging code in.

I added routes but they're more of a suggestion. I've verified that one of the queries in requests.js worked in the browser and verified all the GraphQL queries in the GraphQL API Explorer.

Let me know if this is enough for you to take over.

@radekstepan
Copy link
Owner

OK, I've started updating the branch code https://github.com/radekstepan/burnchart/compare/danvk-graphql-projects and there's too much duplication going on.

I will first refactor the code to handle issues regardless of whether they are attached to a milestone or a project so that's easier to add the GraphQL queries.

@danvk
Copy link
Author

danvk commented Jun 27, 2018

Great! Let me know if there's anything I can do to help.

@radekstepan
Copy link
Owner

Hey @danvk so I've tried to modernize the codebase to modern React/Redux and it was a massive undertaking. So... to get your code working, I just ripped out the old request logic and created a branch where all repos render their project/s and not milestone/s. I've tested with twbs/bootstrap repo and it's working for me.

Here's the branch: danvk-graphql-projects...projects

Would you be able to take it from here?

@danvk
Copy link
Author

danvk commented Jun 30, 2018

Nice @radekstepan, I can see burndown charts for my repo-based projects using your branch.

So what you're asking from me is to add back support for repos?

@danvk
Copy link
Author

danvk commented Jun 30, 2018

In particular, would you want to switch entirely to the GraphQL API for milestones, too?

@danvk
Copy link
Author

danvk commented Jul 1, 2018

Suggestion for what the routes should be:

  '/': 'repos',
  '/new/repo': 'addRepo',
  # preserve backwards-compatibility
  '/:owner/:name': 'milestones',
  '/:owner/:name/:milestone': 'chart',
  '/demo': 'demo'
  # match GitHub URLs
  '/:owner/:name/milestones': 'milestones',
  '/:owner/:name/milestone/:milestone': 'chart',
  '/:owner/:name/projects': 'projects',
  '/:owner/:name/project/:project': 'chart',
  '/orgs/:org/projects': 'projects',
  '/orgs/:org/project/:project': 'chart',

@radekstepan
Copy link
Owner

Hi @danvk I am glad it works for you! I like the URL structure and think we should use GraphQL for milestones too. Yes, it means needing to authenticate every time, but due to rate limiting I think any serious user would need to login anyway.

To use projects in master I'd like to see:

  1. Some tests added against the response returned by Github
  2. Pagination. Currently it's not implemented like you mentioned.

But, I need to do some work on master first to use more modern React/Redux and lay the groundwork for having both milestones and projects work side by side (perhaps fetch them all for each repo and show in one table?).

In the meantime, if you have time, I'd appreciate any help on the projects branch so that when it gets all integrated together, we can test it easily.

Thanks!

@danvk
Copy link
Author

danvk commented Jul 6, 2018

Sounds good. You can fetch two types of data in a single GraphQL call, so that milestones + projects should work well.

I've noticed that whenever I reload a page in the local dev setup, I have to log in to GitHub again. Is this something you've run into? Do you have a workaround? It would make iterating on the GraphQL features much faster.

@radekstepan
Copy link
Owner

That's perfect, I like having just one query to fetch both.

Yep... it's an issue I am aware of. The older version of Firebase would return a GitHub auth token in the response that the client could reuse on app reload/refresh, then they changed it to no longer allow that. I need to look into removing Firebase as a dependency and getting (and reusing) the token some other way.

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