Draft: Implement new API routes to fetch blog posts by `authorIds` and to update a post #2

huaszu · 2023-04-12T20:14:34Z

#1

… the authors specified in the authorIds parameter and returns these posts in the JSON response

api/posts.py

Correct jsonify() argument and return 200 success code Co-authored-by: Jan B <janhenrikbern@users.noreply.github.com>

…y() arguments and 200 success code

… information to match specification for JSON Response Body.

…lp subsequent data manipulation. Implement sorting of blog posts.

…hen value for query parameter is in incorrect format. Create global variable for options on which we can sort posts.

…h introducing reverse_boolean variable closer to where we use it. Separate the error checking and the assigning of reverse_boolean.

…when request asks for posts of author id(s) that are not in the user table in the database. Handle case when the GET request is made correctly and there are no posts to return.

huaszu · 2023-04-16T06:08:21Z

I believe /tests/test_posts.py has one test, test_get_posts, that tests what I have written so far. It passed.

I had not used type hints before you mentioned them, @janhenrikbern . Helped me to update variable names where I was previously indicating the type in the name to help myself remember.

Here are several areas in which I have doubts:

Efficiency

"Sort the blog posts based on the provided query parameters" and "remove any duplicate blog posts (try to be efficient when doing this)" from the issue

What efficiency improvements can I make?

Error handling

Thanks @hdenisenko for emphasizing error checking in the past. I restructured my code significantly after thinking through errors, which I was not expecting! In a few cases where different errors could look the same to the user, I wanted to separate where possible to provide clarity.

What edge cases have I missed?
If someone includes a query parameter we have not planned for, it seems the request would carry on with the rest of the parameters provided, if any. I am not doing anything for this case right now and it seems a nice-to-have to let the user know their excess parameter is irrelevant. Screenshot:

I don't know what the REST best practices are for status codes and implemented based on what I read online. I use 400 a bunch of times. Here is an example when I use 200 with an error message because it's not that the user made the request incorrectly but that certain data does not show up in the db: return jsonify({"error": "None of the author id(s) you requested exist in the database."}), 200
Verify that the user is allowed to perform this action - if the user is not, I suppose I can give a 401 with a message. Right now I don't know how to log out in my terminal 🤔. I couldn't remind myself what currently happens when not logged in. I opened a new terminal window, did not provide an x-access-token header in the request, and was able to get the same responses as in the window I have been testing in. Haha possibly if I wait, my current token will just expire. Tangentially, a strange observation is that when I removed the last character from the token as a test, my request still went through and gave a response providing the information. I looked at Flask documentation and don't understand how @auth_required works yet.

Organization

Should I put this function in another file and import the function to /api/posts.py? Advice on how to name that file?

    def sort_posts_on(item):
        return item[1][sort_by_input]

I made a new file crud.py that currently has one function used in /api/posts.py. Should I name or organize that file in the directory structure better?

I don't know how code reviews would work on a real team so you tell me and look at what interests you?

api/posts.py

hdenisenko · 2023-04-17T03:36:05Z

api/posts.py

+            author_id = int(author_id_input)
+            if crud.check_user_exists(author_id):
+                author_ids.add(author_id)
+    except:


i'm not sure what in your try would hit the except - does crud.check_user_exists return an error?

no, crud.check_user_exists does not return an error. if the user does not exist, the db query in crudwould return None per my understanding and crud.check_user_exists would return False. thanks for the question! i had to remind myself what error i was checking.

i want to hit the except when the request has an invalid authorIds query parameter value, for instance when a character other than "," is used to separate ids or when ids are not typed as numbers (see screenshot). i don't know whether testing for an error in author_ids.split(",") is a great translation of that intent.

an update: crud.check_user_exists can return an error if int(author_id) returns an error, e.g., in the case of the bottom request in the screenshot. whether author_id can be converted to an integer is also part of checking for intended behavior

i suppose we could be more specific to the requester whether the error was that the query parameter value had no integers at all (e.g., "all"), or had integers but incorrectly separated (e.g., "1+2"), or so on

…ctions.

…or route to return according to specification.

…goes with which status code. Make structure of code that handles error checking more consistent across routes.

…ssage if not.

hdenisenko · 2023-05-04T23:25:07Z

api/posts.py

+                                   sort_by=sort_by, 
+                                   direction=direction)
+
+    return jsonify({"posts": result}), 200


I'd recommend returning a flask response object here instead of 2 parameters: https://tedboy.github.io/flask/generated/generated/flask.Response.html

api/posts.py

db/models/post.py

api/util/helpers_to_fetch_posts.py

api/posts.py

… and return results of those queries

…pers_to_fetch_posts.py is part of the service layer, calls that function, and applies business logic. In db/models/post.py, make code less repetitive and more readable by pulling out direction == desc into its own variable.

…th db

…ory layer that contains functions that interact with the database. Improve factoring of code into functions for extensibility, maintability, and readability. Remove print statements from debugging. Make code more concise. Decrease repetitive code. Write comments where helpful. Pass tests.

…op.github.io/sqlite-viewer/ for parsing sqlite files and viewing them online.

Start route that handles GET request for posts having at least one of…

9c54e06

… the authors specified in the authorIds parameter and returns these posts in the JSON response

janhenrikbern reviewed Apr 13, 2023

View reviewed changes

api/posts.py Outdated Show resolved Hide resolved

janhenrikbern reviewed Apr 13, 2023

View reviewed changes

api/posts.py Outdated Show resolved Hide resolved

huaszu and others added 20 commits April 13, 2023 00:02

Update api/posts.py

604f094

Correct jsonify() argument and return 200 success code Co-authored-by: Jan B <janhenrikbern@users.noreply.github.com>

Update URL of route and update return statement to have proper jsonif…

6904073

…y() arguments and 200 success code

Update URL of route and update return statement to have proper jsonif…

6589caf

…y() arguments and 200 success code

Correct URL to match specification. Shorten docstring.

3774363

Make list of author_ids where each author_id is an integer

68326a5

Make list of Post objects to represent in response

470cbf2

Make dictionary containing information to include in response. Format…

44f5f61

… information to match specification for JSON Response Body.

Handle case of user entering duplicate authors.

d660db7

Comment to explain use of dictionary.

3a6f17b

Add file to ignore

9612dc6

Add key-value pair to inner dictionary of posts_data dictionary to he…

76e5d03

…lp subsequent data manipulation. Implement sorting of blog posts.

Implement two directions for sorting blog posts.

2e26c4b

Suggest potential refactoring.

cb60657

Handle error when required query parameter is missing. Handle error w…

04b70b7

…hen value for query parameter is in incorrect format. Create global variable for options on which we can sort posts.

Change error checking of direction by which to sort posts to help wit…

c5b84d0

…h introducing reverse_boolean variable closer to where we use it. Separate the error checking and the assigning of reverse_boolean.

Initial commit

648c0f8

Implement type hints.

3a1e6b6

Update comment on alternative solution.

c208ca4

Refactor so as not to make unnecessary list of Post objects.

5529ef6

Use sets to consider unique author ids and unique posts. Handle case …

9a4d4c4

…when request asks for posts of author id(s) that are not in the user table in the database. Handle case when the GET request is made correctly and there are no posts to return.

hdenisenko reviewed Apr 17, 2023

View reviewed changes

api/posts.py Outdated Show resolved Hide resolved

hdenisenko reviewed Apr 17, 2023

View reviewed changes

api/posts.py Outdated Show resolved Hide resolved

hdenisenko reviewed Apr 17, 2023

View reviewed changes

api/posts.py Outdated Show resolved Hide resolved

hdenisenko reviewed Apr 17, 2023

View reviewed changes

api/posts.py Outdated Show resolved Hide resolved

hdenisenko reviewed Apr 17, 2023

View reviewed changes

api/posts.py Outdated Show resolved Hide resolved

hdenisenko reviewed Apr 17, 2023

View reviewed changes

huaszu added 8 commits April 30, 2023 23:46

Rename file for clarity. Update comment for clarity.

d160ce1

Use helper function to validate post_id and, if valid, return post.

f47e531

Make route more concise by factoring portions of code into helper fun…

e15c332

…ctions.

Write helper function to get post and format information about post f…

dd272e8

…or route to return according to specification.

Refactor for extensibility. Map which kind of error handling message …

06088b0

…goes with which status code. Make structure of code that handles error checking more consistent across routes.

Rewrite for consistency and understanding.

1d058c0

Check whether user included data in request and give helpful error me…

5437c1e

…ssage if not.

Remove unnecessary commented out code.

c712f5a

huaszu changed the title ~~Draft: Implement new API route to fetch blog posts by authorIds~~ Draft: Implement new API routes to fetch blog posts by authorIds and to update a post May 4, 2023

hdenisenko reviewed May 4, 2023

View reviewed changes

api/posts.py Outdated Show resolved Hide resolved

hdenisenko reviewed May 4, 2023

View reviewed changes

api/posts.py Outdated Show resolved Hide resolved

hdenisenko reviewed May 4, 2023

View reviewed changes

db/models/post.py Outdated Show resolved Hide resolved

hdenisenko reviewed May 4, 2023

View reviewed changes

api/util/helpers_to_fetch_posts.py Outdated Show resolved Hide resolved

hdenisenko reviewed May 4, 2023

View reviewed changes

api/posts.py Outdated Show resolved Hide resolved

huaszu added 15 commits June 25, 2023 19:08

Make repository layer for functions that make queries to the database…

c52b43c

… and return results of those queries

Pull out another three functions to repository layer that interact wi…

1f55a4f

…th db

Remove unnecessary imports. Fix code to refer to updated file name

65ce818

Fix database operation to delete a UserPost record

7a69b51

Revise code to use consistent style across API routes

39f41c7

Initial commit

56a7666

Views of SQLite database for reference, with the help of https://inlo…

c628c18

…op.github.io/sqlite-viewer/ for parsing sqlite files and viewing them online.

Include prompt for easy reference. Introduce formatting for readability.

088d1cd

Improve technical correctness, style, organization, and readability

1e15e1a

Update formatting so that all footnotes show

327719b

Edit list indentation for readability

26dfb89

Edit for clarity

9067923

Fix spelling

cb0a60c

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Draft: Implement new API routes to fetch blog posts by `authorIds` and to update a post #2

Draft: Implement new API routes to fetch blog posts by `authorIds` and to update a post #2

huaszu commented Apr 12, 2023

huaszu commented Apr 16, 2023 •

edited

Loading

hdenisenko Apr 17, 2023

huaszu Apr 17, 2023 •

edited

Loading

huaszu Apr 17, 2023

huaszu Apr 17, 2023

hdenisenko May 4, 2023

Draft: Implement new API routes to fetch blog posts by authorIds and to update a post #2

Are you sure you want to change the base?

Draft: Implement new API routes to fetch blog posts by authorIds and to update a post #2

Conversation

huaszu commented Apr 12, 2023

huaszu commented Apr 16, 2023 • edited Loading

Efficiency

Error handling

Organization

hdenisenko Apr 17, 2023

Choose a reason for hiding this comment

huaszu Apr 17, 2023 • edited Loading

Choose a reason for hiding this comment

huaszu Apr 17, 2023

Choose a reason for hiding this comment

huaszu Apr 17, 2023

Choose a reason for hiding this comment

hdenisenko May 4, 2023

Choose a reason for hiding this comment

Draft: Implement new API routes to fetch blog posts by `authorIds` and to update a post #2

Draft: Implement new API routes to fetch blog posts by `authorIds` and to update a post #2

huaszu commented Apr 16, 2023 •

edited

Loading

huaszu Apr 17, 2023 •

edited

Loading