add teacher management page

[135ze]

Notion ticket link

Unstyled Teacher Management

Implementation description

• /manage page now contains teacher data
• can edit teacher role or delete teacher

Steps to test

1. ensure db works and is seeded (either docker or vercel)
2. go to /manage

What should reviewers focus on?

• general good code practices

Checklist

• My PR name is descriptive and in imperative tense
• My commit messages are descriptive and in imperative tense. My commits are atomic and trivial commits are squashed or fixup'd into non-trivial commits
• I have run the appropriate linter(s)
• I have requested a review from the PL, as well as other devs who have background knowledge on this PR or who will be building on top of this PR

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
sistema	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Nov 13, 2024 2:42am

[ChinemeremChigbo]

Please look into the failing deployment. Also, it seems like this PR is dependent on the seeding script PR #12 please help @VJagiasi get his PR approved and then request a re-review.

[ChinemeremChigbo]

Some initial comments. Will continue to do a more thorough review.

[ChinemeremChigbo]

Not sure if any of these changes are needed? Does you PR still work without them?

[ChinemeremChigbo]

I think you should leave this file unchanged in this PR.

[ChinemeremChigbo]

I think you should leave this file unchanged in this PR.

[135ze]

Some initial comments. Will continue to do a more thorough review.

Hey @ChinemeremChigbo , should be good to go now with the deployment and removals of extra changes

[ChinemeremChigbo]

Add type for req

Suggested change

	export default async function handler(req, res: NextApiResponse) {
	export default async function handler(req: NextApiRequest, res: NextApiResponse) {

[ChinemeremChigbo]

Probably should have a try catch here

[ChinemeremChigbo]

Might want to throw a 500 if there's an error.

[ChinemeremChigbo]

Otherwise this can occur:

 ⨯ PrismaClientKnownRequestError: 
Invalid `prisma.user.findMany()` invocation:


The table `public.User` does not exist in the current database.
    at Mn.handleRequestError (/Users/chinemerem/Documents/GitHub/sistema/node_modules/@prisma/client/runtime/library.js:121:7753)
    at Mn.handleAndLogRequestError (/Users/chinemerem/Documents/GitHub/sistema/node_modules/@prisma/client/runtime/library.js:121:7061)
    at Mn.request (/Users/chinemerem/Documents/GitHub/sistema/node_modules/@prisma/client/runtime/library.js:121:6745)
    at async l (/Users/chinemerem/Documents/GitHub/sistema/node_modules/@prisma/client/runtime/library.js:130:9633)
    at async handler (webpack-internal:///(api)/./src/pages/api/users.ts:8:19)
    at async K (/Users/chinemerem/Documents/GitHub/sistema/node_modules/next/dist/compiled/next-server/pages-api.runtime.dev.js:21:2871)
    at async U.render (/Users/chinemerem/Documents/GitHub/sistema/node_modules/next/dist/compiled/next-server/pages-api.runtime.dev.js:21:3955)
    at async DevServer.runApi (/Users/chinemerem/Documents/GitHub/sistema/node_modules/next/dist/server/next-server.js:598:9)
    at async NextNodeServer.handleCatchallRenderRequest (/Users/chinemerem/Documents/GitHub/sistema/node_modules/next/dist/server/next-server.js:269:37)
    at async DevServer.handleRequestImpl (/Users/chinemerem/Documents/GitHub/sistema/node_modules/next/dist/server/base-server.js:810:17)
    at async /Users/chinemerem/Documents/GitHub/sistema/node_modules/next/dist/server/dev/next-dev-server.js:339:20
    at async Span.traceAsyncFn (/Users/chinemerem/Documents/GitHub/sistema/node_modules/next/dist/trace/trace.js:154:20)
    at async DevServer.handleRequest (/Users/chinemerem/Documents/GitHub/sistema/node_modules/next/dist/server/dev/next-dev-server.js:336:24)
    at async invokeRender (/Users/chinemerem/Documents/GitHub/sistema/node_modules/next/dist/server/lib/router-server.js:173:21)
    at async handleRequest (/Users/chinemerem/Documents/GitHub/sistema/node_modules/next/dist/server/lib/router-server.js:350:24) {
  code: 'P2021',
  clientVersion: '5.20.0',
  meta: { modelName: 'User', table: 'public.User' },
  page: '/api/users'
}

[135ze]

Yeah makes sense, but I think that error is only when the database isn't seeded? So that would be a larger issue haha

[ChinemeremChigbo]

Suggestion for this file:

import type { NextApiRequest, NextApiResponse } from 'next';
import { prisma } from '../../../utils/prisma';

export default async function handler(
  req: NextApiRequest,
  res: NextApiResponse
) {
  try {
    const users = await prisma.user.findMany();
    res.status(200).json(users);
  } catch (error) {
    console.error('Error fetching users:', error);
    res.status(500).json({ error: 'Internal server error' });
  }
}

[135ze]

Make sense! Added here: 2c7bce5.

Didn't add type of req originally since it's not being used 😅 but yeah I guess it's nicer

[ChinemeremChigbo]

This is the same logic as in the GET block. I would extract it and run it before.

[ChinemeremChigbo]

Might be helpful to have a console.error here as well

[ChinemeremChigbo]

I would add a console.error here

[ChinemeremChigbo]

recommended file changes

import type { NextApiRequest, NextApiResponse } from 'next';
import { prisma } from '../../../../utils/prisma';

export default async function handler(
  req: NextApiRequest,
  res: NextApiResponse
) {
  const id = Number(req.query.id);

  if (Number.isNaN(id)) {
    return res.status(400).json({ error: 'Invalid ID provided' });
  }

  if (req.method === 'GET') {
    const getAbsences = req.query.getAbsences === 'true';

    try {
      const user = await prisma.user.findUnique({
        where: { id },
        include: { absences: getAbsences },
      });

      if (!user) {
        return res.status(404).json({ error: 'User not found' });
      }

      return res.status(200).json(user);
    } catch (error) {
      console.error('Error fetching user:', error);
      return res.status(500).json({ error: 'Internal server error' });
    }
  } else if (req.method === 'PATCH') {
    const { email, firstName, lastName, role, status } = req.body;
    const id = Number(req.query.id as string);

    if (Number.isNaN(id)) {
      return res.status(400).json({ error: 'Invalid ID provided' });
    }

    try {
      const updatedUser = await prisma.user.update({
        where: { id },
        data: { email, firstName, lastName, role, status },
      });

      if (!updatedUser) {
        return res.status(400).json({ error: 'User not found' });
      }

      return res.status(200).json(updatedUser);
    } catch (error) {
      console.error('Error updating user:', error);
      return res.status(500).json({ error: 'Internal server error' });
    }
  } else {
    return res.status(405).json({ error: 'Method not allowed' });
  }
}

[135ze]

Yep fair! Added: 2c7bce5

[ChinemeremChigbo]

Suggestion on this file:

import { useEffect, useState } from 'react';
interface User {
  id: number;
  firstName: string;
  lastName: string;
  email: string;
  role: string;
  status: string;
  numOfAbsences: string;
}

export default function AnotherPage() {
  const [users, setUsers] = useState<User[]>([]);
  const [loading, setLoading] = useState<boolean>(true);

  useEffect(() => {
    const fetchUsers = async () => {
      const apiUrl = `/api/users/`;

      try {
        const response = await fetch(apiUrl);
        if (!response.ok) {
          throw new Error(response.statusText);
        }
        const data: User[] = await response.json();
        setUsers(data);
      } catch (error: unknown) {
        if (error instanceof Error) {
          console.error('Error fetching users:', error.message);
        }
      } finally {
        setLoading(false);
      }
    };

    fetchUsers();
  }, []);

  const updateUserRole = async (userId: number, newRole: string) => {
    const confirmed = window.confirm('Confirm change role to ' + newRole);
    if (!confirmed) return;

    const apiUrl = `/api/users/${userId}`;
    const originalUsers = [...users];

    setUsers((prevUsers) =>
      prevUsers.map((user) =>
        user.id === userId ? { ...user, role: newRole } : user
      )
    );

    try {
      const response = await fetch(apiUrl, {
        method: 'PATCH',
        headers: {
          'Content-Type': 'application/json',
        },
        body: JSON.stringify({ role: newRole }),
      });

      if (!response.ok) {
        setUsers(originalUsers);
        throw new Error(response.statusText);
      }
    } catch (error: unknown) {
      if (error instanceof Error) {
        console.error('Error updating user:', error.message);
      }
    }
  };

  return (
    <div>
      <h1>Admin Management</h1>
      {loading ? (
        <p>Loading...</p>
      ) : (
        <table>
          <thead>
            <tr>
              <th>Name</th>
              <th>Email</th>
              <th>Role</th>
            </tr>
          </thead>
          <tbody>
            {users.map((user) => (
              <tr key={user.id}>
                <td>
                  {user.firstName} {user.lastName}
                </td>
                <td>{user.email}</td>
                <td>
                  <select
                    value={user.role}
                    onChange={(e) => updateUserRole(user.id, e.target.value)}
                  >
                    <option value="TEACHER">Teacher</option>
                    <option value="ADMIN">Admin</option>
                  </select>
                </td>
              </tr>
            ))}
          </tbody>
        </table>
      )}
    </div>
  );
}

[ChinemeremChigbo]

I like it more to have a separate loading state.

[135ze]

Yeah loading is nice, done: 2c7bce5