Skip to content

Commit dbf55b3

Browse files
refactor: address PR review feedback and improve code quality
Implemented all improvements suggested in PR #7 code review: - Fix hardcoded site URL by using centralized SITE_CONFIG - Improve related posts algorithm to use relative recency scoring - Document reading time regex limitations with comments - Add comprehensive tag validation with Unicode support - Enable previously skipped tests for Portuguese language and reading progress - Extract magic numbers to named constants for better maintainability - Add JSDoc comments to all exported utility functions All tests passing (42/42). Build successful.
1 parent d180f96 commit dbf55b3

6 files changed

Lines changed: 138 additions & 49 deletions

File tree

src/config/site.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ export const SITE_CONFIG = {
66
author: 'Juan Felipe Rivera González',
77
email: 'jjuanrivvera@gmail.com',
88
twitter: '@jjuanrivvera99',
9-
github: 'jjuanrivvera',
9+
github: 'jjuanrivvera99',
1010
linkedin: 'jjuanrivvera99',
1111
defaultLanguage: 'en' as const,
1212
supportedLanguages: ['en', 'es', 'pt'] as const,

src/content/config.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,16 @@ const blog = defineCollection({
2828

2929
// Taxonomy
3030
tags: z
31-
.array(z.string())
31+
.array(
32+
z
33+
.string()
34+
.min(2, 'Tags should be at least 2 characters')
35+
.max(30, 'Tags should be 30 characters or less')
36+
.regex(
37+
/^[\p{L}\p{N}\s-]+$/u,
38+
'Tags should only contain letters, numbers, hyphens, and spaces'
39+
)
40+
)
3241
.min(1, 'At least one tag is required')
3342
.max(5, 'Maximum 5 tags allowed'),
3443

src/utils/blogHreflang.ts

Lines changed: 38 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -4,29 +4,27 @@
44
*/
55

66
import type { BlogPost, HreflangLink, SupportedLang } from '@models/blog';
7-
8-
const SITE_URL = 'https://jjuanrivvera.com';
9-
10-
// Language code mapping for hreflang (ISO 639-1 + ISO 3166-1)
11-
const LANG_CODES: Record<SupportedLang, string> = {
12-
en: 'en-US',
13-
es: 'es-ES',
14-
pt: 'pt-BR',
15-
};
7+
import { SITE_CONFIG, LOCALE_MAP } from '@config/site';
168

179
/**
1810
* Build blog post URL for a specific language
11+
* @param slug - The post slug (with or without language prefix)
12+
* @param lang - The target language code
13+
* @returns Full URL to the blog post
14+
* @example
15+
* buildPostUrl('my-post', 'en') // => 'https://jjuanrivvera.com/blog/my-post'
16+
* buildPostUrl('my-post', 'es') // => 'https://jjuanrivvera.com/es/blog/my-post'
1917
*/
2018
function buildPostUrl(slug: string, lang: SupportedLang): string {
2119
// Remove language prefix from slug if present (safe regex)
2220
const cleanSlug = slug.replace(/^(en|es|pt)\//, '');
2321

2422
// English posts don't have language prefix
2523
if (lang === 'en') {
26-
return `${SITE_URL}/blog/${cleanSlug}`;
24+
return `${SITE_CONFIG.url}/blog/${cleanSlug}`;
2725
}
2826

29-
return `${SITE_URL}/${lang}/blog/${cleanSlug}`;
27+
return `${SITE_CONFIG.url}/${lang}/blog/${cleanSlug}`;
3028
}
3129

3230
/**
@@ -43,7 +41,7 @@ export function generateHreflangLinks(
4341

4442
// Add current post's language
4543
links.push({
46-
lang: LANG_CODES[currentPost.data.lang],
44+
lang: LOCALE_MAP[currentPost.data.lang],
4745
url: buildPostUrl(currentPost.slug, currentPost.data.lang),
4846
});
4947

@@ -58,7 +56,7 @@ export function generateHreflangLinks(
5856

5957
translations.forEach((translation) => {
6058
links.push({
61-
lang: LANG_CODES[translation.data.lang],
59+
lang: LOCALE_MAP[translation.data.lang],
6260
url: buildPostUrl(translation.slug, translation.data.lang),
6361
});
6462
});
@@ -87,6 +85,12 @@ export function generateHreflangLinks(
8785

8886
/**
8987
* Get translation of a post in a specific language
88+
* @param currentPost - The current blog post
89+
* @param targetLang - The target language to find translation for
90+
* @param allPosts - All blog posts to search within
91+
* @returns The translated post or null if not found
92+
* @example
93+
* const spanishPost = getTranslation(englishPost, 'es', allPosts);
9094
*/
9195
export function getTranslation(
9296
currentPost: BlogPost,
@@ -107,6 +111,12 @@ export function getTranslation(
107111

108112
/**
109113
* Get all available translations for a post
114+
* @param currentPost - The current blog post
115+
* @param allPosts - All blog posts to search within
116+
* @returns Map of language codes to their corresponding posts
117+
* @example
118+
* const translations = getAllTranslations(post, allPosts);
119+
* const spanishVersion = translations.get('es');
110120
*/
111121
export function getAllTranslations(
112122
currentPost: BlogPost,
@@ -136,24 +146,36 @@ export function getAllTranslations(
136146

137147
/**
138148
* Build blog listing URL for a specific language
149+
* @param lang - The target language code
150+
* @param page - The page number (defaults to 1)
151+
* @returns Full URL to the blog listing page
152+
* @example
153+
* buildBlogListingUrl('en', 1) // => 'https://jjuanrivvera.com/blog'
154+
* buildBlogListingUrl('es', 2) // => 'https://jjuanrivvera.com/es/blog/2'
139155
*/
140156
export function buildBlogListingUrl(
141157
lang: SupportedLang,
142158
page: number = 1
143159
): string {
144160
const basePath = lang === 'en' ? '/blog' : `/${lang}/blog`;
145161
return page === 1
146-
? `${SITE_URL}${basePath}`
147-
: `${SITE_URL}${basePath}/${page}`;
162+
? `${SITE_CONFIG.url}${basePath}`
163+
: `${SITE_CONFIG.url}${basePath}/${page}`;
148164
}
149165

150166
/**
151167
* Build tag page URL for a specific language
168+
* @param tag - The tag name (will be normalized to lowercase with hyphens)
169+
* @param lang - The target language code
170+
* @returns Full URL to the tag archive page
171+
* @example
172+
* buildTagUrl('TypeScript', 'en') // => 'https://jjuanrivvera.com/blog/tag/typescript'
173+
* buildTagUrl('Web Dev', 'es') // => 'https://jjuanrivvera.com/es/blog/tag/web-dev'
152174
*/
153175
export function buildTagUrl(tag: string, lang: SupportedLang): string {
154176
const normalizedTag = tag.toLowerCase().replace(/\s+/g, '-');
155177
const basePath = lang === 'en' ? '/blog/tag' : `/${lang}/blog/tag`;
156-
return `${SITE_URL}${basePath}/${normalizedTag}`;
178+
return `${SITE_CONFIG.url}${basePath}/${normalizedTag}`;
157179
}
158180

159181
/**

src/utils/readingTime.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,9 @@ export function getReadingTime(
3535
const imageCount = imageMatches.length;
3636

3737
// Count code blocks (both fenced and indented)
38+
// Note: This regex handles most common cases but may not capture
39+
// all edge cases (nested/escaped backticks). For production use with
40+
// complex code examples, consider using the MDX AST parser instead.
3841
const codeBlockMatches =
3942
content.match(/```[\s\S]*?```|~~~[\s\S]*?~~~/g) || [];
4043
const codeBlockCount = codeBlockMatches.length;

src/utils/relatedPosts.ts

Lines changed: 46 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -38,21 +38,41 @@ function calculateTagSimilarity(tagsA: string[], tagsB: string[]): number {
3838
return intersection.size / union.size;
3939
}
4040

41+
// Recency scoring thresholds (in days)
42+
const RECENCY_THRESHOLDS = {
43+
VERY_RECENT: 30, // Within 30 days = max score
44+
RECENT: 90, // Within 90 days = high score
45+
MODERATE: 180, // Within 180 days = medium score
46+
OLD: 365, // Within 365 days = low score
47+
} as const;
48+
49+
// Recency scores for each threshold
50+
const RECENCY_SCORES = {
51+
VERY_RECENT: 1.0,
52+
RECENT: 0.8,
53+
MODERATE: 0.6,
54+
OLD: 0.4,
55+
VERY_OLD: 0.2,
56+
} as const;
57+
4158
/**
4259
* Calculate recency score (0-1 scale)
43-
* More recent posts get higher scores
60+
* Scores posts based on temporal proximity to reference date
61+
* Posts closer in time to the reference get higher scores
4462
*/
45-
function calculateRecencyScore(postDate: Date, newestDate: Date): number {
63+
function calculateRecencyScore(postDate: Date, referenceDate: Date): number {
64+
const MS_PER_DAY = 1000 * 60 * 60 * 24;
4665
const daysDiff = Math.abs(
47-
(newestDate.getTime() - postDate.getTime()) / (1000 * 60 * 60 * 24)
66+
(referenceDate.getTime() - postDate.getTime()) / MS_PER_DAY
4867
);
4968

50-
// Posts within 30 days get max score, older posts decay
51-
if (daysDiff <= 30) return 1;
52-
if (daysDiff <= 90) return 0.8;
53-
if (daysDiff <= 180) return 0.6;
54-
if (daysDiff <= 365) return 0.4;
55-
return 0.2;
69+
// Posts within thresholds get corresponding scores
70+
if (daysDiff <= RECENCY_THRESHOLDS.VERY_RECENT)
71+
return RECENCY_SCORES.VERY_RECENT;
72+
if (daysDiff <= RECENCY_THRESHOLDS.RECENT) return RECENCY_SCORES.RECENT;
73+
if (daysDiff <= RECENCY_THRESHOLDS.MODERATE) return RECENCY_SCORES.MODERATE;
74+
if (daysDiff <= RECENCY_THRESHOLDS.OLD) return RECENCY_SCORES.OLD;
75+
return RECENCY_SCORES.VERY_OLD;
5676
}
5777

5878
/**
@@ -85,16 +105,10 @@ export function getRelatedPosts(
85105
return true;
86106
});
87107

88-
// Find newest post date for recency calculation
89-
const newestDate = new Date(
90-
Math.max(
91-
...candidates.map((p) =>
92-
p.data.updatedDate
93-
? p.data.updatedDate.getTime()
94-
: p.data.pubDate.getTime()
95-
)
96-
)
97-
);
108+
// Use current post's date as reference for recency calculation
109+
// This ensures related posts are relative to the current post's timeframe
110+
const currentPostDate =
111+
currentPost.data.updatedDate || currentPost.data.pubDate;
98112

99113
// Calculate similarity scores
100114
const scoredPosts: RelatedPost[] = candidates.map((post) => {
@@ -104,7 +118,7 @@ export function getRelatedPosts(
104118
);
105119

106120
const postDate = post.data.updatedDate || post.data.pubDate;
107-
const recencyScore = calculateRecencyScore(postDate, newestDate);
121+
const recencyScore = calculateRecencyScore(postDate, currentPostDate);
108122

109123
// Weighted score: configurable weights
110124
const score =
@@ -125,6 +139,12 @@ export function getRelatedPosts(
125139

126140
/**
127141
* Get posts by specific tag
142+
* @param allPosts - All blog posts to filter
143+
* @param tag - The tag to filter by (case-insensitive)
144+
* @param excludeDrafts - Whether to exclude draft posts (defaults to true)
145+
* @returns Array of posts with the specified tag, sorted by date (newest first)
146+
* @example
147+
* const typescriptPosts = getPostsByTag(allPosts, 'TypeScript');
128148
*/
129149
export function getPostsByTag(
130150
allPosts: BlogPost[],
@@ -145,6 +165,12 @@ export function getPostsByTag(
145165

146166
/**
147167
* Get all unique tags from posts with counts
168+
* @param posts - All blog posts to extract tags from
169+
* @param excludeDrafts - Whether to exclude draft posts (defaults to true)
170+
* @returns Map of normalized tag names to their occurrence counts, sorted by count (descending)
171+
* @example
172+
* const tagCounts = getAllTags(allPosts);
173+
* // Map { 'typescript' => 5, 'javascript' => 3, 'react' => 2 }
148174
*/
149175
export function getAllTags(
150176
posts: BlogPost[],

tests/blog.spec.ts

Lines changed: 40 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -196,16 +196,25 @@ test.describe('Blog', () => {
196196
}
197197
});
198198

199-
test.skip('language switcher falls back to blog listing when translation unavailable', async ({
200-
page,
201-
}) => {
202-
// Skip: Portuguese language switcher test needs more investigation
199+
test('language switcher provides Portuguese option', async ({ page }) => {
203200
await page.goto('/blog');
204201
await page.locator('.post-card__title a').first().click();
202+
await page.waitForLoadState('networkidle');
205203

206-
// Try to switch to Portuguese
207-
await page.getByRole('button', { name: 'Change language' }).click();
208-
// Portuguese option may not be immediately visible
204+
// Open language switcher (use first one - desktop navbar)
205+
await page
206+
.getByRole('button', { name: 'Change language' })
207+
.first()
208+
.click();
209+
210+
// Portuguese link should be visible and have a valid href
211+
const ptLink = page.locator('#navbar a[hreflang="pt"]').first();
212+
await expect(ptLink).toBeVisible();
213+
214+
const href = await ptLink.getAttribute('href');
215+
expect(href).toBeTruthy();
216+
// Href should either point to /pt/blog (fallback) or /pt/blog/[slug] (translation)
217+
expect(href).toMatch(/^\/pt\/blog/);
209218
});
210219

211220
test('language switcher is visible on blog posts', async ({ page }) => {
@@ -345,12 +354,32 @@ test.describe('Blog', () => {
345354
});
346355

347356
test.describe('Reading Progress', () => {
348-
test.skip('reading progress bar is visible on blog posts', async ({
349-
page,
350-
}) => {
351-
// Skip: Component may not be visible depending on page scroll position
357+
test('reading progress bar is visible on blog posts', async ({ page }) => {
352358
await page.goto('/blog');
353359
await page.locator('.post-card__title a').first().click();
360+
await page.waitForLoadState('networkidle');
361+
362+
// Reading progress bar should exist
363+
const progressBar = page.locator('.reading-progress');
364+
await expect(progressBar).toBeVisible();
365+
366+
// Verify ARIA attributes
367+
await expect(progressBar).toHaveAttribute('role', 'progressbar');
368+
await expect(progressBar).toHaveAttribute('aria-valuemin', '0');
369+
await expect(progressBar).toHaveAttribute('aria-valuemax', '100');
370+
371+
// Scroll down and verify progress updates
372+
await page.evaluate(() =>
373+
window.scrollTo(0, document.body.scrollHeight / 2)
374+
);
375+
await page.waitForTimeout(100); // Wait for scroll handler
376+
377+
// Progress bar inner should have some width
378+
const progressBarInner = page.locator('#reading-progress-bar');
379+
const width = await progressBarInner.evaluate((el) => {
380+
return window.getComputedStyle(el).width;
381+
});
382+
expect(width).not.toBe('0px');
354383
});
355384
});
356385

0 commit comments

Comments
 (0)