Login and Register APIs integrated along with some file structure improvements #32
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Reviewer's Guide by SourceryThis pull request implements login and registration functionality, along with some server-side changes and file restructuring. The changes include new API endpoints for user authentication, token-based authentication, secure token storage, and improvements to the user interface for login and registration. The pull request also introduces role-based routing and dashboard views for different user types. Sequence diagram for user registration processsequenceDiagram
actor User
participant App as Mobile App
participant Server
User->>App: Fill registration form
App->>Server: POST /user/register
Server-->>App: Registration success
App->>User: Show success message
App->>User: Navigate to picture upload
User->>App: Upload picture
App->>Server: POST /upload/picture
Server-->>App: Picture upload success
App->>User: Navigate to login
Sequence diagram for user login processsequenceDiagram
actor User
participant App as Mobile App
participant Server
User->>App: Enter credentials
App->>Server: POST /user/login
alt Login success
Server-->>App: Return JWT token
App->>User: Navigate to dashboard
else Login failure
Server-->>App: Error message
App->>User: Show error message
end
Class diagram for updated user authenticationclassDiagram
class TokenService {
+saveToken(String token)
+getToken() String?
+deleteToken()
+isTokenValid() bool
+getDecodedToken() Map<String, dynamic>?
}
class LogoutService {
+logoutAndNavigateToLogin(BuildContext context)
}
class LoginPageState {
+_loginUser()
+_navigateBasedOnRole(String token)
}
class RegistrationPageState {
+_submit()
}
TokenService <|-- LogoutService
LoginPageState --> TokenService
RegistrationPageState --> TokenService
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @this-is-mjk - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider improving error handling consistency across the application, especially in API calls and user interactions.
- For future PRs, try to break down large changes into smaller, more focused pull requests to facilitate easier review and reduce the risk of introducing bugs.
Here's what I looked at during the review
- 🟡 General issues: 2 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟡 Complexity: 1 issue found
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| @@ -15,6 +16,9 @@ exports.authenticateToken = (req, res, next) => { | |||
| req.userId = decoded.user.userId; | |||
| next(); | |||
| } catch (err) { | |||
| if (err.name === "TokenExpiredError") { | |||
There was a problem hiding this comment.
suggestion: Improve authentication error handling
Consider adding more specific error handling for different types of authentication errors. This could include distinguishing between expired tokens, invalid signatures, and other potential JWT verification failures.
if (err.name === "TokenExpiredError") {
return res.status(401).json({ message: "Token expired" });
} else if (err.name === "JsonWebTokenError") {
return res.status(401).json({ message: "Invalid token" });
} else if (err.name === "NotBeforeError") {
return res.status(401).json({ message: "Token not active" });
}
| } | ||
| }); | ||
|
|
||
| try { |
There was a problem hiding this comment.
issue (complexity): Consider refactoring the input validation in the markAttendance function to reduce complexity.
The markAttendance function has indeed become more complex with the addition of nested conditional statements for parameter validation. While input validation is crucial, we can simplify this structure to improve readability and maintainability without losing functionality. Consider refactoring the validation logic using object destructuring with default values and a single validation check:
const markAttendance = async (req, res) => {
console.log("Mark User Attendance got hit!");
const { id = '', lat = '', log = '', locationName = '' } = req.body;
try {
if (!id || !lat || !log || !locationName || isNaN(parseFloat(lat)) || isNaN(parseFloat(log))) {
return res.status(400).json({
success: false,
error: {
code: "INVALID_PARAMETERS",
message: "Please provide valid id, lat, log, and locationName",
},
});
}
// Rest of the function remains unchanged
const location = await prisma.location.create({
// ...
});
// ...
} catch (err) {
console.error(err.message);
res.status(500).send("Server Error");
}
};This refactoring achieves the following:
- Uses object destructuring with default empty strings, simplifying the initial parameter extraction.
- Combines all validation checks into a single
ifstatement, reducing nesting and improving readability. - Merges the "missing parameters" and "invalid coordinates" checks into one error response, simplifying the error handling logic.
These changes maintain the added validation while reducing the overall complexity of the function.
its-me-yps
approved these changes
Oct 11, 2024
its-me-yps
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Integration first stage with login and register working
How Has This Been Tested?
locally every function, then the unit test
Types of changes
Checklist:
Summary by Sourcery
Implement user authentication features including registration and login with server-side integration. Introduce token management and logout services. Refactor dashboard components to use a shared profile card. Enhance error handling and UI feedback during authentication processes.
New Features:
Enhancements:
Build:
Tests: