All task completed#6
Conversation
krashish8
left a comment
krashish8
left a comment
There was a problem hiding this comment.
Great work on the assignment @aishwary023 ! Will update the points later!
| if request.method == 'GET': | ||
| return render(request,'authentication/login.html',{'form':AuthenticationForm()}) | ||
| else: | ||
| user = authenticate(request,username=request.POST['username'],password=request.POST['password']) |
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).
| class UserRating(models.Model): | ||
| user=models.ForeignKey(User, related_name='user', null=True, blank=True, on_delete=models.SET_NULL) | ||
| book = models.ForeignKey(Book, on_delete=models.CASCADE) | ||
| rating = models.FloatField(default=0) |
There was a problem hiding this comment.
The rating shall be given as an integer - please read proper instructions.
The user should not be null here, and a better option would be to use on_delete=models.CASCADE
You could have also used unique_together META option here.
| book = get_object_or_404(Book,pk=bid) | ||
| bookcopy = get_list_or_404(BookCopy, book=bid, status=True) |
There was a problem hiding this comment.
Even if the bookcopy does not exist, the book detail can be viewed by the user. Only he cannot loan the book.
| def returnBookView(request): | ||
| pass | ||
| response_data = { | ||
| 'message': None, | ||
| } | ||
|
|
||
| data = request.POST | ||
| if request.method=='POST': | ||
| bid = data.get('bid','') | ||
| book_id = bid | ||
|
|
||
| print ("CONSOLE LOG") | ||
| bookcopy = BookCopy.objects.filter(pk=book_id) | ||
|
|
||
| if len(bookcopy)==0: | ||
| response_data['message'] = 'failure' | ||
| else: | ||
| bookcopy[0].borrower = None | ||
| bookcopy[0].borrow_date = None | ||
| bookcopy[0].status = True | ||
| bookcopy[0].save() | ||
| response_data['message'] = 'success' |
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.
| rate=data.get('rate',0.0) | ||
| print(bid) | ||
| print(rate) | ||
| book = Book.objects.get(pk=bid) | ||
| oldRating=UserRating.objects.filter(user=request.user,book=book) | ||
| rating=UserRating() | ||
| rating.book=book | ||
| rating.user=request.user | ||
| rating.rating=rate | ||
| oldRating.delete() | ||
| rating.save() |
There was a problem hiding this comment.
You've not put a backend validation on the rating, so the user can easily put invalid values of rating.
You could have updated the rating rather than deleting and then saving it.
There was a problem hiding this comment.
But user only has 1-10 option of rating in a dropdown menu? Am I getting this wrong?
|
Also, make sure to run |
|
Points updated! 🎉 |
|
About the originality points, the initial parts of your |
Hi regarding that I think we both used https://simpleisbetterthancomplex.com/tutorial/2017/02/18/how-to-create-user-sign-up-view.html as reference. I did not use his function. |
|
Okay, just make sure not to copy and paste the exact code. You both have done the same. First, study the tutorial completely, and then do the coding part yourself. I have updated the score! |
2 participants
CSoC Task 2 Submission
I have completed the following tasks