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.

Sign up for GitHub

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

Refactor login form and improve error handling #37

Merged
merged 3 commits into from
Dec 9, 2024
Merged

Refactor login form and improve error handling #37

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
44bea03
Refactor login form and error handling; update icons and snackbar mes…
JohanGrims Dec 9, 2024
9efec9f
Add vote.css import to Login component
JohanGrims Dec 9, 2024
56bf402
Updating lazy loading
JohanGrims Dec 9, 2024
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
42 changes: 33 additions & 9 deletions src/Vote.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,17 +121,41 @@ export default function Vote() {
.catch((error) => {
setSending(false);
if (error.code === "permission-denied") {
alert(
"Da hat etwas nicht geklappt. Sie sind nicht (mehr) berechtigt, Ihre Daten abzugeben. Bitte wenden Sie sich an den zuständigen Lehrer."
);
snackbar({
message:
"Es ist ein Berechtigungsfehler aufgetreten.",
action:"Details",
onActionClick: () => {
console.error(error);
alert(
"Es scheint, als sei die Wahl nicht mehr verfügbar. Bitte versuchen Sie es später erneut.\n"+error
)
}
Comment on lines +129 to +133
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Avoid concatenating raw error objects in user alerts

Concatenating the error object directly in the alert message may not display meaningful information and can expose sensitive data. Use error.message to provide a user-friendly error description.

Apply this diff to improve the error message:

 onActionClick: () => {
   console.error(error);
   alert(
-    "Es scheint, als sei die Wahl nicht mehr verfügbar. Bitte versuchen Sie es später erneut.\n" + error
+    "Es scheint, als sei die Wahl nicht mehr verfügbar. Bitte versuchen Sie es später erneut.\n" + error.message
   );
 }

});
} else if (error.message === "Network Error") {
alert(
"Netzwerkprobleme. Bitte überprüfen Sie Ihre Internetverbindung und versuchen Sie es erneut."
);
snackbar({
message:
"Es ist ein Netzwerkfehler aufgetreten.",
action:"Details",
onActionClick: () => {
console.error(error);
alert(
"Es scheint, als gäbe es ein Problem mit Ihrer Internetverbindung. Bitte überprüfen Sie diese und versuchen Sie es erneut.\n"+error
)
}
Comment on lines +141 to +145
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Avoid concatenating raw error objects in user alerts

Similar to the previous issue, ensure that you use error.message instead of the raw error object to provide meaningful error information to the user.

Apply this diff:

 onActionClick: () => {
   console.error(error);
   alert(
-    "Es scheint, als gäbe es ein Problem mit Ihrer Internetverbindung. Bitte überprüfen Sie diese und versuchen Sie es erneut.\n" + error
+    "Es scheint, als gäbe es ein Problem mit Ihrer Internetverbindung. Bitte überprüfen Sie diese und versuchen Sie es erneut.\n" + error.message
   );
 }

Committable suggestion skipped: line range outside the PR's diff.

});
} else {
alert(
"Ein unbekannter Fehler ist aufgetreten. Bitte versuchen Sie es später erneut."
);
snackbar({
message:
"Es ist ein Fehler aufgetreten.",
action:"Details",
onActionClick: () => {
console.error(error);
alert(
"Es ist ein Fehler aufgetreten. Bitte versuchen Sie es später erneut.\n"+error
)
}
Comment on lines +153 to +157
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Avoid concatenating raw error objects in user alerts

Again, to maintain consistency and security, use error.message when displaying error information to the user.

Apply this diff:

 onActionClick: () => {
   console.error(error);
   alert(
-    "Es ist ein Fehler aufgetreten. Bitte versuchen Sie es später erneut.\n" + error
+    "Es ist ein Fehler aufgetreten. Bitte versuchen Sie es später erneut.\n" + error.message
   );
 }

Committable suggestion skipped: line range outside the PR's diff.

});
}
});
}
Expand Down
2 changes: 1 addition & 1 deletion src/admin/Overview.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ export default function Overview() {
}}
>
<h2>SchülerInnen</h2>
<mdui-icon name="people"></mdui-icon>
<mdui-icon name="groups"></mdui-icon>
</div>
Verwalten Sie die Datenbank der SchülerInnen, fügen Sie neue
SchülerInnen hinzu und bearbeiten Sie bestehende Klassenlisten.
Expand Down
114 changes: 86 additions & 28 deletions src/admin/auth/Login.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,43 @@ import {
import { alert, prompt, snackbar } from "mdui";
import React from "react";
import { auth } from "../../firebase";
import "../../vote.css";

export default function Login() {
const [email, setEmail] = React.useState("");
const [password, setPassword] = React.useState("");
const [loading, setLoading] = React.useState(false);
interface LoginFormElements extends HTMLFormControlsCollection {
email: HTMLInputElement;
password: HTMLInputElement;
}

interface LoginForm extends HTMLFormElement {
readonly elements: LoginFormElements;
}

const handleLogin = (e: React.FormEvent<LoginForm>) => {
e.preventDefault();

const formData = new FormData(e.currentTarget);
const email = formData.get("email") as string;
const password = formData.get("password") as string;
Comment on lines +25 to +26
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Ensure email and password are not null before use

The formData.get() method can return null if the form fields are missing. Casting a null value to a string may lead to runtime errors when calling authentication functions. It's important to validate that email and password are not null or undefined before proceeding.

Apply this diff to add null checks:

 const email = formData.get("email") as string;
 const password = formData.get("password") as string;
+if (!email || !password) {
+  snackbar({ message: "Bitte geben Sie Ihre Email und Ihr Passwort ein." });
+  setLoading(false);
+  return;
+}

Committable suggestion skipped: line range outside the PR's diff.


setLoading(true);

const handleLogin = () => {
signInWithEmailAndPassword(auth, email, password)
.then((userCredential) => {})
.then((userCredential) => {
const user = userCredential.user;
snackbar({
message: `Willkommen, ${user.email}`,
closeable: true,
});
setLoading(false);
})
.catch((error) => {
console.error(error);
snackbar({
message: error.message,
});
setLoading(false);
JohanGrims marked this conversation as resolved.
Show resolved Hide resolved
});
};

Expand All @@ -33,47 +58,80 @@ export default function Login() {
label: "Email",
},
onConfirm: (email) => {
setLoading(true);
sendPasswordResetEmail(auth, email)
.then(() => {
alert({
headline: "Email gesendet",
description: "Bitte überprüfen Sie Ihren Posteingang.",
});
setLoading(false);
})
.catch((error) => {
console.error(error);
alert({
headline: "Fehler",
description: error.message,
});
setLoading(false);
Comment on lines +61 to +76
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Validate email input before sending password reset email

Ensure that the email provided is not empty to prevent errors when sending the password reset email. This enhances user experience and avoids unnecessary API calls.

Apply this diff to add an email validation check:

 onConfirm: (email) => {
+  if (!email) {
+    alert({
+      headline: "Email erforderlich",
+      description: "Bitte geben Sie Ihre Email-Adresse ein.",
+    });
+    return;
+  }
   setLoading(true);
   sendPasswordResetEmail(auth, email)

Committable suggestion skipped: line range outside the PR's diff.

});
},
});
};

return (
<mdui-card variant="filled" class="card">
<mdui-card variant="filled" class="card" style={{ position: "absolute" }}>
{loading && (
<mdui-linear-progress
style={{
position: "absolute",
top: 0,
left: 0,
right: 0,
zIndex: 100,
}}
/>
)}
<div className="mdui-prose">
<h1>Administratoren-Bereich</h1>
<mdui-text-field
value={email}
onInput={(e) => setEmail(e.target.value)}
type="email"
placeholder="user@example.com"
label="Email"
></mdui-text-field>
<p />
<mdui-text-field
value={password}
onInput={(e) => setPassword(e.target.value)}
type="password"
label="Passwort"
toggle-password
/>
<p />
<br />
<div className="button-container">
<mdui-button variant="text" onClick={handlePasswordReset}>
Passwort zurücksetzen
</mdui-button>
<mdui-button onClick={handleLogin}>Login</mdui-button>
</div>
<form onSubmit={handleLogin}>
<mdui-text-field
type="email"
placeholder="user@example.com"
label="Email"
name="email"
required
></mdui-text-field>
<p />
<mdui-text-field
type="password"
label="Passwort"
toggle-password
name="password"
required
/>
<p />
<br />
<div className="button-container">
{loading ? (
<>
<mdui-button variant="text" disabled>
Passwort zurücksetzen
</mdui-button>

<mdui-button disabled>Login</mdui-button>
</>
) : (
<>
<mdui-button variant="text" onClick={handlePasswordReset}>
Passwort zurücksetzen
</mdui-button>

<mdui-button type="submit">Login</mdui-button>
</>
)}
</div>
</form>
</div>
</mdui-card>
);
Expand Down
5 changes: 4 additions & 1 deletion src/admin/index.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,10 @@ export default function Admin(props) {
confirmText: "Abmelden",
onConfirm: () => {
auth.signOut();
snackbar({ message: "Sie sind jetzt abgemeldet." });
snackbar({
message: "Sie sind jetzt abgemeldet.",
closeable: true,
});
},
});
}}
Expand Down
2 changes: 1 addition & 1 deletion src/admin/navigation/DrawerList.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,7 @@ export default function DrawerList() {
<DrawerItem
active={active === "students"}
title={"SchülerInnen"}
icon={"people"}
icon={"groups"}
onCLick={() => navigate("/admin/students/new-class")}
/>

Expand Down
Loading
Loading