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

Feature team stats #3182

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
144 changes: 144 additions & 0 deletions api/team.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,144 @@
import { renderStatsCard } from "../src/cards/stats-card.js";
import { blacklist } from "../src/common/blacklist.js";
import {
clampValue,
CONSTANTS,
parseArray,
parseBoolean,
renderError,
} from "../src/common/utils.js";
import { fetchStats } from "../src/fetchers/stats-fetcher.js";
import { isLocaleAvailable } from "../src/translations.js";
import { rank_to_level } from "../src/constant.js";

export default async (req, res) => {
const {
usernames,
hide,
hide_title,
hide_border,
card_width,
hide_rank,
show_icons,
include_all_commits,
line_height,
title_color,
ring_color,
icon_color,
text_color,
text_bold,
bg_color,
theme,
cache_seconds,
exclude_repo,
custom_title,
locale,
disable_animations,
border_radius,
number_format,
border_color,
rank_icon,
show,
team_name,
} = req.query;
res.setHeader("Content-Type", "image/svg+xml");

const username_list = usernames.split(",");

for (const username of username_list) {
if (blacklist.includes(username)) {
return res.send(renderError("Something went wrong"));
}
}
if (locale && !isLocaleAvailable(locale)) {
return res.send(renderError("Something went wrong", "Language not found"));
}

const team_stats = {
name: "",
totalPRs: 0,
totalPRsMerged: 0,
mergedPRsPercentage: 0,
totalReviews: 0,
totalCommits: 0,
totalIssues: 0,
totalStars: 0,
totalDiscussionsStarted: 0,
totalDiscussionsAnswered: 0,
contributedTo: 0,
rank: { level: "C", percentile: 0 },
};

try {
for (let username of username_list) {
const stats = await fetchStats(
username,
parseBoolean(include_all_commits),
parseArray(exclude_repo),
);
team_stats.totalPRs += stats.totalPRs;
team_stats.totalPRsMerged += stats.totalPRsMerged;
team_stats.mergedPRsPercentage += stats.mergedPRsPercentage;
team_stats.totalReviews += stats.totalReviews;
team_stats.totalCommits += stats.totalCommits;
team_stats.totalIssues += stats.totalIssues;
team_stats.totalStars += stats.totalStars;
team_stats.totalDiscussionsStarted += stats.totalDiscussionsStarted;
team_stats.totalDiscussionsAnswered += stats.totalDiscussionsAnswered;
team_stats.contributedTo += stats.contributedTo;
team_stats.rank.percentile += stats.rank.percentile;
}
Comment on lines +73 to +90
Copy link
Collaborator

@qwerty541 qwerty541 Sep 14, 2023

Choose a reason for hiding this comment

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

@pursonchen I think that the following lines of code should be totally reworked if we are going to merge this feature someday. I expect to have only one GraphQL request which will fetch required data about team members. Calling fetchStats() function inside loop is an error since it decrease performace and cause hitting upstream GitHub API rate limits. Algorithm complexity should be improved from $O(n)$ to $O(1)$ as previously noticed @rickstaa.

I also do not see much sense in showing sum stats of n people and their average rank. Is showing rank useful in case of team card? I do not think so since our median and weight values inside rank calculation algorithm (see https://github.com/anuraghazra/github-readme-stats/blob/master/src/calculateRank.js) are intended specially for users. I think it is a place for discussion.

Remember that you can implement anything and deploy it on your own Vercel instance.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@qwerty541, excellent point! It seems that with GraphQL v4, data for multiple users can now be fetched without using more GraphQL points:

{
  user1: user(login: "rickstaa") {
    ...UserFragment
  }
  user2: user(login: "qwerty541") {
    ...UserFragment
  }
  user3: user(login: "pursonchen") {
    ...UserFragment
  }
  rateLimit {
    limit
    cost
    remaining
    resetAt
  }
}

fragment UserFragment on User {
  login
  name
  contributionsCollection {
    totalCommitContributions
    totalPullRequestReviewContributions
  }
  repositoriesContributedTo(
    first: 1
    contributionTypes: [COMMIT, ISSUE, PULL_REQUEST, REPOSITORY]
  ) {
    totalCount
  }
  pullRequests(first: 1) {
    totalCount
  }
  mergedPullRequests: pullRequests(states: MERGED) {
    totalCount
  }
  openIssues: issues(states: OPEN) {
    totalCount
  }
  closedIssues: issues(states: CLOSED) {
    totalCount
  }
  followers {
    totalCount
  }
  repositoryDiscussions {
    totalCount
  }
  repositoryDiscussionComments(onlyAnswers: true) {
    totalCount
  }
}

I was surprised to learn about that feature and that it only uses 1 GraphQL point. Thanks for bringing it to my attention. As a result, making the addition of this card possible without having to wait for the GitHub action to be released.

However, creating a separate team data fetcher is essential so that the codebase doesn't get cluttered. Furthermore, we have to discuss which stats would be interesting to show for teams. I think the following stats are not helpful when looking at teams:

  • Contributed to (last year).
  • Total discussions started.
  • Total discussion answered.
  • PR reviews.

team_stats.name = `${team_name ? team_name : username_list[0]} team `;

team_stats.rank.level = rank_to_level(
team_stats.rank.percentile / 100 / username_list.length,
);

let cacheSeconds = clampValue(
parseInt(cache_seconds || CONSTANTS.FOUR_HOURS, 10),
CONSTANTS.FOUR_HOURS,
CONSTANTS.ONE_DAY,
);
cacheSeconds = process.env.CACHE_SECONDS
? parseInt(process.env.CACHE_SECONDS, 10) || cacheSeconds
: cacheSeconds;

res.setHeader(
"Cache-Control",
`max-age=${
cacheSeconds / 2
}, s-maxage=${cacheSeconds}, stale-while-revalidate=${CONSTANTS.ONE_DAY}`,
);

return res.send(
renderStatsCard(team_stats, {
hide: parseArray(hide),
show_icons: parseBoolean(show_icons),
hide_title: parseBoolean(hide_title),
hide_border: parseBoolean(hide_border),
card_width: parseInt(card_width, 10),
hide_rank: parseBoolean(hide_rank),
include_all_commits: parseBoolean(include_all_commits),
line_height,
title_color,
ring_color,
icon_color,
text_color,
text_bold: parseBoolean(text_bold),
bg_color,
theme,
custom_title,
border_radius,
border_color,
number_format,
locale: locale ? locale.toLowerCase() : null,
disable_animations: parseBoolean(disable_animations),
rank_icon,
show: parseArray(show),
}),
);
} catch (err) {
res.setHeader("Cache-Control", `no-cache, no-store, must-revalidate`); // Don't cache error responses.
return res.send(renderError(err.message, err.secondaryMessage));
}
};
2 changes: 2 additions & 0 deletions express.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import repoCard from "./api/pin.js";
import langCard from "./api/top-langs.js";
import wakatimeCard from "./api/wakatime.js";
import gistCard from "./api/gist.js";
import teamCard from "./api/team.js";
import express from "express";
import dotenv from "dotenv";

Expand All @@ -15,3 +16,4 @@ app.get("/pin", repoCard);
app.get("/top-langs", langCard);
app.get("/wakatime", wakatimeCard);
app.get("/gist", gistCard);
app.get("/team", teamCard);
Loading