fix: address code review feedback

- Add validation for categories and experienceLevel in GET endpoint
- Fix type safety: remove 'any' types, use proper Platform type
- Fix hardcoded translations in Header component
- Refactor platform loading to use centralized platform list
- Remove duplicate getPackageManagerForOS logic
- Improve architectural consistency and DRY principles

Resolves Gemini Code Assist review comments
This commit is contained in:
ersaayan
2025-11-22 18:52:36 +03:00
parent bfc8341ea8
commit 4b45216d22
4 changed files with 75 additions and 56 deletions
+38 -4
View File
@@ -1,6 +1,6 @@
import { NextRequest, NextResponse } from "next/server"; import { NextRequest, NextResponse } from "next/server";
import { RecommendationService } from "@/services/recommendationService"; import { RecommendationService } from "@/services/recommendationService";
import { RecommendationRequest } from "@/types/recommendations"; import { RecommendationRequest, UserCategory, ExperienceLevel } from "@/types/recommendations";
export async function POST(request: NextRequest) { export async function POST(request: NextRequest) {
try { try {
@@ -151,18 +151,52 @@ export async function GET(request: NextRequest) {
); );
} }
// Validate categories
const validCategories: UserCategory[] = [
"development",
"design",
"multimedia",
"system-tools",
"gaming",
"productivity",
"education",
];
const invalidCategories = categories.filter(
(cat) => !validCategories.includes(cat as UserCategory)
);
if (invalidCategories.length > 0) {
return NextResponse.json(
{
error: `Invalid categories: ${invalidCategories.join(", ")}`,
validCategories,
},
{ status: 400 }
);
}
// Validate experience level if provided
const validExperienceLevels: ExperienceLevel[] = ["beginner", "intermediate", "advanced"];
if (experienceLevel && !validExperienceLevels.includes(experienceLevel as ExperienceLevel)) {
return NextResponse.json(
{
error: `Invalid experience_level. Must be one of: ${validExperienceLevels.join(", ")}`,
},
{ status: 400 }
);
}
// Set default limit // Set default limit
const parsedLimit = const parsedLimit =
limit && parseInt(limit) > 0 && parseInt(limit) <= 50 limit && parseInt(limit) > 0 && parseInt(limit) <= 50
? parseInt(limit) ? parseInt(limit)
: 20; : 20;
// Generate recommendations // Generate recommendations with validated types
const recommendations = await RecommendationService.generateRecommendations( const recommendations = await RecommendationService.generateRecommendations(
{ {
platform_id: platformId, platform_id: platformId,
categories: categories as any, categories: categories as UserCategory[],
experienceLevel: experienceLevel as any, experienceLevel: experienceLevel as ExperienceLevel | undefined,
limit: parsedLimit, limit: parsedLimit,
} }
); );
+1 -1
View File
@@ -70,7 +70,7 @@ export function Header({ cryptomusEnabled, onResetPreferences, hasProfile }: Hea
> >
<Settings className="h-4 w-4" /> <Settings className="h-4 w-4" />
<span className="ml-2 hidden sm:inline"> <span className="ml-2 hidden sm:inline">
{locale === 'tr' ? 'Tercihler' : 'Preferences'} {t('recommendations.customize')}
</span> </span>
</Button> </Button>
)} )}
+35 -50
View File
@@ -20,6 +20,7 @@ function RepoHubAppContent({ cryptomusEnabled }: { cryptomusEnabled: boolean })
const [selectedPlatform, setSelectedPlatform] = useState<Platform | null>(null) const [selectedPlatform, setSelectedPlatform] = useState<Platform | null>(null)
const [selectedPackages, setSelectedPackages] = useState<SelectedPackage[]>([]) const [selectedPackages, setSelectedPackages] = useState<SelectedPackage[]>([])
const [generatedScript, setGeneratedScript] = useState<GeneratedScript | null>(null) const [generatedScript, setGeneratedScript] = useState<GeneratedScript | null>(null)
const [availablePlatforms, setAvailablePlatforms] = useState<Platform[]>([])
// Recommendation profile management // Recommendation profile management
const { const {
@@ -32,6 +33,22 @@ function RepoHubAppContent({ cryptomusEnabled }: { cryptomusEnabled: boolean })
const [showOnboarding, setShowOnboarding] = useState(false) const [showOnboarding, setShowOnboarding] = useState(false)
// Load platforms on mount
useEffect(() => {
const loadPlatforms = async () => {
try {
const response = await fetch('/api/platforms')
if (response.ok) {
const platforms = await response.json()
setAvailablePlatforms(platforms)
}
} catch (error) {
console.error('Failed to load platforms:', error)
}
}
loadPlatforms()
}, [])
// Show onboarding modal on first visit // Show onboarding modal on first visit
useEffect(() => { useEffect(() => {
if (!isProfileLoading && !hasCompletedOnboarding) { if (!isProfileLoading && !hasCompletedOnboarding) {
@@ -97,64 +114,29 @@ function RepoHubAppContent({ cryptomusEnabled }: { cryptomusEnabled: boolean })
} }
const handleGenerateScript = () => { const handleGenerateScript = () => {
console.log('🔧 handleGenerateScript called')
console.log('📦 selectedPackages:', selectedPackages)
console.log('🖥️ selectedPlatform:', selectedPlatform)
console.log('✅ hasCompletedOnboarding:', hasCompletedOnboarding)
if (selectedPackages.length === 0) { if (selectedPackages.length === 0) {
console.log('❌ No packages selected')
return return
} }
// Use selected platform, or if not selected, use the platform from recommendations profile // Use selected platform, or if not selected, find platform from available platforms
let platformToUse = selectedPlatform let platformToUse = selectedPlatform
if (!platformToUse && hasCompletedOnboarding) { if (!platformToUse && hasCompletedOnboarding) {
// Get effective OS from profile and find matching platform // Get effective OS from profile and find matching platform from loaded platforms
const effectiveOS = profile.selectedOS || detectedOS const effectiveOS = profile.selectedOS || detectedOS
console.log('🔍 effectiveOS:', effectiveOS)
if (effectiveOS && availablePlatforms.length > 0) {
// We need to fetch the platform data - for now, create a mock platform platformToUse = availablePlatforms.find(p => p.id === effectiveOS) || null
// This should ideally come from the platforms list
if (effectiveOS) { if (!platformToUse) {
platformToUse = { console.warn(`Platform not found for OS: ${effectiveOS}`)
id: effectiveOS,
name: effectiveOS.charAt(0).toUpperCase() + effectiveOS.slice(1),
description: '',
icon: '',
packageManager: getPackageManagerForOS(effectiveOS)
} }
console.log('🎯 Created platform:', platformToUse)
} }
} }
if (platformToUse) { if (platformToUse) {
console.log('✨ Generating script for platform:', platformToUse)
const script = generateScript(selectedPackages, platformToUse) const script = generateScript(selectedPackages, platformToUse)
console.log('📝 Script generated:', script)
setGeneratedScript(script) setGeneratedScript(script)
} else {
console.log('❌ No platform available')
}
}
// Helper function to get package manager for OS
const getPackageManagerForOS = (os: string): string => {
switch (os.toLowerCase()) {
case 'windows':
return 'winget'
case 'macos':
return 'brew'
case 'ubuntu':
case 'debian':
return 'apt'
case 'fedora':
return 'dnf'
case 'arch':
return 'pacman'
default:
return 'unknown'
} }
} }
@@ -228,13 +210,16 @@ function RepoHubAppContent({ cryptomusEnabled }: { cryptomusEnabled: boolean })
<ScriptPreview <ScriptPreview
generatedScript={generatedScript} generatedScript={generatedScript}
selectedPackages={selectedPackages} selectedPackages={selectedPackages}
selectedPlatform={selectedPlatform || { selectedPlatform={selectedPlatform ||
id: generatedScript.platform, availablePlatforms.find(p => p.id === generatedScript.platform) ||
name: generatedScript.platform.charAt(0).toUpperCase() + generatedScript.platform.slice(1), {
description: '', id: generatedScript.platform,
icon: '', name: generatedScript.platform.charAt(0).toUpperCase() + generatedScript.platform.slice(1),
packageManager: getPackageManagerForOS(generatedScript.platform) description: '',
}} icon: '',
packageManager: ''
}
}
onClose={handleCloseScriptPreview} onClose={handleCloseScriptPreview}
/> />
)} )}
+1 -1
View File
@@ -52,7 +52,7 @@ export interface RecommendedPackage {
category?: string; category?: string;
license?: string; license?: string;
type: "gui" | "cli"; type: "gui" | "cli";
platform?: string | any; platform?: Platform;
platform_id?: string; platform_id?: string;
repository: "official" | "third-party" | "aur"; repository: "official" | "third-party" | "aur";
download_url?: string; download_url?: string;