completed all Tasks (i am god) #12
Conversation
|
use venv server to run my file. i was facing some problems in pipenv in my device . all depedencies are given in requirements.txt |
|
Hi! Just to inform you, some portion of your PR resembles somewhat with PR #1. ( |
| rating=$("#rat").val(); | ||
|
|
||
| rating_int=parseInt(rating,10) | ||
| console.log(rating,rating_int) | ||
|
|
||
| if(rating==""||rating<1||rating>10){ | ||
| alert("Rating must be between 1-10 in integer") | ||
| } |
There was a problem hiding this comment.
Here, the value of rating is set using #rat, and for every book copy loaned, the id of the text-field is rat. Therefore, this will produce an error on rating the book. The rating stored in the 1st textbox with id #rat will be taken.
| class Rating(models.Model): | ||
| book=models.ForeignKey(Book,on_delete=models.CASCADE) | ||
| user=models.ForeignKey(User,on_delete=models.CASCADE) | ||
| stars=models.IntegerField(validators=[MinValueValidator(1),MaxValueValidator(10)]) | ||
|
|
||
|
|
||
|
|
||
| def __str__(self): | ||
| return f"{self.book} by {self.user}" |
There was a problem hiding this comment.
Could have used unique_together META option, however its fine.
| @@ -12,9 +14,14 @@ def index(request): | |||
|
|
|||
| def bookDetailView(request, bid): | |||
| template_name = 'store/book_detail.html' | |||
| book=Book.objects.get(id=bid) | |||
There was a problem hiding this comment.
This may fail with invalid book ID given in POST request, and would lead to server error. Expected behavior is to inform user with Not found (404) error.
| alpha=request.user.is_authenticated | ||
| if not alpha: | ||
| return JsonResponse({"message":"login plz"}) | ||
| book_id=request.POST['bid'] |
There was a problem hiding this comment.
You are directly accessing POST data without checking if it even exists. This may lead to server crash if a user access this endpoint with invalid request data. The good behavior would have been to throw a client error (400), rather than server error (500).
| rating01.save() | ||
| return JsonResponse({"message":"rating has been updated"}) | ||
| else: | ||
| rating01=Rating.objects.create(user=current_user,book=BookCopy.objects.get(id=book_id).book,stars=star) |
There was a problem hiding this comment.
This is not a good way to call ORM queries. This will run two queries on database.
Better, use book=book_id.
| star=request.POST["stars"] | ||
| book_id=request.POST["bid"] | ||
| current_user=request.user | ||
| rating=Rating.objects.filter(book__exact=BookCopy.objects.get(id=book_id).book,user__exact=current_user) | ||
| if rating: | ||
| rating01=rating[0] | ||
| rating01.stars=star |
There was a problem hiding this comment.
You've not put a backend validation on the rating, so the user can simply edit the JS code you've written in the template and easily put invalid values of rating.
| if request.method=="POST": | ||
| try: | ||
| book_id=request.POST['bid'] | ||
| book=BookCopy.objects.get(pk=book_id) | ||
| book.status=True | ||
| book.borrower=None | ||
| book.borrow_date=None | ||
| book.save() |
There was a problem hiding this comment.
There must be a validation in the backend when a user is returning the book, to make sure that he has only borrowed the book. Otherwise, a simple POST request will make the BookCopy to be returned, and would set its status as True.
|
Points have been updated! |
CSoC Task 2 Submission
I have completed the following tasks