WIP: Fetch contributions from GitHub API

[mateusabelli]

Closes #29

[netlify]

✅ Deploy Preview for parthmittal ready!

Name	Link
🔨 Latest commit	e0d9943
🔍 Latest deploy log	https://app.netlify.com/sites/parthmittal/deploys/641e2ec36988c40008bfa23a
😎 Deploy Preview	https://deploy-preview-44--parthmittal.netlify.app/
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site settings.

[mateusabelli]

Hello this is my initial commit to solve the issue, it is not complete but it gives something close to the desired output. At the moment this is a minimum code just to grasp the possibility and have a better understanding.

Here one of the objects it outputs from the array:

{
  "id": 1599432588,
  "organization": "mittal-parth",
  "logo": "https://github.com/mittal-parth.png",
  "repo": "personal-portfolio",
  "type": "pull-request",
  "status": "merged",
  "title": "Add footer credit notes",
  "link": "https://github.com/mittal-parth/personal-portfolio/pull/43",
  "number": "#43",
  "date": "2023-02-25T00:13:13Z",
  "linesAdded": "",
  "linesRemoved": ""
}

Somethings that I'd like to discuss:

• I am still trying to figure out the best way to extract the lines added and lines removed from the diff_url
• The code is getting bigger as the complexity grows and I would appreciate to hear some feedbacks for the initial code refactoring.
• I noticed that NPM is giving this error again while installing the dependencies. Since we need to use tokens, we would need to install an external library like dotenv and if I do it now, deployment might break on Netlify due to this error.

Click to expand.

npm ERR! code ERESOLVE
npm ERR! ERESOLVE could not resolve
npm ERR!
npm ERR! While resolving: react-lottie@1.2.3
npm ERR! Found: react@18.2.0
npm ERR! node_modules/react
npm ERR!   react@"^18.2.0" from the root project
npm ERR!   peer react@"^18.0.0" from framer-motion@7.3.5
npm ERR!   node_modules/framer-motion
npm ERR!     framer-motion@"^7.3.5" from the root project
npm ERR!   2 more (react-dom, react-icons)
npm ERR!
npm ERR! Could not resolve dependency:
npm ERR! peer react@"^0.14.7 || ^15.0.0 || ^16.0.0" from react-lottie@1.2.3
npm ERR! node_modules/react-lottie
npm ERR!   react-lottie@"^1.2.3" from the root project
npm ERR!
npm ERR! Conflicting peer dependency: react@16.14.0
npm ERR! node_modules/react
npm ERR!   peer react@"^0.14.7 || ^15.0.0 || ^16.0.0" from react-lottie@1.2.3
npm ERR!   node_modules/react-lottie
npm ERR!     react-lottie@"^1.2.3" from the root project
npm ERR!
npm ERR! Fix the upstream dependency conflict, or retry
npm ERR! this command with --force or --legacy-peer-deps
npm ERR! to accept an incorrect (and potentially broken) dependency resolution.
npm ERR!
npm ERR!
npm ERR! For a full report see:
npm ERR! F:\cache\npm-cache\_logs\2023-02-26T14_49_27_672Z-eresolve-report.txt

npm ERR! A complete log of this run can be found in:
npm ERR!     F:\cache\npm-cache\_logs\2023-02-26T14_49_27_672Z-debug-0.log

[mittal-parth]

Hi @mateusabelli
Thanks for the updates, always appreciate working with draft pull requests.

Looks good. Few things:

1. I am assuming you made a search request to all issues and PRs of the user because it hits the API only once. GitHub has separate endpoints for issues and pull requests. Are there some problems if we use those endpoints instead of checking startsWith?
2. I'll also give diff_url a thought, little later, slightly caught with things

I noticed that NPM is giving this error again while installing the dependencies.

About this, I came across this issue on another project of mine. Turns out, that react-lottie has issues with React 18. We then decided to use react-lottie-player. This causes no issues. However, it works slightly differently in terms of the syntax. It will then require a separate PR to refactor all the instances of lotties.

[mateusabelli]

Hello @mittal-parth thank you for the feedback!

1. I am assuming you made a search request to all issues and PRs of the user because it hits the API only once. GitHub has separate endpoints for issues and pull requests. Are there some problems if we use those endpoints instead of checking startsWith?

Yea that was it. I don't think there would be problems fetching from separated endpoints other than the extra request. I feel like it could make the code more readable if we opt to use different endpoints instead of separating them with startsWith. How do you feel about this?

GET https://api.github.com/search/issues?q=is:pr+author:<username>
GET https://api.github.com/search/issues?q=is:issue+author:<username>

2. I'll also give diff_url a thought, little later, slightly caught with things

Nice it's okay, I'll be looking into this throughout the week as well. It's something that caught me off guard as I only referenced a small PR with a single commit while studying the diff_url.

I noticed that NPM is giving this error again while installing the dependencies.

About this, I came across this issue on another project of mine. Turns out, that react-lottie has issues with React 18. We then decided to use react-lottie-player. This causes no issues. However, it works slightly differently in terms of the syntax. It will then require a separate PR to refactor all the instances of lotties.

That's interesting, if time allows me I'm going to study the react-lottie and react-lottie-player to see if I can help with the refactoring on a separate PR.

[mittal-parth]

GET https://api.github.com/search/issues?q=is:pr+author:<username>
GET https://api.github.com/search/issues?q=is:issue+author:<username>

Yes, this looks better!

[mateusabelli]

Hello @mittal-parth. With this update I've merged this branch with my second PR that fixes NPM and react-lottie it was also updated the OpenSource.jsx component to use the data from contributions.js in the constants folder. I'm attaching a couple screenshots to demonstrate the result.

Normal usage	API Usage Exceed/Other

There a few things I'd like to discuss.

The first one is that it was not necessary to install the dotenv like I mentioned before, Vite can handle environment variables by itself, however for the GH token it needed to be exposed to the app with VITE_ prefix, I'm not sure if this would be a problem, please let know what you think.

I've used the slice function to reduce the amount of contributions to only 12 to it doesn't clutter the UI but I'm open for suggestions if you'd like it removed or the limit number changed.

At the moment the data fetching is only getting the first 12 latest contributions without excluding your own repos or own organizations. The filter to switch between organizations is still not implemented yet. Please let me know how do you feel about it so far.

[mittal-parth]

Hi @mateusabelli!

Looks great!
I'll go ahead merge the other PR.

I don't think we should limit it to 12. The code restricting it shouldn't be the case. If they'd like, they could do it themselves. Rest is great

Vite environment variables shouldn't be an issue. We can probably add a section in the readme which specifies the usage. If they'd like to use Netlify, they can go ahead with putting the environment variable there, otherwise use dotenc.

[mateusabelli]

Hello @mittal-parth to exclude user owned repository was very simple, but for the user owned organizations it will be a bit more difficult because of how github tags the user depending on their relationship with the org/repo.

For example, I've seen this property author_association appear with the values:

• "OWNER" <- the one I've used as mentioned above
• "COLLABORATOR"
• "CONTRIBUTOR"
• "MEMBER"

There might be more, but with this alone it's tricky to determine if the organization is owned by the user or if the user is just a member of it.

I thought about a workaround that would be to have in the constants folder along side the contributions.js a json file "ignore.json" that could be used similar to a gitignore to exclude any organizations or projects that the user wouldnt want to show up.

Another thing that I'd like to discuss, is the fact that the GitHub API seems perform the search only within 1 year period. Any contributions older than a year won't show up, so contributions like zulip/zulip#19760 do not appear when fetching. Please let me know what do you think.

[mittal-parth]

Hi @mateusabelli!
I love how you have very detailed approaches and questions.

Which orgs to hide could be tricky because of a slightly abstract definition of an 'open source contribution'. I would argue that if I am an owner or a collaborator, it wouldn't be an open source contribution. But there are very large open source orgs and being an owner/collaborator there would still mean an open source contribution.

Probably, giving the user the choice would be the best way I believe (using the method you mentioned)

Another thing that I'd like to discuss, is the fact that the GitHub API seems perform the search only within 1 year period

Are you sure about this? I couldn't find this documented anywhere as such. Could it be pagination because the API returns the results paginated?

[mateusabelli]

Hello @mittal-parth Thanks!

With this update I've added the missing organization filter and also added the ignore feature.
I did a small refactoring and joined two conditions into one if statement, the way it works is to first parse the origin and check if the contribution is from the "OWNER" or if the organization name is contained inside the "ignore.json" file.

When one these conditions are met the contribution is skipped and won't show up.

pulls.items.forEach((pr) => {
  const {organization, repo, logoUrl} = parseOriginFromUrl(pr.url)
  const isIgnored = ignore.find((item) => (item.toLowerCase() === organization.toLowerCase()))

  if (pr.author_association === "OWNER" || isIgnored) {
    return
  }
  ...
}

Are you sure about this? I couldn't find this documented anywhere as such. Could it be pagination because the API returns the results paginated?

This was a wild guess because older contributions were not showing up, but I'm still looking up for documentation on this subject, I also think it could be some sort of pagination or maybe it needs another search query to allow deeper results.

Please let me know what do you think.