Skip to content

Conversation

@ssofiejohansson
Copy link

@ssofiejohansson ssofiejohansson commented Jun 15, 2025

oskarnordin and others added 8 commits June 4, 2025 20:31
connect to postman with get/post
Connect to frontend
Update
Update backend 12/6
Update
@ssofiejohansson ssofiejohansson changed the title Happy thoughts Happy thoughts by Sofie and Oskar Jun 15, 2025
@solen80a
Copy link

solen80a commented Jun 18, 2025

Great work Sofie and Oskar always a cute touche to it.
Think you have wrapped upp this project in a nice way, it´s easy to follow and understand the functions.
Appreciate that you have added a logout and clear notifications on issues.
Love the cute star, so friendly and communicative ⭐
Added som comment in the code.

@ssofiejohansson
Copy link
Author

@solen80a Thank you Sofia, but cant seem to see the comments you left?

@@ -0,0 +1,33 @@
const mongoose = require('mongoose');

Choose a reason for hiding this comment

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

Seems like this is a duplicate of the Schema in server.js. I suggest you use this Schema instead thou it has more properties for e.g. message, and import it into server.js.

const port = process.env.PORT || 8081;
const app = express();

const happythoughtsData = JSON.parse(fs.readFileSync('./data.json', 'utf-8'));

Choose a reason for hiding this comment

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

The connection to the json-file can be removed, all data is fetch from the database.

mongoose.Promise = Promise;

// RESET_DB logic
if (process.env.RESET_DB) {

Choose a reason for hiding this comment

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

Same as with the file, can be removed, all data is fetch from database.

}
});

app.get('/users', async (req, res) => {

Choose a reason for hiding this comment

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

Consider grouping endpoints with similar content, thoughts with thoughts-related and users with user-related.

}

// Add this log:
console.log(

Choose a reason for hiding this comment

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

Consider removing all console.logs.

message: 'Username and password are required',
});
}

Choose a reason for hiding this comment

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

Like the approach that you clearly address the user if the username is already taken, nice!

Copy link

@JennieDalgren JennieDalgren left a comment

Choose a reason for hiding this comment

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

Great job with this project! I love the cute little hint in the beginning and you have also met all requirements.

If you were to work more with this I would suggest to start breaking up the server.js file into smaller files. Just as you have done with the model.

Also, commit messages could be improved ;)

Keep up the good work and I look forward to see your final projects.

Comment on lines +133 to +145
// //return all thoughts sorted by likes (most likes on top)(has to be placed before the id route)
// app.get('/thoughts/likes', async (req, res) => {
// try {
// const sortedThoughts = await HappyThoughts.find().sort({ hearts: -1 });
// if (sortedThoughts.length > 0) {
// res.json(sortedThoughts);
// } else {
// res.status(404).json({ error: 'No thoughts' });
// }
// } catch (error) {
// res.status(400).json({ error: 'Invalid request' });
// }
// });

Choose a reason for hiding this comment

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

If you dont use this, you could remove it. If you want to keep it to remember or fix later you can add that as a comment

return res.status(404).json({ error: 'Thought not found' });
}

// Only allow the owner to edit

Choose a reason for hiding this comment

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

good work with relevant comments!

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.

4 participants