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

added new job #163

Merged
merged 14 commits into from
Nov 19, 2024
Merged
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
1 change: 1 addition & 0 deletions apps/registry/.eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ module.exports = {
'@next/next/no-html-link-for-pages': 'off',
'@next/next/no-sync-scripts': 'off',
'@next/next/no-page-custom-font': 'off',
'@next/next/no-img-element': 'off',
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider using Next.js Image component instead of disabling this rule

While disabling the @next/next/no-img-element rule works, using the Next.js Image component (next/image) would provide better performance through automatic image optimization, lazy loading, and responsive image handling. This is particularly important for job listings where images could impact page load times and user experience.

Would you like help refactoring the JobList component to use the Next.js Image component? I can provide an example implementation or create a GitHub issue to track this improvement.

},
env: {
browser: true,
Expand Down
5 changes: 4 additions & 1 deletion apps/registry/app/[username]/jobs/JobList.js
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,10 @@ const JobDescription = ({ job, makeCoverletter }) => {
};

const JobList = ({ jobs, makeCoverletter }) => {
const fullJobs = jobs?.map((job) => {
const validJobs = jobs?.filter(
(job) => job.gpt_content && job.gpt_content !== 'FAILED'
);
Comment on lines +176 to +178
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add null check before filtering jobs

The filter operation could throw if jobs is null/undefined. Consider adding a default empty array.

- const validJobs = jobs?.filter(
+ const validJobs = (jobs || []).filter(
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const validJobs = jobs?.filter(
(job) => job.gpt_content && job.gpt_content !== 'FAILED'
);
const validJobs = (jobs || []).filter(
(job) => job.gpt_content && job.gpt_content !== 'FAILED'
);

const fullJobs = validJobs?.map((job) => {
const fullJob = JSON.parse(job.gpt_content);
fullJob.raw = job;
return fullJob;
Expand Down
35 changes: 35 additions & 0 deletions apps/registry/app/api/jobs/[uuid]/route.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import { createClient } from '@supabase/supabase-js';
import { NextResponse } from 'next/server';

const supabaseUrl = 'https://itxuhvvwryeuzuyihpkp.supabase.co';
const supabaseKey = process.env.SUPABASE_KEY;
const supabase = createClient(supabaseUrl, supabaseKey);
Comment on lines +4 to +6
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Security: Remove hardcoded database URL and add environment variable validation

  1. The Supabase URL should not be hardcoded as it appears to be a production credential.
  2. The SUPABASE_KEY environment variable should be validated before use.

Apply this diff to fix these issues:

-const supabaseUrl = 'https://itxuhvvwryeuzuyihpkp.supabase.co';
-const supabaseKey = process.env.SUPABASE_KEY;
+const supabaseUrl = process.env.NEXT_PUBLIC_SUPABASE_URL;
+const supabaseKey = process.env.SUPABASE_KEY;
+
+if (!supabaseUrl || !supabaseKey) {
+  throw new Error('Missing required environment variables for Supabase configuration');
+}
+
const supabase = createClient(supabaseUrl, supabaseKey);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const supabaseUrl = 'https://itxuhvvwryeuzuyihpkp.supabase.co';
const supabaseKey = process.env.SUPABASE_KEY;
const supabase = createClient(supabaseUrl, supabaseKey);
const supabaseUrl = process.env.NEXT_PUBLIC_SUPABASE_URL;
const supabaseKey = process.env.SUPABASE_KEY;
if (!supabaseUrl || !supabaseKey) {
throw new Error('Missing required environment variables for Supabase configuration');
}
const supabase = createClient(supabaseUrl, supabaseKey);


export async function GET(request, { params }) {
try {
const { data: job, error } = await supabase
.from('jobs')
.select('*')
.eq('uuid', params.uuid)
.single();
Comment on lines +8 to +14
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add input validation for UUID parameter

The UUID parameter should be validated before querying the database to prevent invalid requests and potential SQL injection.

Apply this diff:

 export async function GET(request, { params }) {
+  const uuid = params.uuid;
+  const uuidRegex = /^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i;
+  
+  if (!uuid || !uuidRegex.test(uuid)) {
+    return NextResponse.json(
+      { message: 'Invalid UUID format' },
+      { status: 400 }
+    );
+  }
+
   try {
     const { data: job, error } = await supabase
       .from('jobs')
       .select('*')
-      .eq('uuid', params.uuid)
+      .eq('uuid', uuid)
       .single();
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export async function GET(request, { params }) {
try {
const { data: job, error } = await supabase
.from('jobs')
.select('*')
.eq('uuid', params.uuid)
.single();
export async function GET(request, { params }) {
const uuid = params.uuid;
const uuidRegex = /^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i;
if (!uuid || !uuidRegex.test(uuid)) {
return NextResponse.json(
{ message: 'Invalid UUID format' },
{ status: 400 }
);
}
try {
const { data: job, error } = await supabase
.from('jobs')
.select('*')
.eq('uuid', uuid)
.single();


if (error) {
return NextResponse.json(
{ message: 'Error fetching job' },
{ status: 500 }
);
}

if (!job) {
return NextResponse.json({ message: 'Job not found' }, { status: 404 });
}

return NextResponse.json(job);
} catch (error) {
console.error('Error fetching job:', error);
return NextResponse.json(
{ message: 'Internal server error' },
{ status: 500 }
);
}
}
73 changes: 73 additions & 0 deletions apps/registry/app/api/resumes/route.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
import { createClient } from '@supabase/supabase-js';
import { NextResponse } from 'next/server';
import gravatar from 'gravatar';

const supabaseUrl = 'https://itxuhvvwryeuzuyihpkp.supabase.co';
const supabaseKey = process.env.SUPABASE_KEY;
const supabase = createClient(supabaseUrl, supabaseKey);
Comment on lines +5 to +7
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Move Supabase URL to environment variables

The Supabase URL should not be hardcoded in the source code. This makes it difficult to manage different environments and poses a security risk.

Move the URL to an environment variable:

-const supabaseUrl = 'https://itxuhvvwryeuzuyihpkp.supabase.co';
+const supabaseUrl = process.env.SUPABASE_URL;
const supabaseKey = process.env.SUPABASE_KEY;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const supabaseUrl = 'https://itxuhvvwryeuzuyihpkp.supabase.co';
const supabaseKey = process.env.SUPABASE_KEY;
const supabase = createClient(supabaseUrl, supabaseKey);
const supabaseUrl = process.env.SUPABASE_URL;
const supabaseKey = process.env.SUPABASE_KEY;
const supabase = createClient(supabaseUrl, supabaseKey);


export async function GET(request) {
try {
const { searchParams } = new URL(request.url);
const limit = parseInt(searchParams.get('limit')) || 3000;
const page = parseInt(searchParams.get('page')) || 1;
const search = searchParams.get('search') || '';
Comment on lines +11 to +14
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Improve query parameter validation and defaults

The current implementation has several concerns:

  1. The default limit of 3000 records is too high and could impact performance
  2. Missing validation for negative numbers and maximum limits
  3. No sanitization of the search parameter to prevent SQL injection via ILIKE

Consider implementing these improvements:

-const limit = parseInt(searchParams.get('limit')) || 3000;
-const page = parseInt(searchParams.get('page')) || 1;
-const search = searchParams.get('search') || '';
+const MAX_LIMIT = 100;
+const requestedLimit = parseInt(searchParams.get('limit')) || 50;
+const limit = Math.min(Math.max(1, requestedLimit), MAX_LIMIT);
+const page = Math.max(1, parseInt(searchParams.get('page')) || 1);
+const search = (searchParams.get('search') || '').replace(/[%_]/g, '');
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const { searchParams } = new URL(request.url);
const limit = parseInt(searchParams.get('limit')) || 3000;
const page = parseInt(searchParams.get('page')) || 1;
const search = searchParams.get('search') || '';
const { searchParams } = new URL(request.url);
const MAX_LIMIT = 100;
const requestedLimit = parseInt(searchParams.get('limit')) || 50;
const limit = Math.min(Math.max(1, requestedLimit), MAX_LIMIT);
const page = Math.max(1, parseInt(searchParams.get('page')) || 1);
const search = (searchParams.get('search') || '').replace(/[%_]/g, '');


console.time('getResumes');
const query = supabase
.from('resumes')
.select('*')
.order('created_at', { ascending: false })
.limit(limit)
.range((page - 1) * limit, page * limit - 1);

if (search) {
query.ilike('name', `%${search}%`);
}

const { data, error } = await query;
Comment on lines +16 to +28
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Optimize database query performance

The current query implementation could be improved:

  1. Using select('*') fetches all columns when only specific ones are needed
  2. No index hint for the name search
  3. Missing error handling for invalid search patterns

Consider these optimizations:

 console.time('getResumes');
 const query = supabase
   .from('resumes')
-  .select('*')
+  .select('username, name, resume, updated_at, created_at')
   .order('created_at', { ascending: false })
   .limit(limit)
   .range((page - 1) * limit, page * limit - 1);

 if (search) {
+  try {
     query.ilike('name', `%${search}%`);
+  } catch (e) {
+    throw new Error('Invalid search pattern');
+  }
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
console.time('getResumes');
const query = supabase
.from('resumes')
.select('*')
.order('created_at', { ascending: false })
.limit(limit)
.range((page - 1) * limit, page * limit - 1);
if (search) {
query.ilike('name', `%${search}%`);
}
const { data, error } = await query;
console.time('getResumes');
const query = supabase
.from('resumes')
.select('username, name, resume, updated_at, created_at')
.order('created_at', { ascending: false })
.limit(limit)
.range((page - 1) * limit, page * limit - 1);
if (search) {
try {
query.ilike('name', `%${search}%`);
} catch (e) {
throw new Error('Invalid search pattern');
}
}
const { data, error } = await query;

console.timeEnd('getResumes');

if (error) {
return NextResponse.json(
{ message: 'Error fetching resumes' },
{ status: 500 }
);
}

console.time('mapResumes');
const resumes = data.map((row) => {
const resume = JSON.parse(row.resume);
return {
username: row.username,
label: resume?.basics?.label,
image:
resume?.basics?.image ||
gravatar.url(
resume?.basics?.email,
{ s: '200', r: 'x', d: 'retro' },
true
),
name: resume?.basics?.name,
location: resume?.basics?.location,
updated_at: row.updated_at,
created_at: row.created_at,
};
});
console.timeEnd('mapResumes');
Comment on lines +38 to +57
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Improve data processing robustness and performance

The current implementation has several potential issues:

  1. No error handling for JSON.parse
  2. Processing could be slow for large datasets
  3. Missing validation for required fields

Consider these improvements:

 console.time('mapResumes');
 const resumes = data.map((row) => {
+  let resume;
+  try {
     resume = JSON.parse(row.resume);
+  } catch (e) {
+    console.error(`Invalid JSON for username ${row.username}:`, e);
+    return null;
+  }
+
+  if (!resume?.basics?.name) {
+    console.warn(`Missing required fields for username ${row.username}`);
+    return null;
+  }
+
   return {
     username: row.username,
     label: resume?.basics?.label,
     image:
       resume?.basics?.image ||
       gravatar.url(
         resume?.basics?.email,
         { s: '200', r: 'x', d: 'retro' },
         true
       ),
     name: resume?.basics?.name,
     location: resume?.basics?.location,
     updated_at: row.updated_at,
     created_at: row.created_at,
   };
-});
+}).filter(Boolean);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
console.time('mapResumes');
const resumes = data.map((row) => {
const resume = JSON.parse(row.resume);
return {
username: row.username,
label: resume?.basics?.label,
image:
resume?.basics?.image ||
gravatar.url(
resume?.basics?.email,
{ s: '200', r: 'x', d: 'retro' },
true
),
name: resume?.basics?.name,
location: resume?.basics?.location,
updated_at: row.updated_at,
created_at: row.created_at,
};
});
console.timeEnd('mapResumes');
console.time('mapResumes');
const resumes = data.map((row) => {
let resume;
try {
resume = JSON.parse(row.resume);
} catch (e) {
console.error(`Invalid JSON for username ${row.username}:`, e);
return null;
}
if (!resume?.basics?.name) {
console.warn(`Missing required fields for username ${row.username}`);
return null;
}
return {
username: row.username,
label: resume?.basics?.label,
image:
resume?.basics?.image ||
gravatar.url(
resume?.basics?.email,
{ s: '200', r: 'x', d: 'retro' },
true
),
name: resume?.basics?.name,
location: resume?.basics?.location,
updated_at: row.updated_at,
created_at: row.created_at,
};
}).filter(Boolean);
console.timeEnd('mapResumes');


console.time('sortResumes');
resumes.sort((a, b) => {
return new Date(b.created_at) - new Date(a.created_at);
});
console.timeEnd('sortResumes');
Comment on lines +59 to +63
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Remove redundant sorting operation

The data is already sorted by created_at in descending order by the database query. The additional in-memory sort is unnecessary and impacts performance.

Remove the redundant sorting:

-console.time('sortResumes');
-resumes.sort((a, b) => {
-  return new Date(b.created_at) - new Date(a.created_at);
-});
-console.timeEnd('sortResumes');
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
console.time('sortResumes');
resumes.sort((a, b) => {
return new Date(b.created_at) - new Date(a.created_at);
});
console.timeEnd('sortResumes');


return NextResponse.json(resumes);
} catch (error) {
console.error('Error processing resumes:', error);
return NextResponse.json(
{ message: 'Internal server error' },
{ status: 500 }
);
}
}
6 changes: 3 additions & 3 deletions apps/registry/app/components/Menu.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,16 +34,16 @@ export default function Menu({ session }) {
>
Explore
</Link>
{/* <Link
<Link
href="/jobs"
className={`text-xl font-bold ${
isActive('/jobs')
isActive('/jobs') || pathname.startsWith('/jobs/')
? 'text-secondary-900 underline'
: 'text-black'
} hover:text-secondary-900 transition-colors duration-200`}
>
Jobs
</Link> */}
</Link>
<a
href="https://github.com/jsonresume/jsonresume.org"
className="text-xl font-bold text-black hover:text-secondary-900 transition-colors duration-200"
Expand Down
Loading
Loading