[Features] verify & implement `add_item_route`

[Pk13055]

Things to note:

• [ ] Ensure feature branch created with [Features] verify & implement add_item_route: route-fix, so eg: get_category_route-fix
• [ ] Rebase frequently to avoid conflicts later on
• [ ] Try to ensure only a couple of commits
• [ ] Test your code thoroughly before pushing
• [ ] Add to description, "Fixes #issue-no, eg: Fixes #8 (where #8 is the issue id)"

Final merge request will be of

Merge route-fix of [username]/recommender-portal into OSDG-IIITH/recommender-portal

Things to note:

• [ ] Ensure feature branch created with [Features] verify & implement add_item_route: route-fix, so eg: get_category_route-fix
• [ ] Rebase frequently to avoid conflicts later on
• [ ] Try to ensure only a couple of commits
• [ ] Test your code thoroughly before pushing
• [ ] Add to description, "Fixes #issue-no, eg: Fixes #8 (where #8 is the issue id)"

Final merge request will be of

Merge route-fix of [username]/recommender-portal into OSDG-IIITH/recommender-portal

[gsc2001]

I would like to work on this

[gsc2001]

I have few doubts regarding this.

• What are the category ids exactly. Like category id for movies is (movie or movies) ?
• What should be the database name for that paritcular category? Same as category_id?
• Sending a random string in place of category_id in add_item_route does not result in any error. Is it ok?
• Sending a valid response for movies to add item route with category_id of shows gives no error. Is it ok ?

The last two can be taken care if implementation of front_end is done correctly. If that is the case then last two are not needed

[Pk13055]

• the category IDs are:
  • movies
  • shows
  • anime
  • music
  • books
• The database name(s) should be same as category_id
• It's not OK, it has to return an error. You can do something like

  
  from ..models.base import CategoryEnum

if category_id not in CategoryEnum: return ResponseBase(success=False, error=["Invalid category"]) else:

add item to category_id-named db

- No it is not OK, you have to enfore that.
```python
from ..models.base import CategoryEnum
if Movie.category_id != CategoryEnum.movies:
    return ResponseBase(success=False, data=...)
else:
   # add item successfully

No, frontend just implements basic validation. At the end of the day, it is the responsibility of the backend to ensure incorrect data does not end up in the database.

[gsc2001]

Ok. For the last one we dont have any category_id field in our request model, so i manually check for all the required corresponding to a particular field which i think is not a good way of doing that or we can add a category_id field in the request body.

[Pk13055]

The controller for the add_item_route is as follows:

@router.post("/{category_id}", response_model=ItemInResponse, dependencies=[Depends(verify_token)], tags=["add"])
async def add_item_route(category_id: str, item: Union[Movie, Anime, Show, Music],
                         db: AsyncIOMotorClient = Depends(get_database)) -> ItemInResponse:
    """Add item to a given category"""
    # TODO add item to db
    return {"success": True}

The url itself has the parameter {category_id} so you can just use that, that's why it's not part of the request model.

[gsc2001]

Also by the method you told it will return a 200 OK status. Which i dont think is nice. So we can follow something like this from the documention to restrict possible paths. I have verified this for one path it works very well, even in the documentation.

Also we can refer this for setting status codes

[gsc2001]

The controller for the add_item_route is as follows:

@router.post("/{category_id}", response_model=ItemInResponse, dependencies=[Depends(verify_token)], tags=["add"])
async def add_item_route(category_id: str, item: Union[Movie, Anime, Show, Music],
                         db: AsyncIOMotorClient = Depends(get_database)) -> ItemInResponse:
    """Add item to a given category"""
    # TODO add item to db
    return {"success": True}

The url itself has the parameter {category_id} so you can just use that, that's why it's not part of the request model.

Basically i am saying that if I grab category_id from url i need to manually check for each required component for each category_id . That is if the url is for movies, and request is of the form shows i need to verify from the request body that the object structure is of the type mentioned in the url

[Pk13055]

As I had mentioned earlier:

from ..models.base import CategoryEnum
if category_id not in CategoryEnum:
    return ResponseBase(success=False, data=...)
else:
   # here just add a simple if/else
   if category_id == CategoryEnum.movies:
      item = Movie(data)
   # similarly for all else
   # finally add to db
   _res = await db[category_id]["data"].insert_one(item.dict())
   return ItemInResponse(data=item)

PS: Refer to this for details on models and validation

[gsc2001]

There are some problems in this. First setting data to elipsis (...) is giving this error.

Which i fixed by not passing any value in data. But the MAJOR problem is that the route expects ItemInResponse model which has data required, but we are sending ResponseBase which has no data, therefore giving this error.

I tried sending an empty dict with data but that didn't work too ( i expected this ) as it requires different fields required. Resulting into this.

Now i am stuck as i dont know how to send error back. I read the link you send but coudn't take out something fruitful.

The solution for this i think is that we validate category_id the way i told earlier using Enum class

[kjain1810]

from ..models.base import CategoryEnum

if category_id not in CategoryEnum:
    return ResponseBase(success=False, error=["Invalid category"])
else:
    # add item to `category_id`-named db

@gsc2001 is this working for you? The if condition always returns true for me as checking in CategoryEnum gives me objects of type CategoryEnum.movies , etc. How did you access the values in them?

[gsc2001]

@kjain1810 Yeah I realized that just now. The if condition is also not working. I tried fixing it by casting casting_id to CategoryEnum like this

# just cast category_id to CategoryEnum
category_id = CategoryEnum(category_id)
if category_id not in CategoryEnum:
# now normal

But there is a problem in this also as if category_id is not a CategoryEnum it will result into error like this

All these problems can be fixed if use the method as told in documentation, which I told earlier

[kjain1810]

@gsc2001 doesn't that just hard codes the different categories that we have?That'll have issues if we want to expand our category list in the future. There must be a better way for this.

[gsc2001]

@kjain1810 are you referring to method in the documentation or in which i told. The method which is in the documentation is very good we can easily add more categories to it without any hassle by adding then in the CategoryEnum class.

And also we would be needing to hardcode all the categories in one place as we need to create different databases and schemas for each category

[kjain1810]

@gsc2001 I am unable to understand how the method provided in these documentations would provide us an easy way to add more categories in the future. However, I tried a couple of methods and found this to be the easiest one:

    for x in CategoryEnum:
        if category_id == x.value:
            # do what you have to
    return ItemsInResponse(success = False, error = ["Invalid category!"])

[Pk13055]

Check this out https://stackoverflow.com/a/29795561

Basically you can do:

if category_id not in CategoryEnum.__members__:
    # raise an error
else:
    # do whatever required

Also for future reference, the code I suggest will probably not work as it’s just meant to illustrate the ideas. Always Google first before trying to implement.

[Pk13055]

@gsc2001 I am unable to understand how the method provided in these documentations would provide us an easy way to add more categories in the future. However, I tried a couple of methods and found this to be the easiest one:

    for x in CategoryEnum:

        if category_id == x.value:

            # do what you have to

    return ItemsInResponse(success = False, error = ["Invalid category!"])

This is bad practice since a loop is run each time And you need it to be category agnostic

[gsc2001]

So i have figured this out we can send error by raising HTTPException and setting status codes. but initially even if i set 404 status code it sent 200. This could be rectified just by adding status_code in JSONResponse in HTTPException handeler. and convert it to something like this.

async def http_error_handler(request: Request, exc: HTTPException) -> JSONResponse:
    return JSONResponse({"errors": [exc.detail]}, status_code=exc.status_code)

and raising the HTTPException like this:-

if category_id not in CategoryEnum.__members__:
        raise HTTPException(status_code=404, detail="Category ID not found")

And all of this works pretty well.

[gsc2001]

Bson is not able to encode Set of typing. Its showing error as follows What can be done . I searched on google and found this but this requires a lot of changes. And also why cant we use simple List in place of Set?

[Pk13055]

Okay best option is convert it to a list and use $addToSet instead of $push

[gsc2001]

You mean to use these these while updating ?

[Pk13055]

Yes, $addToSet will avoid adding new genres if they already exist in the array