Get details for one book #14

Merged
j285he merged 8 commits from j285he-one-book into main 2022-03-20 22:22:58 -04:00
Member

Closes #3

Closes #3
j285he added 2 commits 2022-02-15 22:26:53 -05:00
j285he reviewed 2022-02-15 22:33:57 -05:00
lib/books.ts Outdated
@ -0,0 +20,4 @@
}
});
});
// .then((newBook) => {
Author
Member

Some questions...

Would it be better to have the database.close in the finally statement? Also, would it be benificial to have the catch statement so that error messages are console.logged?

However, adding the catch statement changes the type of book to be Promise<unknown>. I believe this is because the catch function doesn't return anything, the promise returned by then gets resolved with an undefined value..

Some questions... Would it be better to have the database.close in the `finally` statement? Also, would it be benificial to have the `catch` statement so that error messages are console.logged? However, adding the catch statement changes the type of book to be `Promise<unknown>`. I believe this is because the catch function [doesn't return anything, the promise returned by then gets resolved with an undefined value.](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise/then).
Owner

Good questions!

Would it be better to have the database.close in the finally statement?

This is a good question! It seems like if we put the database.close in the finally block, then the database is only closed after the promise has been fulfilled or rejected, whereas if the database.close is not in the finally block, it seems like the database closes earlier but everything still works? Maybe @a3thakra or @n3parikh can give some insight...

Also, would it be benificial to have the catch statement so that error messages are console.logged?

This probably isn't necessary, as any errors should be logged automatically.

I also found this article about error handling that has a short section on "Returning errors in promises" - it seems like we can just reject(err), and then when we call getBook from the frontend or wherever we can consider catching errors there.

Good questions! > Would it be better to have the database.close in the `finally` statement? This is a good question! It seems like if we put the `database.close` in the `finally` block, then the database is only closed after the promise has been fulfilled or rejected, whereas if the `database.close` is not in the `finally` block, it seems like the database closes earlier but everything still works? Maybe @a3thakra or @n3parikh can give some insight... > Also, would it be benificial to have the `catch` statement so that error messages are console.logged? This probably isn't necessary, as [any errors should be logged automatically](https://stackoverflow.com/a/50896443). I also found [this article](https://medium.com/swlh/error-handling-in-javascript-a-quick-guide-54b954427e47) about error handling that has a short section on "Returning errors in promises" - it seems like we can just `reject(err)`, and then when we call `getBook` from the frontend or wherever we can consider catching errors there.
j285he requested review from n3parikh 2022-02-15 23:09:43 -05:00
j285he requested review from a258wang 2022-02-15 23:09:45 -05:00
j285he added 1 commit 2022-02-16 08:55:53 -05:00
j285he added 1 commit 2022-02-17 09:38:32 -05:00
continuous-integration/drone/push Build is passing Details
continuous-integration/drone/pr Build is passing Details
0028372789
Merge branch 'main' into j285he-one-book
j285he added 1 commit 2022-02-22 23:08:43 -05:00
continuous-integration/drone/pr Build is passing Details
continuous-integration/drone/push Build is passing Details
955c906f4e
Fix type for title
a258wang reviewed 2022-02-28 11:59:31 -05:00
lib/books.ts Outdated
@ -0,0 +6,4 @@
const database = new sqlite3.Database(DATABASE_PATH, sqlite3.OPEN_READONLY);
const sql =
"SELECT isbn, lccn, title, subtitle, authors, edition, publisher, publish_year, publish_month, publish_location, pages, pagination, weight, last_updated, deleted FROM books WHERE id = ? ";
Owner

Do we want to be able to query for deleted books, given the deleted book's id? Or should we perhaps restrict ourselves to only books with deleted = 0?

Do we want to be able to query for deleted books, given the deleted book's id? Or should we perhaps restrict ourselves to only books with `deleted = 0`?
Author
Member

Good idea, querying a deleted book now throws an error

Good idea, querying a deleted book now throws an error
j285he marked this conversation as resolved
j285he added 2 commits 2022-03-16 18:19:03 -04:00
continuous-integration/drone/pr Build is passing Details
continuous-integration/drone/push Build is passing Details
f1994b2f5d
Merge main
e26chiu reviewed 2022-03-18 10:33:19 -04:00
@ -25,0 +49,4 @@
isbn: string | null;
lccn: string | null;
title: string | null;
authors: string | null;
First-time contributor

Are the authors going to be stored in a list of strings or are we going to have one string with the list of all author names separated by a comma?

Are the authors going to be stored in a list of strings or are we going to have one string with the list of all author names separated by a comma?
Owner

The database stores the authors field as one string, with names separated by commas.

The database stores the authors field as one string, with names separated by commas.
a258wang reviewed 2022-03-18 11:01:10 -04:00
lib/books.ts Outdated
@ -5,0 +6,4 @@
const database = new sqlite3.Database(DATABASE_PATH, sqlite3.OPEN_READONLY);
const sql =
"SELECT isbn, lccn, title, subtitle, authors, edition, publisher, publish_year, publish_month, publish_location, pages, pagination, weight, last_updated, deleted FROM books WHERE id = ? AND deleted = 0";
Owner

Just a small nitpick, we probably don't need to SELECT deleted FROM books since we know we're only querying for books with deleted = 0.

Just a small nitpick, we probably don't need to `SELECT deleted FROM books` since we know we're only querying for books with `deleted = 0`.
j285he marked this conversation as resolved
j285he added 1 commit 2022-03-18 16:42:59 -04:00
continuous-integration/drone/pr Build is passing Details
continuous-integration/drone/push Build is passing Details
21985f8aa6
Change SQL query
a258wang approved these changes 2022-03-20 13:32:06 -04:00
a258wang left a comment
Owner

👍

👍
j285he merged commit 2040c63311 into main 2022-03-20 22:22:58 -04:00
Sign in to join this conversation.
No reviewers
No Label
Backend
Frontend
No Milestone
No project
No Assignees
3 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: www/library#14
No description provided.