All Task Completed with added features #11
Conversation
| class Profile(models.Model): | ||
| user = models.OneToOneField(User, on_delete=models.CASCADE) | ||
| first_name = models.CharField(max_length=100, blank=True) | ||
| last_name = models.CharField(max_length=100, blank=True) | ||
| email = models.EmailField(max_length=150) | ||
|
|
||
| def __str__(self): | ||
| return self.user.username | ||
|
|
||
|
|
||
| @receiver(post_save, sender=User) | ||
| def update_profile_signal(sender, instance, created, **kwargs): | ||
| if created: | ||
| Profile.objects.create(user=instance) | ||
| instance.profile.save() |
There was a problem hiding this comment.
Good use of receiver decorator! 👍
However, one should create an extra model only if required. Here, the user model is sufficient to store these details.
| user = form.save() | ||
| user.refresh_from_db() |
| TIME_ZONE = 'UTC' | ||
| TIME_ZONE = 'Asia/Kolkata' |
There was a problem hiding this comment.
It's good that you've modified the TIME_ZONE to IST.
| class BookRating(models.Model): | ||
| book = models.ForeignKey(Book, on_delete=models.CASCADE) | ||
| username = models.ForeignKey(User, on_delete=models.CASCADE) | ||
| rating = models.IntegerField( | ||
| default=1, validators=[MaxValueValidator(10), MinValueValidator(0)]) |
There was a problem hiding this comment.
You could've used unique_together META option here. However, its fine for this assignment.
| document.querySelectorAll('input').forEach((i) => { | ||
| i.addEventListener('keyup', (event) => { | ||
| if (event.keyCode === 13) { | ||
| event.preventDefault(); | ||
| document.querySelector("#search_button").click(); | ||
| } | ||
| }) | ||
| }) |
| book = get_object_or_404(Book, pk=bid) | ||
| book.rating = BookRating.objects.filter( | ||
| book=book).aggregate(rating=Avg('rating'))['rating'] |
There was a problem hiding this comment.
Good to have used aggregation to calculate Avg.
store/views.py
Outdated
| bid = request.POST['bid'] | ||
| book = get_object_or_404(BookCopy, pk=bid) | ||
|
|
||
| if book: | ||
| book.borrower = None |
There was a problem hiding this comment.
During the book return, you must validate that the borrower of the book is the current user.
store/views.py
Outdated
| book = get_object_or_404(Book, pk=bid) | ||
|
|
||
| user = request.user | ||
| rating = request.POST['rating'] |
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).
|
Points updated! 🎉 |
|
@krashish8 thanks for your reviews I will make sure these issues don't occur in upcoming assignments. |
CSoC Task 2 Submission
I have completed the following tasks
Addded Features
Supports search on 'Enter' keypress.
It also deals with the user trying to log in and signup again even though already authenticated.
Deployed at: https://pacific-basin-13105.herokuapp.com/