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

[Finishes #187354252-implementing-profile-page-settings #38

Closed
wants to merge 13 commits into from

Conversation

hatfelicien
Copy link
Contributor

Purpose

This pull request aims to implement user profiles in the application to allow users to maintain their personal information.

Changes Made

Implemented user profile creation upon user registration.
Added fields for personal information such as name, email, gender, birthdate, language
Added fields for billing address.
Implemented profile editing functionality.
Associated a timestamp with each user profile.
Documented the feature using Swagger.

Testing Instructions

Register as a new user in the application.
Verify that upon registration, a user profile is created with the provided personal information.
Verify that you can edit your own profile but not another user's profile.
Ensure that a timestamp is associated with each profile creation.
Review the Swagger documentation to understand the profile endpoints and their usage.

Related Issues

Checklist

Please review the following checklist and make sure all tasks are complete before submitting:

  • Code follows the project's coding standards
  • Changes are covered by tests
  • Documentation is updated (if applicable)
  • All tests pass

[finishes #187354240] setting up docker for backend
Copy link
Collaborator

@niyontwali niyontwali left a comment

Choose a reason for hiding this comment

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

Well done! However, there are several areas where you need to improve. Please carefully consider the feedback and apply it accordingly.

Comment on lines 5 to 17
const getAllUserProfiles = async (req: Request, res: Response): Promise<void> => {
try {
// Find all user profiles
const users = await User.findAll();

// Return all user profiles
res.status(200).json(users);
} catch (error) {
// Handle errors
console.error('Error fetching user profiles:', error);
res.status(500).send('Internal Server Error');
}
};
Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove this, because we already have getAllUsers

Comment on lines 24 to 36
const user = await User.findByPk(userId);
if (!user) {
res.status(404).json({ message: 'User not found' });
return;
}

// Return user profile
res.status(200).json({
id: user.id,
firstName: user.firstName,
lastName: user.lastName,
email: user.email,
});
Copy link
Collaborator

Choose a reason for hiding this comment

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

Here, we need to retrieve a single user by their ID along with their roles. We require all the data pertaining to the user, considering it's a user profile. Therefore, you should fetch all the data for the user as well as any related data from other tables/models but for now it is just the model of Role only related.

});
} catch (error) {
// Handle errors
console.error('Error fetching user profile:', error);
Copy link
Collaborator

Choose a reason for hiding this comment

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

remove this console and use the logger. Please refer to other controllers your teammates worked on.

} catch (error) {
// Handle errors
console.error('Error updating user profile:', error);
res.status(500).send('Internal Server Error');
Copy link
Collaborator

Choose a reason for hiding this comment

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

Use the re-usable function in th validations. Please refer to how your teammates did it


const updateUserProfile = async (req: Request, res: Response): Promise<void> => {
const userId: string = req.params.userId;
const newData = req.body;
Copy link
Collaborator

Choose a reason for hiding this comment

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

specify which can be updated, for now from the user model, and please refer to how updateUser was done.

import dotenv from 'dotenv';
import cors from 'cors';
import express from 'express';
import swaggerUI from 'swagger-ui-express';
import swaggerJsDoc from 'swagger-jsdoc';
import options from './docs/swaggerdocs';
import routes from './routes';
//import { validateCreateProfile, validateUpdateProfile } from './validations/profileValidation';
Copy link
Collaborator

Choose a reason for hiding this comment

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

remove this line, why comment it?

import dotenv from 'dotenv';
import cors from 'cors';
import express from 'express';
import swaggerUI from 'swagger-ui-express';
import swaggerJsDoc from 'swagger-jsdoc';
import options from './docs/swaggerdocs';
import routes from './routes';
//import { validateCreateProfile, validateUpdateProfile } from './validations/profileValidation';
import profileRoutes from './routes/profileRoutes';
Copy link
Collaborator

Choose a reason for hiding this comment

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

remove this please, because you already have the profile route define in the overall routes that is in /routes/index.ts

@@ -23,32 +26,49 @@ const swaggerSpec = swaggerJsDoc(options);
app.use('/api/docs', swaggerUI.serve, swaggerUI.setup(swaggerSpec));

app.use('/api', routes);
app.use('/docs', swaggerUI.serve, swaggerUI.setup(swaggerSpec));
Copy link
Collaborator

Choose a reason for hiding this comment

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

remove this please, why did you add it there, yet it is alread on line 26 of this file

logger.error('Testing logger');
res.send('Welcome at Mavericks E-commerce Website Apis');
// Profile routes
app.use('/profiles', profileRoutes);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove this please, because all of this is in the index.ts or the routes

Comment on lines +41 to 46

// Error handling middleware for profile routes
app.use((err: any, req: Request, res: Response, next: NextFunction) => {
console.error(err.stack);
res.status(500).send('Internal Server Error');
});
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why did you add this? You should remove everything you added in the server.ts, I don't think your task involves changing anything from this file

@hozayves hozayves added the bug Something isn't working label Apr 29, 2024
niyontwali and others added 9 commits April 29, 2024 13:43
Fixed some misbehaviour in README.md
…rations

bugfix: configure swagger documentation url and login by checking status and verified fields.
	modified:   src/controllers/userController.ts
	modified:   src/database/models/user.ts
	modified:   src/helpers/token.generator.ts
	new file:   src/middlewares/authMiddlewares.ts
	modified:   src/routes/roleRoute.ts
	modified:   tsconfig.json
	modified:   src/middlewares/authMiddlewares.ts
@hatfelicien hatfelicien closed this May 3, 2024
@hatfelicien hatfelicien deleted the #187354252-ft-profile-page-settings branch May 3, 2024 13:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants