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

Add file edit options on frontend #164

Merged
merged 5 commits into from
Apr 15, 2024
Merged

Add file edit options on frontend #164

merged 5 commits into from
Apr 15, 2024

Conversation

nickbar01234
Copy link
Collaborator

Description

  • Add options to edit files on frontend
  • Allow chapter leaders to edit files if they're assigned to a senior
  • Update CRUD API for files

Copy link

vercel bot commented Apr 15, 2024

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

Name Status Preview Updated (UTC)
the-legacy-project2 🔄 Building (Inspect) Visit Preview Apr 15, 2024 7:18am

@@ -105,7 +105,11 @@ model File {
id String @id @default(auto()) @map("_id") @db.ObjectId
date DateTime // will zero out the hours
filetype String

// Use with Google API
fileId String
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Make it easier to keep track instead of parsing the string.

@@ -106,13 +106,14 @@ export const PATCH = withSessionAndRole(
);
const studentIDsToAdd = seniorBody.StudentIDs;

const { StudentIDs: _, ...other } = seniorBody;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This actually was a bug we didn't notice before. Without destructuring StudentIDs, the relation on User side is not updated.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Good catch, nickbar!

},
include: { senior: true },
});
const otherFiles = await prisma.file.findMany({
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Without this check, user can make files with the same date!

!user.SeniorIDs.some(
(seniorId: string) => seniorId === fileData.seniorId
) ||
otherFiles.length > 0
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Same problem here!

}: {
seniorId: string;
seniorFolder: string;
files: PrismaFile[];
editFile?: PrismaFile;
setEditFile: React.Dispatch<React.SetStateAction<PrismaFile | undefined>>;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't think this is too clean, but it makes the most sense given the time constraint.


if (
!user.SeniorIDs.some((seniorId: string) => seniorId === fileData.seniorId)
otherFiles.length > 0 ||
maybeFile == null ||
Copy link
Collaborator

Choose a reason for hiding this comment

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

use triple equals ===

},
});
onClick={async () => {
editFile == null
Copy link
Collaborator

Choose a reason for hiding this comment

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

===

const fetching = fetchingCreateFile || fetchingUpdateFile;

React.useEffect(() => {
if (editFile != undefined) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

!==

@@ -168,19 +200,28 @@ const AddFile = ({
selectedTags.length == 0 ||
Copy link
Collaborator

Choose a reason for hiding this comment

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

this wasn't one of your changes but triple equals ===

}}
>
{!fetching ? "Create" : "Loading..."}
{fetching ? "Loading..." : editFile == null ? "Create" : "Update"}
Copy link
Collaborator

Choose a reason for hiding this comment

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

===

});

const fetching = fetchingDeleteFile;
const editable = setFileEdit != undefined;
Copy link
Collaborator

Choose a reason for hiding this comment

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

!==

}) => {
const [showAddFilePopUp, setShowAddFilePopUp] = useState<boolean>(false);

const handlePopUp = () => {
setShowAddFilePopUp(!showAddFilePopUp);
};

React.useEffect(() => {
if (editFile != null) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

!==

},
});

if (file === null) {
if (file == null || !file.senior.StudentIDs.includes(session.user.id)) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

===

Copy link
Collaborator

@johnny-t06 johnny-t06 left a comment

Choose a reason for hiding this comment

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

Nice work Nick! Changes requested for the error codes and maybe a later ticket for user feed back

@@ -106,13 +106,14 @@ export const PATCH = withSessionAndRole(
);
const studentIDsToAdd = seniorBody.StudentIDs;

const { StudentIDs: _, ...other } = seniorBody;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Good catch, nickbar!

FileResponse.parse({
code: "INVALID_REQUEST",
message: "Not a valid request",
code: "SUCCESS_UPDATE",
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think unsuccessful error code to display on front end for some user feedback is more smooth

Copy link
Collaborator

@johnny-t06 johnny-t06 left a comment

Choose a reason for hiding this comment

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

Query logic works great. Great changes, should be good to merge!

@nickbar01234 nickbar01234 changed the base branch from nickbar01234/fix-branding to main April 15, 2024 18:26
@nickbar01234 nickbar01234 force-pushed the nickbar01234/file-api branch from 65f9a7e to ba2876b Compare April 15, 2024 18:27
@nickbar01234
Copy link
Collaborator Author

Resolved with @wkim10 on null vs undefined equality check. I think we're good for now!

@nickbar01234 nickbar01234 merged commit 5aa5bc7 into main Apr 15, 2024
4 checks passed
@nickbar01234 nickbar01234 deleted the nickbar01234/file-api branch April 15, 2024 18:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants