Skip to content

Commit 3a30ff9

Browse files
committed
fix: update percentageDifference to be nullable in comparison logic and related components
1 parent 2460eb6 commit 3a30ff9

8 files changed

Lines changed: 162 additions & 34 deletions

File tree

app/api/compare/route.ts

Lines changed: 27 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -77,12 +77,12 @@ function calculateWinner(users: ComparedUserResult[]): {
7777
winner?: {
7878
username: string;
7979
finalScoreDifference: number;
80-
percentageDifference: number;
80+
percentageDifference: number | null;
8181
};
8282
languageWinner?: {
8383
username: string;
8484
finalScoreDifference: number;
85-
percentageDifference: number;
85+
percentageDifference: number | null;
8686
selectedLanguages: string[];
8787
};
8888
} {
@@ -95,28 +95,29 @@ function calculateWinner(users: ComparedUserResult[]): {
9595
const overallLoser =
9696
overallWinner.username === userA.username ? userB : userA;
9797
const overallDifference = Math.abs(userA.finalScore - userB.finalScore);
98-
const overallPercentage =
99-
overallLoser.finalScore > 0
100-
? (overallDifference / overallLoser.finalScore) * 100
101-
: 0;
98+
const overallPercentage = calculatePercentageDifference(
99+
overallDifference,
100+
overallLoser.finalScore,
101+
);
102102

103103
const result: {
104104
winner: {
105105
username: string;
106106
finalScoreDifference: number;
107-
percentageDifference: number;
107+
percentageDifference: number | null;
108108
};
109109
languageWinner?: {
110110
username: string;
111111
finalScoreDifference: number;
112-
percentageDifference: number;
112+
percentageDifference: number | null;
113113
selectedLanguages: string[];
114114
};
115115
} = {
116116
winner: {
117117
username: overallWinner.username,
118118
finalScoreDifference: Math.round(overallDifference),
119-
percentageDifference: Math.round(overallPercentage),
119+
percentageDifference:
120+
overallPercentage === null ? null : Math.round(overallPercentage),
120121
},
121122
};
122123

@@ -132,22 +133,34 @@ function calculateWinner(users: ComparedUserResult[]): {
132133
const languageDifference = Math.abs(
133134
winnerLanguageScores.finalScore - loserLanguageScores.finalScore,
134135
);
135-
const languagePercentage =
136-
loserLanguageScores.finalScore > 0
137-
? (languageDifference / loserLanguageScores.finalScore) * 100
138-
: 0;
136+
const languagePercentage = calculatePercentageDifference(
137+
languageDifference,
138+
loserLanguageScores.finalScore,
139+
);
139140

140141
result.languageWinner = {
141142
username: languageWinner.username,
142143
finalScoreDifference: Math.round(languageDifference),
143-
percentageDifference: Math.round(languagePercentage),
144+
percentageDifference:
145+
languagePercentage === null ? null : Math.round(languagePercentage),
144146
selectedLanguages: winnerLanguageScores.selectedLanguages,
145147
};
146148
}
147149

148150
return result;
149151
}
150152

153+
function calculatePercentageDifference(
154+
difference: number,
155+
baseline: number,
156+
): number | null {
157+
if (baseline <= 0) {
158+
return difference > 0 ? null : 0;
159+
}
160+
161+
return (difference / baseline) * 100;
162+
}
163+
151164
function createComparisonInsights(
152165
users: ComparedUserResult[],
153166
locale: Locale,

components/home-page-client.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ type ComparisonData = {
2626
languageWinner?: {
2727
username: string;
2828
finalScoreDifference: number;
29-
percentageDifference: number;
29+
percentageDifference: number | null;
3030
selectedLanguages: string[];
3131
};
3232
insights?: CompareInsights;

components/result-dashboard.tsx

Lines changed: 36 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ type Props = {
2121
languageWinner?: {
2222
username: string;
2323
finalScoreDifference: number;
24-
percentageDifference: number;
24+
percentageDifference: number | null;
2525
selectedLanguages: string[];
2626
};
2727
insights?: CompareInsights;
@@ -42,6 +42,17 @@ function getGithubProfileUrl(username: string): string {
4242
return `https://github.com/${username}`;
4343
}
4444

45+
function getPercentageDifference(
46+
winnerScore: number,
47+
loserScore: number,
48+
): number | null {
49+
if (loserScore <= 0) {
50+
return winnerScore > loserScore ? null : 0;
51+
}
52+
53+
return Math.round(((winnerScore - loserScore) / loserScore) * 100);
54+
}
55+
4556
export function ResultDashboard({
4657
user1,
4758
user2,
@@ -88,13 +99,20 @@ export function ResultDashboard({
8899
: winnerFromScores;
89100
const loserUser =
90101
overallWinnerUser === user1 ? user2 : overallWinnerUser === user2 ? user1 : null;
91-
const winnerDiffPct =
92-
overallWinnerUser && loserUser && loserUser.finalScore > 0
93-
? Math.round(
94-
((overallWinnerUser.finalScore - loserUser.finalScore) / loserUser.finalScore) *
95-
100,
96-
)
102+
const winnerDiffPct = winner
103+
? winner.percentageDifference
104+
: overallWinnerUser && loserUser
105+
? getPercentageDifference(overallWinnerUser.finalScore, loserUser.finalScore)
97106
: 0;
107+
const winnerDiffPoints =
108+
winner?.finalScoreDifference ??
109+
(overallWinnerUser && loserUser
110+
? Math.round(Math.abs(overallWinnerUser.finalScore - loserUser.finalScore))
111+
: 0);
112+
const winnerLeadValue =
113+
winnerDiffPct === null
114+
? t("results.pointsLead", { points: winnerDiffPoints })
115+
: `${winnerDiffPct}%`;
98116

99117
const handleCopy = async () => {
100118
try {
@@ -210,7 +228,7 @@ export function ResultDashboard({
210228
);
211229
};
212230

213-
const renderSignalStats = (user: UserResult) => {
231+
const renderSignalStats = (user: UserResult, idx: number) => {
214232
if (!user.signals) return null;
215233

216234
const signalEntries: Array<{ label: string; value: string | number }> = [
@@ -258,7 +276,7 @@ export function ResultDashboard({
258276
];
259277

260278
return (
261-
<Card key={`signal-${user.username}`}>
279+
<Card key={`signal-${user.username}-${idx}`}>
262280
<CardHeader className="pb-3">
263281
<CardTitle className="flex items-center gap-2 text-base">
264282
<Avatar
@@ -320,7 +338,7 @@ export function ResultDashboard({
320338
</div>
321339
<div className="text-right">
322340
<p className="text-sm text-muted-foreground">{t("banner.leadby")}</p>
323-
<p className="text-2xl font-bold text-primary">{winnerDiffPct}%</p>
341+
<p className="text-2xl font-bold text-primary">{winnerLeadValue}</p>
324342
</div>
325343
</>
326344
) : (
@@ -377,7 +395,12 @@ export function ResultDashboard({
377395
{t("language.focus")}: {languageWinner.selectedLanguages.join(", ")}
378396
</p>
379397
<p className="mt-1 text-xs text-muted-foreground">
380-
{t("banner.leadby")} {languageWinner.percentageDifference}%
398+
{t("banner.leadby")}{" "}
399+
{languageWinner.percentageDifference === null
400+
? t("results.pointsLead", {
401+
points: languageWinner.finalScoreDifference,
402+
})
403+
: `${languageWinner.percentageDifference}%`}
381404
</p>
382405
<p className="mt-2 text-xs text-muted-foreground">{t("language.winnerNote")}</p>
383406
</CardContent>
@@ -449,8 +472,8 @@ export function ResultDashboard({
449472
{t("signals.title")}
450473
</summary>
451474
<div className="mt-4 grid gap-4 lg:grid-cols-2">
452-
{renderSignalStats(user1)}
453-
{renderSignalStats(user2)}
475+
{renderSignalStats(user1, 1)}
476+
{renderSignalStats(user2, 2)}
454477
</div>
455478
</details>
456479

components/top-list.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -206,8 +206,8 @@ export function TopList({ userResults, selectedLanguages = [] }: Props) {
206206

207207
return (
208208
<div className="grid gap-6 md:grid-cols-2">
209-
{userResults.map((user) => (
210-
<Card key={`top-${user.username}`}>
209+
{userResults.map((user, idx) => (
210+
<Card key={`top-${user.username}-${idx}`}>
211211
<CardHeader>
212212
<CardTitle className="text-lg">
213213
{t("topwork.titleForUser", { username: user.name || user.username })}

locales/ar.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,7 @@
170170
"results.copied": "تم النسخ!",
171171
"results.copy": "نسخ النتيجة",
172172
"results.copyAria": "نسخ نتائج المقارنة إلى الحافظة",
173+
"results.pointsLead": "+{points} نقطة",
173174
"score.rawLabel": "الدرجة الخام: {value}",
174175
"results.scoreVersion": "إصدار التقييم",
175176
"section.language": "النتيجة اللغوية",

locales/en.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,7 @@
170170
"results.copied": "Copied!",
171171
"results.copy": "Copy Result",
172172
"results.copyAria": "Copy comparison results to clipboard",
173+
"results.pointsLead": "+{points} points",
173174
"score.rawLabel": "Raw: {value}",
174175
"results.scoreVersion": "Score version",
175176
"section.language": "Language-focused Score",

test/api/compare.route.test.ts

Lines changed: 92 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,26 @@ function makeUser(login: string, name: string) {
5151
};
5252
}
5353

54-
function makeScore(finalScore: number) {
54+
function makeLanguageScores(finalScore: number) {
55+
return {
56+
selectedLanguages: ["TypeScript"],
57+
repoScore: 0,
58+
prScore: 0,
59+
contributionScore: 0,
60+
finalScore,
61+
normalizedRepoScore: 0,
62+
normalizedPRScore: 0,
63+
normalizedContributionScore: 0,
64+
normalizedFinalScore: 0,
65+
topRepos: [],
66+
topPullRequests: [],
67+
};
68+
}
69+
70+
function makeScore(
71+
finalScore: number,
72+
languageScores?: ReturnType<typeof makeLanguageScores>,
73+
) {
5574
return {
5675
repoScore: 10,
5776
prScore: 20,
@@ -64,7 +83,7 @@ function makeScore(finalScore: number) {
6483
topRepos: [],
6584
topPullRequests: [],
6685
topCommunityContributions: [],
67-
languageScores: undefined,
86+
languageScores,
6887
signals: {
6988
reposAnalyzed: 1,
7089
pullRequestsAnalyzed: 1,
@@ -188,6 +207,77 @@ describe("GET /api/compare", () => {
188207
expect(body.winner?.username).toBe("user-a");
189208
});
190209

210+
test("returns null percentage when the losing final score is zero", async () => {
211+
mocks.getUserData.mockResolvedValueOnce({
212+
data: makeUser("user-a", "User A"),
213+
metrics: { duration: 0, errors: [] },
214+
});
215+
mocks.getUserData.mockResolvedValueOnce({
216+
data: makeUser("user-b", "User B"),
217+
metrics: { duration: 0, errors: [] },
218+
});
219+
220+
mocks.calculateUserScore.mockReturnValueOnce(makeScore(20));
221+
mocks.calculateUserScore.mockReturnValueOnce(makeScore(0));
222+
223+
const response = await GET(
224+
makeRequest({
225+
username: ["user-a", "user-b"],
226+
}),
227+
);
228+
const body = (await response.json()) as {
229+
winner?: {
230+
username: string;
231+
finalScoreDifference: number;
232+
percentageDifference: number | null;
233+
};
234+
};
235+
236+
expect(response.status).toBe(200);
237+
expect(body.winner).toEqual({
238+
username: "user-a",
239+
finalScoreDifference: 20,
240+
percentageDifference: null,
241+
});
242+
});
243+
244+
test("returns null language winner percentage when language loser score is zero", async () => {
245+
mocks.getUserData.mockResolvedValueOnce({
246+
data: makeUser("user-a", "User A"),
247+
metrics: { duration: 0, errors: [] },
248+
});
249+
mocks.getUserData.mockResolvedValueOnce({
250+
data: makeUser("user-b", "User B"),
251+
metrics: { duration: 0, errors: [] },
252+
});
253+
254+
mocks.calculateUserScore.mockReturnValueOnce(makeScore(20, makeLanguageScores(12)));
255+
mocks.calculateUserScore.mockReturnValueOnce(makeScore(10, makeLanguageScores(0)));
256+
257+
const response = await GET(
258+
makeRequest({
259+
username: ["user-a", "user-b"],
260+
selectedLanguage: "TypeScript",
261+
}),
262+
);
263+
const body = (await response.json()) as {
264+
languageWinner?: {
265+
username: string;
266+
finalScoreDifference: number;
267+
percentageDifference: number | null;
268+
selectedLanguages: string[];
269+
};
270+
};
271+
272+
expect(response.status).toBe(200);
273+
expect(body.languageWinner).toEqual({
274+
username: "user-a",
275+
finalScoreDifference: 12,
276+
percentageDifference: null,
277+
selectedLanguages: ["TypeScript"],
278+
});
279+
});
280+
191281
test("returns targeted username for not-found errors", async () => {
192282
mocks.getUserData.mockRejectedValueOnce(new Error("User not found"));
193283

types/api-response.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { UserResult } from "./user-result";
33
export type CompareWinner = {
44
username: string;
55
finalScoreDifference: number;
6-
percentageDifference: number;
6+
percentageDifference: number | null;
77
};
88

99
export type SafeApiError = {
@@ -48,7 +48,7 @@ export type ApiResponse = {
4848
languageWinner?: {
4949
username: string;
5050
finalScoreDifference: number;
51-
percentageDifference: number;
51+
percentageDifference: number | null;
5252
selectedLanguages: string[];
5353
};
5454
insights?: CompareInsights;

0 commit comments

Comments
 (0)