ChangelingX / 1b31

This project was created for an assignment for Hatchways.io. Not all work is mine, diff first and last commits to see my work.
0 stars 0 forks source link

Feature: Fetch and Update Blog Posts #1

Open ChangelingX opened 2 years ago

ChangelingX commented 2 years ago

THIS ISSUE WAS AUTOGENERATED BY HATCHWAYS Please open a single pull request adding the following API routes:

Please ensure that only a logged in User can use these new routes.

Part 1: Fetching Blog Posts

Implement a route for fetching blog posts by authorIds. The route should:

Your route should exactly match the following specification for the request and responses. Note that the data returned from the database may need to be modified to match the expected response format.

Request

Route: /api/posts

Method: GET

Query Parameters:

Field Type Description Default Value Example Value
authorIds String (required) A comma separated list of integer user IDs. N/A "1,5”
sortBy String (optional) The field to sort the blog posts by. The only acceptable values for this parameter are: id, reads, likes, popularity “id” “popularity”
direction String (optional) The direction for sorting. The only acceptable values for this parameter are: asc, desc “asc” “asc”

Success Response

If the request was formed correctly, a success response containing a JSON payload should be returned.

All the properties of Post should be returned in the response in the format specified below:

Response Status Code: 200

Response Body (JSON):

{
  "posts": [
    {
      "id": 1,  # number
      "likes": 960,  # number
      "popularity": 0.13,  # number
      "reads": 50361,  # number
      "tags": ["tech", "health"],  # array of strings
      "text": "Some text here."  # string
    },
    ...
  ]
}

Error Response

Your route implementation should include error handling. When designing your implementation, think about what edge cases you need to handle and what error messages would be helpful. An example of error handling is verifying the format of the query parameters and that the user is allowed to perform this action. Your error responses should match the provided response format below:

Response Status Code: Use REST best practices when determining which status code to use

Response Body (JSON):

{
  "error": "<error message>"
}

Part 2: Updating a Blog Post

Implement a route for an author to update one of their blog posts. The route should:

Your route should exactly match this specification for the request and responses:

Request

Route: /api/posts/<postId>, where postId is an integer

Method: PATCH

Request Body (JSON):

The following JSON properties can be updated (all properties are optional - if they are excluded from the request, they will not be modified):

Field Type Description Example Value
authorIds Array (Integer)(optional) An array of author IDs. [1, 5]
tags Array (String) (optional) An array of tags. [“health”, “tech”]
text String (optional) The blog post text. “Some long blog post text here.”

The example below shows a request modifying all the available properties. Your route should follow the same format:

{
  "authorIds": [1, 5],
  "tags": ["tech", "health"],
  "text": "Some very short blog post text here."
}

Success Response

If the request was formed correctly, a success response containing a JSON payload should be returned. All the properties of Post should be returned in the response, as well as the user IDs of its authors. This is what your response format should be:

Response Status Code: 200

Response Body (JSON):

{
  "post": {
    "id": 1,  # number
    "authorIds": [1, 5],  # array of numbers
    "likes": 960,  # number
    "popularity": 0.13,  # number
    "reads": 50361,  # number
    "tags": ["health", "tech"],  # array of strings
    "text": "Some very short blog post text here."  # string
  }
}

Error Response

Your route implementation should include error handling. When designing your implementation, think about what edge cases you need to handle and what error messages would be helpful.An example of error handling is verifying the format of the query parameters and that the user is allowed to perform this action. Your error responses should match the provided response format below:

Response Status Code: Use REST best practices when determining which status code to use

Response Body (JSON):

{
  "error": "<error message>"
}

Part 3: Written Evaluation

Lastly, please provide a markdown file with the answers to the following questions below.

The product team has decided that we want to make a change to this application such that authors of a blog post can have different roles:

Questions:

  1. What database changes would be required to the starter code to allow for different roles for authors of a blog post? Imagine that we’d want to also be able to add custom roles and change the permission sets for certain roles on the fly without any code changes.
  2. How would you have to change the PATCH route given your answer above to handle roles?

Considerations

ChangelingX commented 2 years ago

Part 1: Fetching Blog Posts considerations:

Input permutations:

Authentication - User is logged in, user is not logged in. AuthorsIds - None, invalid, one, or many SortBy - None, invalid, or valid direction - None, invalid, or valid

Data state permutations:

AuthorIds - not present, present, multiple present SortBy - unequal or equal

Output permutations: Errors empty sets sets with no duplicates sets with duplicates - must trim

ChangelingX commented 2 years ago

/api/posts GET cannot handle sending json, but it is how the code in the test harness is written. Need to parse query string for get_post() rather than using get_json.

ChangelingX commented 2 years ago

Pushed changes to get_posts_attempt_1. Next need to finish my tests for get_posts, then implement search, sort, de-duplicate logic. I think I want to implement sub methods in get_posts() of validate_args(args), get_posts_by_authorId(authorIds), and sort_posts(posts, sort_column, direction). It may be more efficient to sort as I gather them, but perhaps not.

ChangelingX commented 2 years ago

Comparing two posts to sort them requires changing the Post model to inherit from ComparibleEntity in db/models/post.py. I am going to try this but if it hopelessly breaks something I will have to do comparison the gross way in /api/posts. I think that modifying models/post is in scope for this problem.

ChangelingX commented 2 years ago

"A new API route for fetching blog posts by authors" is tested and functioning.

ChangelingX commented 2 years ago

Part 2: Updating a blog post -

Requirements -

Ensure only author can update own posts
Update the blog post, if it exists, in the database
return updated blog post as JSON

fields -

authorIds - Optional, array of integers.
tags - optional, tags as array of strings
text - string, body of the post.

Input permutations -
  Authentication:
    Not authenticated
    authenticated as someone other than post author
    authenticated as a post author
  fields:
    authorId
      present
        blank
        1 value
        multiple values
      absent
    tags
      present
        blank
        1 value
        multiple values
      absent
    text
      present
      absent

Database state permutations - 
  post
    exists
    does not exist

Responses -
    not authenticated
    post not found
    post found
      authenticated as wrong user
      authenticated as post author
        post not modified
        post modified
ChangelingX commented 2 years ago

Feedback:

code extensibility Partially Meets Expectations

Your solution was well documented and easy to follow but be careful not to [overuse comments](https://qvault.io/clean-code/code-comments/) - comments should say something that is not obvious in the code and that would provide value to future developers.

Some logic can be refactored to be more concise, with less repetition. e.g your logic for validating the request parameters.

We noticed that the [names chosen](https://www.freshconsulting.com/development-principle-1-choose-appropriate-variable-names/) for some variables were non-descriptive (like i, although it is okay to use it in for loop, but one should avoid it).

Print statements are good for testing, but not good practice for production, and should be removed.

best practices & proficiency Partially Meets Expectations

There were instances of the wrong status code being returned.

Some logic could have been made more efficient by using a slightly different database query (bulk fetch/insert) or making the calls in parallel.

When removing duplicates, instead of hashing and comparing the whole object, it would be better and more accurate to just compare the IDs of each post.

You could have used the python's sort function instead of creating it on your own.

Instead of always sorting by asc and then reversing if desc is passed, it is more efficient to sort only once by the correct direction since the number of posts can be quite large.

Instead of deleting all previous records, you can compare the previous authors with the proposed changes and ascertain which ones should be added and which ones should be removed.

written communication Meets Expectations

Your writing is clear and easy to understand.

The written section answer could be improved by providing more details, e.g possible edge cases and how they might be handled.
ChangelingX commented 2 years ago

Print statements removed.

ChangelingX commented 2 years ago

Condensed validation logic.

ChangelingX commented 2 years ago

Removed excessive comments in api/posts.

ChangelingX commented 2 years ago

Reworked sort logic to use built in sort and use correct sorting direction for the start.

ChangelingX commented 2 years ago

Remaining issues:

ChangelingX commented 2 years ago

I have reviewed the standard HTTP response codes and I do not know where wrong status codes are returned. It seems that 400 is the proper response for malformed data, 200 for successful queries, and 404 for absent data. I have changes a couple fields to use 404 instead of 200 for no-results, but otherwise it all looks correct to me?

ChangelingX commented 2 years ago

Changed fetch/delete/insert queries to use bulk logic in sqlalchemy instead of working record by record.