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

feat: Initial implementation of app #1

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

xitij2000
Copy link
Member

@xitij2000 xitij2000 commented Jan 15, 2025

Implements a UI using next and nextui that allows browsing the list of repos user has access to and the contributors for a repo.

Implements openedx/wg-coordination#148

Implements a UI using next and nextui that allows browsing the list of repos user has access to and the contributors for a repo.
Copy link
Member

@tecoholic tecoholic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@xitij2000 Hi, I tested this locally. I have a few suggestions. Summarising them below

  1. I couldn't browse any of the openedx repos as there were other repos in the first 30 repos that the API pulled. Switching from user repo listing to org repo listing, with pagination will make is easier to get just the openedx ones.
  2. The collaborators API seems to only work when the user already has PUSH rights to the repo. So, it wouldn't pull any information for a casual community user like me.
  3. (Optional) Because the collaborators API is not suitable, we can also consider ditching the Github API token requirement, if the alternate solution can make use of a public API.

<li>Save and see your changes instantly.</li>
</ol>
const [filter, setFilter] = React.useState<string>("");
const data = useAuthenticatedQuery(['repos-list'], "https://api.github.com/user/repos");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I couldn't browse any of the "openedx" repos. I believe List organization's repositories would be a better API to call, and it can be called without using the User Auth Token.

curl -L \
∙   -H "Accept: application/vnd.github+json" \
∙   -H "X-GitHub-Api-Version: 2022-11-28" \
∙ https://api.github.com/orgs/openedx/repos
[
  {
    "id": 4732120,
    "node_id": "MDEwOlJlcG9zaXRvcnk0NzMyMTIw",
    "name": "cs_comments_service",
    "full_name": "openedx/cs_comments_service",
    "private": false,
    "owner": {
      "login": "openedx",
      "id": 40179672,
      "node_id": "MDEyOk9yZ2FuaXphdGlvbjQwMTc5Njcy",
      "avatar_url": "https://avatars.githubusercontent.com/u/40179672?v=4",
      "gravatar_id": "",
      "url": "https://api.github.com/users/openedx",
      "html_url": "https://github.com/openedx",
      "followers_url": "https://api.github.com/users/openedx/followers",
      "following_url": "https://api.github.com/users/openedx/following{/other_user}",
      "gists_url": "https://api.github.com/users/openedx/gists{/gist_id}",
      "starred_url": "https://api.github.com/users/openedx/starred{/owner}{/repo}",
      "subscriptions_url": "https://api.github.com/users/openedx/subscriptions",
      "organizations_url": "https://api.github.com/users/openedx/orgs",
      "repos_url": "https://api.github.com/users/openedx/repos",
      "events_url": "https://api.github.com/users/openedx/events{/privacy}",
      "received_events_url": "https://api.github.com/users/openedx/received_events",
      "type": "Organization",
      "user_view_type": "public",
      "site_admin": false
    },
    "html_url": "https://github.com/openedx/cs_comments_service",
    "description": "server side of the comment service",
    "fork": false,
...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Secondly, I think the API by default only returns like 30 entries, so pagination logic should be implemented to fetch more repos.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense, I figured this what it's more generic.

As for pagination. I know that is missing; however, this is a prototype-level APP. I just wanted to show-case the idea before investing more time and effort into it.

</TableCell>
<TableCell>
<Link href={item.full_name}>
Browse Contributors
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Browse Contributors
Show Core Contributors

In Github terminology, "Contributors" are anyone who has a commit in a repo. So, this might create some confusion. Making it explicit can provide clarity.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is exactly why I say Contributors and not Core Contributors, because we are not filtering and some of the people listed here will be admins etc who have access but are not CCs.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we enable filtering by cross-referencing the global list of CCs from openedx/wg-coordination#149? (not necessarily as part of this very first POC but in general)

src/lib/utils.ts Outdated
import { useQuery } from "@tanstack/react-query";

export const getGithubToken = () => {
return localStorage.getItem("github-token");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This keeps throwing me errors when running via npm run dev. I think this is a known issue. Adding if (window.localStorage !== undefined) solved it for me. I also see other options like wrapping it in a useEffect being suggested.

const {org, repo} = React.use(params);
const data = useAuthenticatedQuery(
[org, repo, 'contributor'],
`https://api.github.com/repos/${org}/${repo}/collaborators?affiliation=all&permission=push`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This API only seem to work for repos to which the user already has rights. When I tried to browse an openedx repo, this is what I got

{
  "message": "Must have push access to view repository collaborators.",
  "documentation_url": "https://docs.github.com/rest/collaborators/collaborators#list-repository-collaborators",
  "status": "403"
}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, that is a definite limitation, however I don't know how to get around that other than having a proxy API that fetches the required data using a higher-privilege token.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@xitij2000 Thank you for explaining the reasons behind the decisions and limitations. I agree, finding a way to do this directly via API might be tricky. An option could be to have a Github actions commit a CSV or JSON file to the repo on a periodic interval. I have seen github actions used this way for collecting periodic data somewhere, but can't find a quick example.

@xitij2000
Copy link
Member Author

@itsjeyd Could you have a look at this app as well and do tell if you agree with the scope I picked for the prototype. If not I can add the missing features.

@itsjeyd
Copy link
Member

itsjeyd commented Jan 21, 2025

@xitij2000 Yep I can take a luck later this week.

Copy link
Member

@itsjeyd itsjeyd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@xitij2000 I had a look at the code and the comments above. Based on that, my understanding of the functionality that the app currently provides is this:

  1. User accesses the app and is asked to log in with a GitHub token.
  2. After they log in they are presented with a list of repos.
    • Right now this list is limited in two ways: pagination (fine to exclude for now) and repo access level of the user that's viewing the list.
  3. From that list they can navigate to pages for individual repos listing contributors.
    • The list of contributors for each repo isn't filtered by CC status; it includes other users with elevated access to the repo.

Both the list of repos and the lists of contributors on repo pages have a field that allows to search for a specific repo/contributor. (This is great!)

Let me know if I'm missing something here.


If my understanding is correct, here's what I would say in terms of functionality that the POC should include:

It would be good to find a way to remove the repo access level limitation mentioned above. Unless they are also coding CCs, OSPR managers generally won't have write access to any repos. And for the app to be helpful we'd need it to show an exhaustive list of all repos in the openedx organization.

As for filtering lists of contributors by CC status, that's something I think we'd ultimately want as well. But if there's no straightforward way to add that right now, it would be fine to exclude from the POC.

@@ -1,5 +1,12 @@
This is a [Next.js](https://nextjs.org) project bootstrapped with [`create-next-app`](https://nextjs.org/docs/app/api-reference/cli/create-next-app).

# Intro

This application is designed to browse the list of contributors to a repo and the list of repos a user has access to.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the list of repos a user has access to

Would that be the user accessing the app on GitHub pages?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes.

</TableCell>
<TableCell>
<Link href={item.full_name}>
Browse Contributors
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we enable filtering by cross-referencing the global list of CCs from openedx/wg-coordination#149? (not necessarily as part of this very first POC but in general)

@xitij2000
Copy link
Member Author

@itsjeyd If the proposal from openedx/wg-coordination#149 to reorganise teams is completed, then this might not even be needed since you could just go to the team core 'committers-REPO_NAME' to see committers from that repo.

@itsjeyd
Copy link
Member

itsjeyd commented Jan 24, 2025

@xitij2000 Sure. From the perspective of OSPR management, the fewer steps we need to take to pull up CC info for a repo, the better. As I mentioned in conversations leading up to and after the CC summit, it would be ideal if repo-specific CC info could be integrated into the Contributions board somehow. With a 'committers-REPO_NAME' team available for each repo, maybe introducing a field for the table view that simply links to that team would be an option?

CC @mphilbrick211

@xitij2000
Copy link
Member Author

@itsjeyd I think if there is already movement towards CC teams for each repo then that is the best way to discover CCs for repo and the global CC team could list non-coding CCs. That would make this particular app obsolete, so should we wait on that discussion before putting more time in this?

@itsjeyd
Copy link
Member

itsjeyd commented Jan 31, 2025

@xitij2000 Yep, that sounds good to me.

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.

3 participants