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
3 changed files with 56 additions and 11 deletions

View File

@ -2,6 +2,29 @@ import sqlite3 from "sqlite3";
const DATABASE_PATH = "catalogue.db"; const DATABASE_PATH = "catalogue.db";
export async function getBook(id: number): Promise<DetailedBook> {
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 FROM books WHERE id = ? AND deleted = 0";
j285he marked this conversation as resolved Outdated

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`?

Good idea, querying a deleted book now throws an error

Good idea, querying a deleted book now throws an error

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`.
const book: Promise<DetailedBook> = new Promise((resolve, reject) => {
database.get(sql, [id], (err: Error | null, book: DetailedBook) => {
if (err) {
reject(err);
}
if (book) {
resolve(book);
} else {
reject(new Error("Not a valid id"));
}
});
});

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).

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.
database.close();
return book;
}
export async function getAllBooks() { export async function getAllBooks() {
const database = new sqlite3.Database(DATABASE_PATH, sqlite3.OPEN_READONLY); const database = new sqlite3.Database(DATABASE_PATH, sqlite3.OPEN_READONLY);
@ -22,6 +45,23 @@ export async function getAllBooks() {
return books; return books;
} }
export interface DetailedBook {
isbn: string | null;
lccn: string | null;
title: string | null;
authors: string | null;
Review

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?
Review

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.
edition: string | null;
publisher: string | null;
publish_year: string | null;
publish_month: string | null;
publish_location: string | null;
pages: string | null;
pagination: string | null;
weight: string | null;
last_updated: Date | null;
deleted: boolean | null;
}
export interface SimpleBook { export interface SimpleBook {
id: number; id: number;
title: string | null; title: string | null;

View File

@ -1,28 +1,33 @@
import React from "react"; import React from "react";
import { getAllBooks, SimpleBook } from "../lib/books"; import { getBook, getAllBooks, DetailedBook, SimpleBook } from "../lib/books";
export default function Home(props: Props) { export default function Home(props: Props) {
return ( return (
<ul> <div>
{props.books.map((book, idx) => { <ul>
return ( {props.books.map((book, idx) => {
<li key={`${idx}_${book.id}`}> return (
{book.id}; {book.title}; {book.authors}; {book.isbn} <li key={`${idx}_${book.id}`}>
</li> {book.id}; {book.title}; {book.authors}; {book.isbn}
); </li>
})} );
</ul> })}
</ul>
<p>{props.book.title}</p>
</div>
); );
} }
interface Props { interface Props {
book: DetailedBook;
books: SimpleBook[]; books: SimpleBook[];
} }
export async function getServerSideProps() { export async function getServerSideProps() {
return { return {
props: { props: {
book: await getBook(44),
books: await getAllBooks(), books: await getAllBooks(),
}, },
}; };

View File

@ -3,7 +3,7 @@
/* Basic Options */ /* Basic Options */
"incremental": true, "incremental": true,
"target": "ES6", "target": "ES6",
"module": "esnext", "module": "CommonJS",
"moduleResolution": "node", "moduleResolution": "node",
"lib": ["dom", "dom.iterable", "esnext"], "lib": ["dom", "dom.iterable", "esnext"],
"sourceMap": true, "sourceMap": true,