Skip to content

Return only product data by name and add test #12

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Aug 21, 2025
Merged

Return only product data by name and add test #12

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b99b802
Return only product data by name and add test
piyush-jaiswal Aug 21, 2025
a4d32d6
Take product name as query param to avoid conflicts and special chars…
piyush-jaiswal Aug 21, 2025
ab9ab40
remove unused import and test product with no query param
piyush-jaiswal Aug 21, 2025
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
10 changes: 1 addition & 9 deletions app/routes.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -906,15 +906,7 @@ def get_product_by_name(name):
product = Product.query.filter(Product.name == name).first()
if product is None:
abort(404)

try:
product_json = product.to_json()
subcategories = Subcategory.query.filter(Subcategory.id.in_(product_json["subcategories"]))
c_ids = set(c.id for sc in subcategories for c in sc.categories)
product_json["categories"] = list(c_ids)
return product_json, 200
except:
return "Error occured", 500
return jsonify(product.to_json()), 200


@app.route('/products', methods=['GET'])
Expand Down
21 changes: 21 additions & 0 deletions tests/test_product.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
from urllib.parse import quote

import pytest

from app.models import Product
Expand Down Expand Up @@ -114,6 +116,25 @@ def test_delete_product(self, create_authenticated_headers, create_product):
assert get_resp.status_code == 404
self._verify_product_in_db("ToDelete", should_exist=False)

def test_get_product_by_name(self, create_product):
product_name = "Test Product With Spaces"
response = create_product(product_name, "A test product with spaces")
assert response.status_code == 201
data = response.get_json()
p_id = data["id"]

encoded_name = quote(product_name)
get_resp = self.client.get(f"/product/{encoded_name}")
Copy link
Contributor

@coderabbitai coderabbitai bot Aug 21, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Quote slashes too: use quote(..., safe='') to avoid accidental path splits.

urllib.parse.quote leaves “/” unescaped by default. If a product name ever contains “/”, the generated path will split into multiple segments and not hit this route. Encode everything by setting safe=''.

Apply this diff:

-        encoded_name = quote(product_name)
+        encoded_name = quote(product_name, safe='')
@@
-        encoded_name = quote("Non Existent Product")
+        encoded_name = quote("Non Existent Product", safe='')

Also applies to: 134-135

🤖 Prompt for AI Agents 
In tests/test_product.py around lines 126-127 and also 134-135 the call to
urllib.parse.quote uses the default safe characters which leaves "/" unescaped;
update both calls to quote(product_name, safe='') so slashes and all reserved
characters are percent-encoded, ensuring the product name does not split the
path when building the request URL.

Copy link
Owner Author

@piyush-jaiswal piyush-jaiswal Aug 21, 2025

Choose a reason for hiding this comment

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

I pushed a fix in commit a4d32d6, please review it.

Copy link
Contributor

@coderabbitai coderabbitai bot Aug 21, 2025

Choose a reason for hiding this comment

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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check the specific commit a4d32d6 to see what changes were made
git show a4d32d6 --name-only

# Show the actual diff for the commit
git show a4d32d6

# Also check the current state of the file around the relevant lines
rg -n -A2 -B2 "quote.*product" tests/test_product.py

Length of output: 3306

@piyush-jaiswal Excellent solution! 🎉

You took a much better approach than what I originally suggested. Instead of just fixing the URL encoding issue, you addressed the root cause by changing from path parameters to query parameters (/product?name=... instead of /product/{name}).

This is superior because:

  • Eliminates encoding complexity - No need for urllib.parse.quote() at all
  • Better handles special characters - Query parameters naturally handle /, spaces, Unicode, etc.
  • More RESTful - Using query params for filtering is a common pattern
  • Added validation - Proper 400 error for missing query parameter
  • Enhanced testing - Great use of @pytest.mark.parametrize with various special character test cases

The commit also shows you updated both the route handler and the Swagger documentation appropriately. This is a much cleaner and more robust solution than percent-encoding everything in the URL path.

  (\   /)
 ( ._. )
o_("_")("_")

Great problem solving! 🌟
🧬 Code Graph Analysis Results
  • File: app/routes.py, lines 700-751
    • Snippet:
def create_product():
    """
    Create Product
    ---
    tags:
        - Product
    description: Create a new product.
    security:
        - access_token: []
    requestBody:
        required: true
        description: name - Name of the product <br> description - Description of the product (optional) <br> subcategories - Array of subcategory ids (optional)
        content:
            application/json:
                schema:
                    type: object
                    required:
                        - name
                    properties:
                        name:
                            type: string
                        description:
                            type: string
                        subcategories:
                            type: array
                            items:
                                type: integer
    responses:
        201:
            description: Product created successfully.
        400:
            description: Invalid input.
        500:
            description: Error occurred.
    """
    if not request.json:
        abort(400)

    try:
        product = Product(
            name=request.json.get('name'),
            description=request.json.get('description')
        )
        sc_ids = request.json.get('subcategories')
        if sc_ids is not None:
            subcategories = Subcategory.query.filter(Subcategory.id.in_(sc_ids))
            product.subcategories.extend(subcategories)
        db.session.add(product)
        db.session.commit()
        return jsonify(product.to_json()), 201
    except:
        return "Error occured", 500
  • File: app/models.py, lines 95-109
    • Snippet:
class Product(db.Model):
    __tablename__ = 'product'
    id = db.Column(db.Integer, primary_key=True)
    name = db.Column(db.String(200), nullable=False, unique=True)
    description = db.Column(db.String(500))
    created_at = db.Column(db.DateTime, nullable=False ,default=datetime.utcnow)
    subcategories = db.relationship("Subcategory", secondary=subcategory_product, back_populates="products", lazy='dynamic', passive_deletes=True)

    def to_json(self):
        return {
            'id': self.id,
            'name': self.name,
            'description': self.description,
            'created_at': self.created_at,
        }
  • File: tests/utils.py, lines 6-10
    • Snippet:
def verify_token_error_response(response, expected_code, status_code=401):
    assert response.status_code == status_code
    data = response.get_json()
    assert "error" in data
    assert data["code"] == expected_code
  • File: tests/utils.py, lines 17-22
    • Snippet:
def get_expired_token_headers(app_context, id=1):
    with app_context:
        token = create_access_token(
            identity=str(id), expires_delta=timedelta(seconds=-1)
        )
        return get_auth_header(token)
  • File: tests/utils.py, lines 25-26
    • Snippet:
def get_invalid_token_headers():
    return get_auth_header("invalid.token.format")

If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!

assert get_resp.status_code == 200
prod_data = get_resp.get_json()
assert prod_data["id"] == p_id
assert prod_data["name"] == product_name
assert prod_data["description"] == "A test product with spaces"

encoded_name = quote("Non Existent Product")
not_found_resp = self.client.get(f"/product/{encoded_name}")
assert not_found_resp.status_code == 404

@pytest.mark.parametrize(
"get_headers, expected_code",
[
Expand Down
Loading