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

Spruce: Sarah and Kaitlyn #45

Open
wants to merge 37 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
6f5693a
Created Video model and routes
kaitlyngore Nov 5, 2021
8cda78f
remove comment
kaitlyngore Nov 5, 2021
67ada14
Added Customer Model.
sjolivas Nov 8, 2021
a59e4e6
Updated migration folder
sjolivas Nov 8, 2021
baef22f
Added customer routes.
sjolivas Nov 8, 2021
ae0898c
Merge pull request #1 from kaitlyngore/customer
kaitlyngore Nov 8, 2021
aec2fcc
rental model created
kaitlyngore Nov 8, 2021
6aa5a85
update Rental model and add references
kaitlyngore Nov 9, 2021
ba08a8b
edit video model
kaitlyngore Nov 9, 2021
39b7123
fixed errors with Video model
kaitlyngore Nov 9, 2021
ba94d12
added check-out route
kaitlyngore Nov 9, 2021
f1622a3
working check-out route
kaitlyngore Nov 9, 2021
2874b8c
added videos routes for rentals
kaitlyngore Nov 9, 2021
74495f0
redo calculations
kaitlyngore Nov 10, 2021
b0944e1
fix check-out route
kaitlyngore Nov 10, 2021
c4ff6b4
Added /<customer_id>/rentals endpoint
sjolivas Nov 12, 2021
332590b
Minor formatting in video.py and video_routes.py
sjolivas Nov 12, 2021
f2397e3
Updated rental files, Added checkin endpoint
sjolivas Nov 12, 2021
1fba000
Reordered commit in checkout endpoint
sjolivas Nov 12, 2021
cc4c3a5
Added comments to rcheckin enapoint
sjolivas Nov 12, 2021
b159db1
Corrected comments
sjolivas Nov 12, 2021
fdb4689
modify wave 1 delete tests
kaitlyngore Nov 12, 2021
e9c9c3b
add delete customer column
kaitlyngore Nov 14, 2021
7d67dc3
Merge branch 'master' into delete_customer
kaitlyngore Nov 14, 2021
cf5d7c4
Merge pull request #2 from kaitlyngore/delete_customer
kaitlyngore Nov 14, 2021
f9d14cb
update customer routes
kaitlyngore Nov 14, 2021
f781561
update rental calculations
kaitlyngore Nov 14, 2021
507322a
rename variable
kaitlyngore Nov 14, 2021
4121b70
Added last delete for vid rental & formatting
sjolivas Nov 15, 2021
a1d11ac
Merge branch 'master' of https://github.com/kaitlyngore/retro-video-s…
sjolivas Nov 15, 2021
65f8a5c
Added unique constraint to customer_id in rental
sjolivas Nov 15, 2021
69db09e
upgraded migrations
sjolivas Nov 15, 2021
54c4f78
Updated the rental model relationship
sjolivas Nov 15, 2021
a244082
update video GET
kaitlyngore Nov 15, 2021
1fecfbe
migrations and check-in route fix
kaitlyngore Nov 15, 2021
18ca179
Added Procfile
sjolivas Nov 15, 2021
86d14dd
video_routes deleted checks
kaitlyngore Nov 16, 2021
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion Procfile
Original file line number Diff line number Diff line change
@@ -1,2 +1 @@
web: gunicorn 'app:create_app()'

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 .video_routes import videos_bp
app.register_blueprint(videos_bp)

from .customer_routes import customers_bp
app.register_blueprint(customers_bp)

from .rental_routes import rentals_bp
app.register_blueprint(rentals_bp)
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.

👍

Nice job making multiple files containing routes separated by the resource type. For organization, we might put them in a routes folder instead of leaving them in the root. Inside that folder (along with a __init__.py) we could have a file per resource, so customer.py, video.py, and rental.py. If they're all in a routes folder, we wouldn't need to include route in the name. Each resource route file would have a blueprint and endpoints for that resource. When we have one blueprint per file, we often name the blueprint simply bp rather than including the resource name as part of it.

Then here, we could import and register the blueprints like

    from .routes import customer, video, rental
    app.register_blueprint(customer.bp)
    app.register_blueprint(video.bp)
    app.register_blueprint(rental.bp)


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

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

Choose a reason for hiding this comment

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

Since this is the only blueprint in this file now, we can just call it bp (this is a common convention).



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

👍

Nice job splitting your endpoints into separate functions. There are still shared pieces of functionality in some of the endpoints that we can think about trying to reduce duplication (especially looking up a record by its id), but on the plus side, in each function, we only need to think about the logic related to that specific endpoint.

def get_customers():
customers = Customer.query.all()
for customer in customers:
if customer.deleted_at:
customers.remove(customer)
customer_response = [customer.customer_dict() for customer in customers]
Comment on lines +14 to +17

Choose a reason for hiding this comment

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

remove is itself O(n), so doing it in a loop like this is potentially O(n^2).

Instead, we can simply skip over the records we don't want during the list comprehension, like:

    customer_response = [customer.customer_dict() for customer in customers if not customer.deleted_at]

return jsonify(customer_response), 200


@customers_bp.route("/<customer_id>", methods=["GET"])
def get_customers_by_id(customer_id):
if not customer_id.isnumeric():

Choose a reason for hiding this comment

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

The most dependable way to check whether a python string represents an integral value is to try to convert it to an int in a try block, then if it raises a ValueError, we know it wasn't an int, so here, that could look like:

    try:
        int(video_id)
    except ValueError:
        return none_value()

The string methods like isnumeric and isdigit are more about detecting classes of characters (think Unicode) than they are for determining what numeric types we might be able to convert the string value into.

return jsonify(None), 400

customer = Customer.query.get(customer_id)
if not customer:
response_body = {"message": f"Customer {customer_id} was not found"}
return jsonify(response_body), 404
if customer.deleted_at:
return jsonify(None), 404
Comment on lines +26 to +31

Choose a reason for hiding this comment

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

Instead of needing to have this extra logic here, consider a helper in the Customer class that looks up the id (with query.get) and would still return None if the record was deleted. Just like we don't want the tests to need to care about how things are being stored in the db, we also want to reduce the amount that the routes themselves need to know about how the data is being managed.

response_body = customer.customer_dict()
return jsonify(response_body), 200


@customers_bp.route("", methods=["POST"])
def post_customer():
request_body = request.get_json()
if "name" not in request_body:
response_body = {"details": "Request body must include name."}
return jsonify(response_body), 400
elif "postal_code" not in request_body:
response_body = {"details": "Request body must include postal_code."}
return jsonify(response_body), 400
elif "phone" not in request_body:
response_body = {
"details": "Request body must include phone."}
return jsonify(response_body), 400
Comment on lines +39 to +48

Choose a reason for hiding this comment

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

Since the outcome of any of these conditions being true is for the function to exit, these could all be if rather than elif. More interestingly, notice how each of these conditions is more or less the same. They differ only in the key being checked for in the request, and the string being used in the error message. If we put the keys into a list and iterated over it, how could we rewrite this to avoid duplicating the logic three times (or potentially more if there were other keys we needed to check!)?


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 = new_customer.customer_dict()
return jsonify(response_body), 201


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

customer = Customer.query.get(customer_id)

Choose a reason for hiding this comment

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

Notice that we should be checking the validity of the id when looking up a model in all of our endpoints. If the client passed in an id to this endpoint like "hello", this code would crash.

if not customer:
return jsonify({"message": f"Customer {customer_id} was not found"}), 404
if customer.deleted_at:
return jsonify(None), 404

request_body = request.get_json()
if (
"name" not in request_body or
"postal_code" not in request_body or
"phone" not in request_body
):
return jsonify({"details": "Invalid data"}), 400

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

db.session.commit()

response_body = customer.customer_dict()
return jsonify(response_body), 200


@customers_bp.route("/<customer_id>", methods=["DELETE"])
def delete_customer(customer_id):
customer = Customer.query.get(customer_id)
if not customer:
return jsonify({"message": f"Customer {customer_id} was not found"}), 404
Comment on lines +90 to +92

Choose a reason for hiding this comment

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

We should also check whether the customer is already deleted in this route. Basically, we need to do it everywhere we try to work with a customer, which is why moving that into a helper method can be useful!


customer.deleted_at = datetime.datetime.now()

db.session.commit()

response_body = customer.customer_dict()
return jsonify(response_body), 200


@customers_bp.route("/<customer_id>/rentals", methods=["GET"])
def rentals_by_id(customer_id):
customer = Customer.query.get(customer_id)
if not customer:
response_body = {"message": f"Customer {customer_id} was not found"}
return jsonify(response_body), 404
if customer.deleted_at:
return jsonify(None), 404
rental_response = [rental.id for rental in customer.rentals]

Choose a reason for hiding this comment

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

Notice this is getting the rental id, not the id of the video in the rental. The tests happen to pass because of the relatively low number of records involved (since there's basically one of each kind, each record ends up with id 1). We need to be really careful to make sure that the tests don't just pass, but also are logically consistent.

Also, due to the relationships defined on your models, a separate video lookup isn't really required. You could go directly from the customer to the video dictionaries like:

    response_body = [rental.video.create_dict() for rental in customer.rentals]

Note that this has the potential to return rentals about videos that have been deleted. Maybe that's ok, maybe not (sort of a business rule decision), but just be aware of that. When we leave data in the database, we have to be especially careful to filter those "phantom" records everywhere.

if not rental_response:
return jsonify([]), 200

response_body = [
Video.query.get(video).create_dict() for video in rental_response
]
return jsonify(response_body), 200
22 changes: 21 additions & 1 deletion app/models/customer.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,24 @@
from sqlalchemy.sql.functions import func
from app import db


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.

According to the SqlAlchemy docs about autoincrement:

The default value is the string "auto" which indicates that a single-column primary key that is of an INTEGER type with no stated client-side or python-side defaults should receive auto increment semantics automatically; all other varieties of primary key columns will not.

This means that here, since the primary is not a compound key, the type is integer, and there's no specified default value, the desired autoincrement behavior will apply even without listing it. It doesn't hurt anything to list it, but it's not required.

name = db.Column(db.String)
postal_code = db.Column(db.String)
phone = db.Column(db.String)
register_at = db.Column(
db.DateTime(timezone=True), server_default=func.now()

Choose a reason for hiding this comment

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

Nice use of a server default value. This actually affects the schema, and ensures that the database will always populate a default value when a customer row is created, regardless of whether or not it's through our API. However, the functions we can specify for the server default are a bit of a toss-up. Ideally, we'd just like to store things in UTC and not worry about timezones, but that might not be straightforward with the server default. For a little more flexibility, we could mark the column as non-nullable, and use a client default, which lets us run any Python code, and would still prevent the addition of rows lacking the data. You would need to play around with this a bit more to see which approach might work best.

)
videos_checked_out = db.Column(db.Integer, default=0)

Choose a reason for hiding this comment

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

Rather than storing this as a column, we can calculate it as needed. It's the number of outstanding rentals related to this customer. Generally, we prefer to avoid storing data that can be calculated from other, more authoritative data, since if the column exists, there are effectively two sources of truth, which could come to disagree.

deleted_at = db.Column(db.DateTime, nullable=True)
Comment on lines +7 to +14

Choose a reason for hiding this comment

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

Keep in mind that the default value for nullable is True. When adding column definitions to our models, we should consider whether it makes sense to allow a record to exist without that data, such as a Customer without a name, a Video without a title, or a Rental without a due date. It's true that we can add checks for this in our logic that adds entries, but we should try to leverage as much validation help from the database as possible. If we tell the database that certain columns are required (not nullable), then we'll encounter an error if we accidentally try to create a row without a value for those columns (which is a good thing)!

rentals = db.relationship("Rental", backref="customer")

Choose a reason for hiding this comment

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

👍

Nice relationship declaration.


def customer_dict(self):
return {
"id": self.customer_id,
"name": self.name,
"postal_code": self.postal_code,
"phone": self.phone,
"register_at": self.register_at.strftime("%a, %d %b %Y %X %z")
}
Comment on lines +17 to +24

Choose a reason for hiding this comment

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

👍

Nice method to convert the model into a dict for returning. We have a variety of endpoint outputs that we need to handle in this project, but having a general-purpose dictionary converter is a great start! Notice that it's a little redundant to have customer in the method name itself. Consider the case where we have a customer instance in a variable called customer. Then to convert to a dictionary, we end up writing customer.customer_dict(). A name like to_dict is a little less repetitive.

34 changes: 33 additions & 1 deletion app/models/rental.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,36 @@
from app import db
from sqlalchemy.sql.functions import func

Choose a reason for hiding this comment

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

func isn't being used in this file, so we don't need to import this.

import datetime
from app.models.video import Video
from app.models.customer import Customer


class Rental(db.Model):
id = db.Column(db.Integer, primary_key=True)
# __tablename__ = "rentals"
id = db.Column(db.Integer, primary_key=True, autoincrement=True)
video_id = db.Column(db.Integer, db.ForeignKey('video.id'), primary_key=True, nullable=False)
customer_id = db.Column(db.Integer, db.ForeignKey('customer.customer_id'), primary_key=True, nullable=False)
Comment on lines +11 to +12

Choose a reason for hiding this comment

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

I generally agree with your decision here to make the foreign keys not nullable. A rental without a customer or video is of questionable value. However, when a row referenced by a foreign key is deleted, postgres attempts to resolve the lost data by setting those foreign keys to NULL, which this constraint prevents, and would cause the final tests (deleting videos and customers in a rental) to fail. In this case, the secondary relationship is what is allowing those tests to pass, as it causes SQLAlchemy to delete the rows in the secondary table that depend on a deleted foreign key. This may be what we want, but on the other hand, data is very valuable to a company. Just be sure you have considered the alternatives and are intentionally choosing an approach after weighing your options.

Also, it's more usual to include the foreign keys in a compound primary key when working with a pure join table, where the compound key also signifies that there can be only a single record in the table between the pair due to primary key uniqueness constraints. In our situation, we wouldn't want that since that would mean that a customer could only ever check out a particular video once and only once! The inclusion of the autoincrementing id key prevents this effect (since the compound key will always be unique) but it also means that having the foreign keys as part of the primary key has no effect. So we can leave the primary key designation off the foreign keys.

due_date = db.Column(db.DateTime, nullable=False, default=datetime.datetime.utcnow() + datetime.timedelta(days=7))

Choose a reason for hiding this comment

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

👍

Something like this would be the alternative to using a server_default as in customer.


def rental_dict(self):
video = Video.query.get(self.video_id)
available_inventory = video.total_inventory - len(video.rentals)
customer = Customer.query.get(self.customer_id)

return {
"video_id": self.video_id,
"customer_id": self.customer_id,
"videos_checked_out_count": len(customer.rentals),
"available_inventory": available_inventory
}

# Using static method decorator. The method references Rental but doesn't
# depend on any instance or behavior of the Rental model, but adds functionality to this model.
@staticmethod
def checkin_dict(customer, video):
Comment on lines +29 to +30

Choose a reason for hiding this comment

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

👍

Nice job adding an additional helper. It can be tough to decide where to put it when multiple types are involved. Sometimes making it an instance method of one or the other class might feel more appropriate, or we might make it a class or static method, or we might make an entirely separate class that just "knows" about building the desired output structure from several inputs. It will vary from case to case.

return {
"video_id": video.id,
"customer_id": customer.customer_id,
"videos_checked_out_count": len(customer.rentals),

Choose a reason for hiding this comment

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

It looks like you made the decision to represent checking in a video by deleting a prior rental record. Notice that if you had instead chosen to mark the rental as "returned" somehow, then the count of rentals wouldn't necessarily be the count of "checked out" rentals.

I mention this only because since you went to the effort of avoiding the deletion of videos and customers, it feels a bit incongruous to delete rental rows. If we had a column in the rental to track the returned data (like the deleted date for videos and customers) then we would need to use that to filter out rentals from this count.

"available_inventory": video.total_inventory - len(video.rentals)
}
38 changes: 37 additions & 1 deletion app/models/video.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,40 @@
from app import db
from flask import request


class Video(db.Model):
id = db.Column(db.Integer, primary_key=True)
id = db.Column(db.Integer, primary_key=True, autoincrement=True)
title = db.Column(db.String, nullable=False)
release_date = db.Column(db.Date, nullable=False)
total_inventory = db.Column(db.Integer, nullable=False)
deleted_at = db.Column(db.DateTime, nullable=True)
Comment on lines +7 to +10

Choose a reason for hiding this comment

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

👍

Nice job thinking about which columns should be nullable.

rentals = db.relationship("Rental", backref="video")

def create_dict(self):
return {
"id": self.id,
"title": self.title,
"release_date": self.release_date,
"total_inventory": self.total_inventory
}

def update(self, form_data):
self.title = form_data["title"]
self.release_date = form_data["release_date"]
self.total_inventory = form_data["total_inventory"]

db.session.commit()
Comment on lines +21 to +26

Choose a reason for hiding this comment

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

👍
Nice helper for updating the record. Notice, though, that this method could crash if it received a dictionary without the required keys. It would be helpful if there was validation logic here as well.

I really like adding methods that update the record (and database) to the model class. It already has a dependency on the db instance (it needed it to derived from Model) and its responsibility really centers around moving data in and out of the database. So anything that does that is an excellent match for becoming a helper here.


@classmethod
def from_json(cls):
Comment on lines +28 to +29

Choose a reason for hiding this comment

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

Having a method to create an instance from a dict is a great idea, but instead of becoming dependent on the request instance (notice you had to import that), you could instead get the request body in the route, and pass the dictionary in as a parameter (making this a sort of alternative constructor). We would definitely want to perform validation here as well, as a missing dictionary key would cause a crash here.

request_body = request.get_json()

new_video = Video(
title=request_body["title"],
release_date=request_body["release_date"],
total_inventory=request_body["total_inventory"]
)
db.session.add(new_video)
db.session.commit()

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

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


@rentals_bp.route("/check-out", methods=["POST"])
def check_out():
request_body = request.get_json()
if "customer_id" not in request_body:
response_body = {"details": "Request body must include customer_id."}
return jsonify(response_body), 400
elif "video_id" not in request_body:
response_body = {"details": "Request body must include video_id."}
return jsonify(response_body), 400

video_id = request_body["video_id"]
customer_id = request_body["customer_id"]
video = Video.query.get(video_id)

Choose a reason for hiding this comment

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

Even here we should be doing id format checks, and whether the record was deleted. Leaving the data in the database has far-reaching effects!

if not video:
return jsonify(None), 404
video_inventory = video.total_inventory - len(video.rentals)

Choose a reason for hiding this comment

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

Consider adding a helper method to the video model so that we don't have to put the logic for this calculation here in the route.

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

customer = Customer.query.get(customer_id)
if not customer:
return jsonify(None), 404

new_rental = Rental(
video_id=request_body["video_id"],
customer_id=request_body["customer_id"]
)

db.session.add(new_rental)
customer.videos_checked_out = len(customer.rentals)

Choose a reason for hiding this comment

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

Your rental_dict helper already calculates the videos checked out count dynamically. Think about removing this column entirely.

db.session.commit()
response_body = new_rental.rental_dict()
return jsonify(response_body), 200


@rentals_bp.route("/check-in", methods=["POST"])
def check_in():
# TODO: make a helper function(s) to make this check and query. Repeating logic from checkout endpoint.
request_body = request.get_json()
if "customer_id" not in request_body:
response_body = {"details": "Request body must include customer_id."}
return jsonify(response_body), 400
elif "video_id" not in request_body:
response_body = {"details": "Request body must include video_id."}
return jsonify(response_body), 400

video_id = request_body["video_id"]
video = Video.query.get(video_id)
customer_id = request_body["customer_id"]
customer = Customer.query.get(customer_id)

# Separate checks beacuse they return different status codes.
if not (video and customer):
response_body = {"message": f"No outstanding rentals for customer {customer_id} and video {video_id}"}
return jsonify(response_body), 404
Comment on lines +63 to +65

Choose a reason for hiding this comment

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

Notice the test for this doesn't check the message. The 404 (not found) message could indicate which of the customer or video wasn't found, not that there are no outstanding rentals.


if customer.videos_checked_out == 0:
response_body = {"message": f"No outstanding rentals for customer {customer_id} and video {video_id}"}
return jsonify(response_body), 400
Comment on lines +67 to +69

Choose a reason for hiding this comment

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

I would be more comfortable doing the actual Rental lookup you perform a few lines down first, and if no rental is found, then this message would be appropriate. The current check is a proxy for that lookup, but could drift out of agreement if direct modifications are ever made to the database.


# finding rental associated with specfic the custoemr_id and video_id and
# deleting their rental record of the sepcified video.
Rental.query.filter_by(customer_id=customer_id, video_id=video_id).delete()
db.session.commit()
# creating checkin dictionary for the response body of a successful request.
# Updates the videos_checked_out_count and available inventory.
customer.videos_checked_out = len(customer.rentals)
response_body = Rental.checkin_dict(customer, video)
return jsonify(response_body), 200
Loading