www
/
library
18
1
Fork 0
Code Issues 6 Pull Requests Projects Releases Wiki Activity

Get All Books API #13

Merged
a258wang merged 10 commits from feat/api-all-books into main 2022-02-23 00:17:03 -05:00
Conversation 0 Commits 10 Files Changed 2 +56 -5
a258wang commented 2022-02-15 17:39:27 -05:00
Owner
Copy Link

Closes #2

This PR implements the getAllBooks function that can be used to get summary information about all the books, in order to display them on the main page.

I'm not sure why I waited so long to create this PR, but I suspect that we were blocked by #11 (which has now been merged).

Closes #2 This PR implements the getAllBooks function that can be used to get summary information about all the books, in order to display them on the main page. I'm not sure why I waited so long to create this PR, but I suspect that we were blocked by #11 (which has now been merged).
a258wang added 8 commits 2022-02-15 17:39:28 -05:00
a258wang requested review from j285he 2022-02-15 17:39:54 -05:00
a258wang requested review from n3parikh 2022-02-15 17:40:00 -05:00
j285he reviewed 2022-02-15 22:45:44 -05:00
lib/books.ts Outdated
@ -0,0 +23,4 @@
}
export interface SimpleBook {
authors: string;
j285he commented 2022-02-15 22:42:23 -05:00
Member
Copy Link

There are data rows where authors is NULL, isbn is NULL, and title is NULL. How are we going to handle this?

There are data rows where authors is NULL, isbn is NULL, and title is NULL. How are we going to handle this?
a258wang commented 2022-02-22 22:17:09 -05:00
Author
Owner
Copy Link

Good catch! I can think of a couple of ways to handle this:

  1. When dealing with the result from the database query, replace null values with empty strings.
  2. Update the interface to allow these fields to be string | null, and then accomodate for null values appropriately on the "frontend".

I don't really like option 1 because:
a) I feel like the getAllBooks() function should only be responsible for getting the information from the database, and not for performing additional logic on it.
b) Although replacing with an empty string might work for null strings, what would we do for fields that are supposed to be numbers or booleans or another data type?

So I have implemented option 2.

Good catch! I can think of a couple of ways to handle this: 1. When dealing with the result from the database query, replace `null` values with empty strings. 2. Update the interface to allow these fields to be `string | null`, and then accomodate for `null` values appropriately on the "frontend". I don't really like option 1 because: a) I feel like the `getAllBooks()` function should only be responsible for getting the information from the database, and not for performing additional logic on it. b) Although replacing with an empty string might work for null strings, what would we do for fields that are supposed to be numbers or booleans or another data type? So I have implemented option 2.
j285he commented 2022-02-22 23:09:02 -05:00
Member
Copy Link

Agreed, I also used this solution

Agreed, I also used this solution
👍 1
lib/books.ts Outdated
@ -0,0 +24,4 @@
export interface SimpleBook {
authors: string;
isbn: string;
j285he commented 2022-02-15 22:38:54 -05:00
Member
Copy Link

id is the primary key in the database, do we want that in the interface as well?

`id` is the primary key in the database, do we want that in the interface as well?
n3parikh commented 2022-02-16 03:38:50 -05:00
Owner
Copy Link

Ya, I think id is worth adding since the permalink for the one book page will probably be based on the ID.

Ya, I think `id` is worth adding since the permalink for the one book page will probably be based on the ID.
👍 1
a258wang added 1 commit 2022-02-22 22:08:13 -05:00
a258wang added 1 commit 2022-02-22 22:12:40 -05:00
j285he approved these changes 2022-02-22 23:09:09 -05:00
a258wang merged commit 2a308e1f37 into main 2022-02-23 00:17:03 -05:00
a258wang referenced this issue from a commit 2022-02-23 00:17:03 -05:00
Get All Books API (#13)
a258wang deleted branch feat/api-all-books 2022-02-23 00:17:04 -05:00
Sign in to join this conversation.
Reviewers
No reviewers
n3parikh
j285he
Labels
Clear labels   
Backend

   
Frontend
No Label
Backend
Frontend
Milestone
Clear milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
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#13
No description provided.
Delete Branch "feat/api-all-books"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?