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

Mongo API #504

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Mongo API #504

wants to merge 5 commits into from

Conversation

gittebe
Copy link

@gittebe gittebe commented Dec 13, 2024

Copy link
Contributor

@HIPPIEKICK HIPPIEKICK left a comment

Choose a reason for hiding this comment

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

Since the filters was a stretch goal, I'll allow that you've used separate endpoints for them instead of query params, but please think about it going forward :)


// Defines the port the app will run on. Defaults to 8080, but can be overridden
// when starting the server. Example command to overwrite PORT env variable value:
// PORT=9000 npm start
const port = process.env.PORT || 8080;
const app = express();
const listEndpoints = require("express-list-endpoints")
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this not be imported like your other packages? 👀

Comment on lines +41 to +49
if (process.env.RESET_DB) {
const seedDatabase = async () => {
await Book.deleteMany({});
booksData.forEach(book => {
new Book(book).save()
})
}
seedDatabase()
}
Copy link
Contributor

Choose a reason for hiding this comment

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

OK to remove once you've seeded your DB

});

// Route to get books by author
app.get('/books/author/:author', async (req, res) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

You're still returning books and not authors, so it would be more RESTful to put this as a query param under the /books endpoint

Comment on lines +91 to 134
app.get('/books/title/:title', async (req, res) => {
const { title } = req.params; // Get the title from the URL parameter
try {
const books = await Book.find({
title: {$regex: title, $options: 'i' }})
if (books.length === 0) {
return res.status(404).send('No books found for this author');
}
res.json(books);
} catch (error) {
console.error('Error retrieving book', error);
res.status(500).send('Server error');
}
});

// Route to get books by isbn
app.get('/books/isbn/:isbn', async (req, res) => {
const { isbn } = req.params;
try {
const book = await Book.findOne({ isbn: isbn });
if (!book) {
return res.status(404).send('Book not found');
}
res.json(book);
} catch (error) {
console.error('Error retrieving book by ISBN', error);
res.status(500).send('Server error');
}
});

// Route to get books by language
app.get('/books/language/:language', async (req, res) => {
const { language } = req.params;
try {
const books = await Book.find({ language_code: language });
if (books.length === 0) {
return res.status(404).send('No books found for this language');
}
res.json(books);
} catch (error) {
console.error('Error retrieving books by language', error);
res.status(500).send('Server error');
}
});
Copy link
Contributor

Choose a reason for hiding this comment

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

Same with these filters

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.

2 participants