Skip to content
This repository has been archived by the owner on Jun 16, 2021. It is now read-only.

All task completed #6

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Conversation

aishwary023
Copy link
Member

@aishwary023 aishwary023 commented May 6, 2020

CSoC Task 2 Submission

I have completed the following tasks

  • Stage 1
  • Stage 2
  • Stage 3
  • Stage 4

Copy link
Member

@krashish8 krashish8 left a comment

Choose a reason for hiding this comment

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

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'])
Copy link
Member

Choose a reason for hiding this comment

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

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

Comment on lines +20 to +23
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)
Copy link
Member

Choose a reason for hiding this comment

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

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.

Comment on lines +22 to +23
book = get_object_or_404(Book,pk=bid)
bookcopy = get_list_or_404(BookCopy, book=bid, status=True)
Copy link
Member

Choose a reason for hiding this comment

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

Even if the bookcopy does not exist, the book detail can be viewed by the user. Only he cannot loan the book.

Comment on lines 84 to +104
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'
Copy link
Member

Choose a reason for hiding this comment

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

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.

Comment on lines +115 to +125
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()
Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Member Author

Choose a reason for hiding this comment

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

But user only has 1-10 option of rating in a dropdown menu? Am I getting this wrong?

@krashish8
Copy link
Member

Also, make sure to run python manage.py makemigrations whenever you make any changes to the model.

@krashish8
Copy link
Member

Points updated! 🎉

@krashish8 krashish8 added the Judged The Pull Requests which are judged label May 9, 2020
@krashish8
Copy link
Member

krashish8 commented May 9, 2020

About the originality points, the initial parts of your registerView() function and the SignUpForm() is matching with PR #4.

@aishwary023
Copy link
Member Author

About the originality points, the initial parts of your registerView() function is matching with PR #4.

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.

@krashish8
Copy link
Member

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!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Judged The Pull Requests which are judged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants