Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Asya & Nourhan - Sprucies #49

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
2 changes: 2 additions & 0 deletions ada-project-docs/wave_01.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ Videos are entities that describe a video at the video store. They contain:
- release date datetime of when the video was release_date
- total inventory of how many copies are owned by the video store



Comment on lines +20 to +21

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think adding changes to this file was intentional. Remember that git add . will add files with changes to be staged ( the . representing 'all' ). Whenever a file is accidentally added into staging, we can remove it from the current batch of changes using the command git rm path/of/file. Always use git status to check which files you're actually staging to be committed!

Our goal for this wave is to be able to do all CRUD actions for these two entities.

## Error Handling Requirements for Every Endpoint
Expand Down
14 changes: 11 additions & 3 deletions app/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
migrate = Migrate()
load_dotenv()


def create_app(test_config=None):
app = Flask(__name__)
app.url_map.strict_slashes = False
Expand All @@ -21,7 +22,6 @@ def create_app(test_config=None):
app.config["SQLALCHEMY_DATABASE_URI"] = os.environ.get(
"SQLALCHEMY_TEST_DATABASE_URI")


# import models for Alembic Setup
from app.models.customer import Customer
from app.models.video import Video
Expand All @@ -31,6 +31,14 @@ def create_app(test_config=None):
db.init_app(app)
migrate.init_app(app, db)

#Register Blueprints Here
# Register Blueprints Here
from .routes.customer_routes import customers_bp
app.register_blueprint(customers_bp)

from .routes.video_routes import videos_bp
app.register_blueprint(videos_bp)

from .routes.rental_routes import rentals_bp
app.register_blueprint(rentals_bp)

return app
return app
10 changes: 8 additions & 2 deletions app/models/customer.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,10 @@
from enum import unique
from app import db

from flask import current_app
class Customer(db.Model):
id = db.Column(db.Integer, primary_key=True)
customer_id = db.Column(db.Integer, primary_key=True, autoincrement = True)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Integer primary key columns will auto-increment by default so it's not required to provide that setting.

We can read more about the autoincrement attribute in the SQL_Alchemy documentation

name = db.Column(db.String)
postal_code = db.Column(db.String)
phone = db.Column(db.String)
registered_at = db.Column(db.DateTime, nullable = True)
Comment on lines +6 to +9

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At the moment, any of these columns can contain null values. Consider revisiting each column to determine which column should truly have null values versus not. For example, is it useful to store a Customer without a name or phone number? Those two columns can be considered required information to store about a customer, despite our requirements not strictly describing that behavior.

videos = db.relationship("Video", secondary="rental", backref="customers")

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To decrease repetition in the customer_routes file, we can include a helper function to return the data about an instance in a dictionary.

Example:

   def to_dict(self):
        return {
            "id" : self.customer_id,
            "name": self.name,
            "phone": self.phone,
            "postal_code": self.postal_code
        }

11 changes: 10 additions & 1 deletion app/models/rental.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,13 @@
from app import db

class Rental(db.Model):
id = db.Column(db.Integer, primary_key=True)
rental_id = db.Column(db.Integer, primary_key=True, autoincrement = True)
customer_id = db.Column(db.Integer, db.ForeignKey('customer.customer_id'), primary_key=True,nullable=True)
video_id = db.Column(db.Integer, db.ForeignKey('video.video_id'), primary_key=True,nullable=True)
due_date = db.Column(db.DateTime)
videos_checked_out_count = db.Column(db.Integer)
available_inventory = db.Column(db.Integer)
videos_checked_in = db.Column(db.Boolean, default=False)
Comment on lines +4 to +10

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comments from Customer class can be applied here.




8 changes: 7 additions & 1 deletion app/models/video.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,10 @@
from enum import unique
from app import db
from flask import current_app


class Video(db.Model):
id = db.Column(db.Integer, primary_key=True)
video_id = db.Column(db.Integer, primary_key=True, autoincrement=True)
title = db.Column(db.String)
total_inventory = db.Column(db.Integer)
release_date = db.Column(db.DateTime, nullable=True)
Empty file removed app/routes.py
Empty file.
153 changes: 153 additions & 0 deletions app/routes/customer_routes.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,153 @@
from app import db
from app.models.customer import Customer
from app.models.video import Video
from app.models.rental import Rental
from flask import Blueprint, jsonify, request, make_response
import requests
from datetime import datetime, timedelta

customers_bp = Blueprint("customers", __name__, url_prefix="/customers")


# refactored and cleaned


@customers_bp.route("", methods=["POST"])
def create_customer():
request_body = request.get_json()

if "name" not in request_body:
return jsonify({"details": "Request body must include name."}), 400
if "postal_code" not in request_body:
return jsonify({"details": "Request body must include postal_code."}), 400
if "phone" not in request_body:
return jsonify({"details": "Request body must include phone."}), 400

new_customer = Customer(
name=request_body["name"],
postal_code=request_body["postal_code"],
phone=request_body["phone"],
)
db.session.add(new_customer)
db.session.commit()

response_body = {


"id": new_customer.customer_id,
"name": new_customer.name,
"postal_code": new_customer.postal_code,
"phone": new_customer.phone,

}
Comment on lines +34 to +42

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You'll need to provide a similar response body for the rest of the customer routes. This is where a helper function in the Customer class would be helpful to use.

Suggested change
response_body = {
"id": new_customer.customer_id,
"name": new_customer.name,
"postal_code": new_customer.postal_code,
"phone": new_customer.phone,
}
response_body = new_customer.to_dict()


return jsonify(response_body), 201


@customers_bp.route("", methods=["GET"])

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

def get_customers():
customers = Customer.query.all()
customer_response = []

for customer in customers:
customer_response.append({
"id": customer.customer_id,
"name": customer.name,
"registered_at": customer.registered_at,
"postal_code": customer.postal_code,
"phone": customer.phone,
})
Comment on lines +53 to +59

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All of the classes could utilize a helper function to return instance data into dictionaries. In this case, a helper function would have reduced these 5 lines of code into 1.

Suggested change
customer_response.append({
"id": customer.customer_id,
"name": customer.name,
"registered_at": customer.registered_at,
"postal_code": customer.postal_code,
"phone": customer.phone,
})
customer_response.append(customer.to_dict())

return jsonify(customer_response)


@customers_bp.route("/<customer_id>", methods=["GET"])
def get_one_customer(customer_id):

if customer_id.isnumeric() != True:
return jsonify({"error": "Invalid Data"}), 400
Comment on lines +66 to +67

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch for invalid customer_id's


customer = Customer.query.get(customer_id)

if customer is None:
return {"message": f"Customer {customer_id} was not found"}, 404
# refactor with helper function
response_body = {
"id": customer.customer_id,
"name": customer.name,
"registered_at": customer.registered_at,
"postal_code": customer.postal_code,
"phone": customer.phone}
Comment on lines +74 to +79

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To stop sounding like a broken record, the helper function would be really useful here!

response_body = customer.to_dict()

return jsonify(response_body), 200


@customers_bp.route("/<customer_id>", methods=["PUT"])

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Good work! Like POST requests, we should also check if data for PUT requests are also valid.

def update_one_customer(customer_id):
customer = Customer.query.get(customer_id)
request_body = request.get_json()

if customer is None:
return {"message": f"Customer {customer_id} was not found"}, 404

if "name" not in request_body or type(customer.name) != str:
return make_response("", 400)
if "postal_code" not in request_body or type(customer.postal_code) != str:
return make_response("", 400)
if "phone" not in request_body or type(customer.phone) != str:
return make_response("", 400)

customer.name = request_body["name"]
customer.postal_code = request_body["postal_code"]
customer.phone = request_body["phone"]

db.session.add(customer)
db.session.commit()

response_body = {
"id": customer.customer_id,
"name": customer.name,
"registered_at": customer.registered_at,
"postal_code": customer.postal_code,
"phone": customer.phone,
}

return jsonify(response_body), 200


@customers_bp.route("/<customer_id>", methods=["DELETE"])

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

def delete_one_customer(customer_id):
customer = Customer.query.get(customer_id)

if customer is None:
return {"message": f"Customer {customer_id} was not found"}, 404

db.session.delete(customer)
db.session.commit()

return {"id": customer.customer_id}


@customers_bp.route("/<customer_id>/rentals", methods=["GET"])
def customer_rentals(customer_id):
if customer_id.isnumeric() != True:
return jsonify({"error": "Invalid Data"}), 400

customer = Customer.query.get(customer_id)

if customer is None:
return {"message": f"Customer {customer_id} was not found"}, 404

rentals = Rental.query.filter_by(
customer_id=customer.customer_id, videos_checked_in=False)

response_body = list()

for rental in rentals:
video = Video.query.get(rental.video_id)

response_body.append(
{
"release_date": video.release_date,
"title": video.title,
"due_date": rental.due_date})

return jsonify(response_body), 200
Comment on lines +134 to +153

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Queries have resource cost (mostly time due to how small the dataset it) but as we scale our applications we want to consider the speed/memory it takes to perform tasks.

In this case, we can reduce the amount of queries (aka the get() method) from 3 to 1 by utilizing the videos attribute in Customer. customer.videos derives from one-to-many relationship Customer has with rentals. Using customer.videos provides all the videos that the customer has rented in the form of a list of video instances.

Using a helper dictionary in the Video to turn video data in dictionary form, we can build the response body for this function like so:

response_body = [video.to_dict() for video in customer.videos]

101 changes: 101 additions & 0 deletions app/routes/rental_routes.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
from app import db
from app.models.customer import Customer
from app.models.video import Video
from app.models.rental import Rental
from flask import Blueprint, jsonify, request, make_response
import requests
from datetime import datetime, timedelta


rentals_bp = Blueprint("rentals", __name__, url_prefix="/rentals")


@rentals_bp.route("/check-out", methods=["POST"])

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

def check_out_vid():
request_body = request.get_json()

if "customer_id" not in request_body or "video_id" not in request_body:
return make_response("", 400)

customer_id = request_body["customer_id"]
video_id = request_body["video_id"]

customer = Customer.query.get(customer_id)
video = Video.query.get(video_id)

if video is None or customer is None:
return jsonify(""), 404

num_current_checked_out = Rental.query.filter_by(
video_id=video.video_id, videos_checked_in=False).count()
current_available_inventory = video.total_inventory - num_current_checked_out

if current_available_inventory == 0:
return jsonify({
"message": "Could not perform checkout"
}), 400

new_rental = Rental(customer_id=customer.customer_id,
video_id=video.video_id,
due_date=(datetime.now() + timedelta(days=7)))

db.session.add(new_rental)
db.session.commit()

num_videos_checked_out = Rental.query.filter_by(
video_id=video.video_id, videos_checked_in=False).count() # .count() returns length
available_inventory = video.total_inventory - num_videos_checked_out

videos_checked_out_count = Rental.query.filter_by(
customer_id=customer.customer_id, videos_checked_in=False).count()

response_body = {
"customer_id": new_rental.customer_id,
"video_id": new_rental.video_id,
"due_date": new_rental.due_date,
"videos_checked_out_count": videos_checked_out_count,
"available_inventory": available_inventory
}
return jsonify(response_body), 200
Comment on lines +29 to +59

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like there's duplicate logic here for checking how many videos were checked out and available. num_current_checked_out and num_videos_checked_out produce the same value, along with current_available_inventory and `available_inventory.



@rentals_bp.route("/check-in", methods=["POST"])
def check_in_vid():

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

request_body = request.get_json()

if "customer_id" not in request_body or "video_id" not in request_body:
return make_response("", 400)

customer_id = request_body["customer_id"]
video_id = request_body["video_id"]

customer = Customer.query.get(customer_id)
video = Video.query.get(video_id)

if video is None or customer is None:
return jsonify(""), 404

rental = Rental.query.filter_by(
video_id=video.video_id, customer_id=customer.customer_id, videos_checked_in=False).first()

if rental is None:
return jsonify({"message": f"No outstanding rentals for customer {customer.customer_id} and video {video.video_id}"}), 400

rental.videos_checked_in = True

num_videos_checked_out = Rental.query.filter_by(
video_id=video.video_id, videos_checked_in=False).count() # .count() returns length
available_inventory = video.total_inventory - num_videos_checked_out

videos_checked_out_count = Rental.query.filter_by(
customer_id=customer.customer_id, videos_checked_in=False).count()

db.session.commit()
response_body = {
"customer_id": rental.customer_id,
"video_id": rental.video_id,

"videos_checked_out_count": videos_checked_out_count,
"available_inventory": available_inventory
}
return jsonify(response_body), 200
Loading