ALL Tasks Done #18
ALL Tasks Done #18
Conversation
There was a problem hiding this comment.
Great work on the assignment! @hex-plex 👍
I see that you've made many errors in the assignment. However, it is good that you have tried. Now, see other's submissions and try to complete the assignment. Also, check how you could have done better.
| class User(models.Model): | ||
| username = models.CharField(max_length=50) | ||
| email = db.Column(db.String(120),index=True,unique=True) | ||
| password_hash = db.Column(db.String(128)) | ||
| posts = db.relationship('Post', backref='author', lazy='dynamic') | ||
| about_me=db.Column(db.String(140)) | ||
| last_seen = db.Column(db.DateTime, default = datetime.utcnow) | ||
| def __repr__(self): | ||
| return '<User '+self.username+'>' | ||
| def set_password(self,password): | ||
| self.password_hash=generate_password_hash(password) | ||
| def check_password(self,password): | ||
| return check_password_hash(self.password_hash,password) | ||
| def avatar(self,size): | ||
| digest = md5(self.email.lower().encode("utf-8")).hexdigest() | ||
| return 'https://www.gravatar.com/avatar/{}?d=identicon&s={}'.format(digest,size) |
There was a problem hiding this comment.
There isn't any use of creating this User model here. And you aren't required to include those extra field.
| q=Q() | ||
| if 'title' in get_data.keys(): | ||
| if get_data['title']!='': | ||
| q |=Q(title__iexact=get_data['title']) | ||
| if get_data['author']!='': | ||
| q |=Q(author__iexact=get_data['author']) | ||
| if get_data['genre']!='': | ||
| q |=Q(genre__iexact=get_data['genre']) | ||
| context['books']=Book.objects.filter(q) | ||
| return render(request, template_name, context=context) |
There was a problem hiding this comment.
You should display all the books here, and filter them only when you get GET request with the parameters. Also, while filtering, you should have used icontains for a case-insensitive match, instead of an exact match.
| return render(request,template_name) | ||
| else: | ||
| get_data=request.POST | ||
| user = authenticate( username=get_data['username'], password=get_data['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).
| self.rating= sum([i[1] for i in self.ratings])/len(self.ratings) | ||
| self.save() | ||
| return 1 | ||
| except: |
There was a problem hiding this comment.
As a good coding practice, whenever you use try-except block, capture only the exceptions which you want to catch (IndexError, IntegrityError, etc.)
| ratings = [] | ||
| class Meta: | ||
| ordering = ('title',) | ||
| def checkUser(self,user): | ||
| ind=0 | ||
| for i in self.ratings: | ||
| if(i[0]==user): |
There was a problem hiding this comment.
There's an error with your rating. You should have created another model here to store the Book, the User and the rating given by that user. And then to calculate the average rating of a book, you could have filtered that model to get the corresponding book and the user, and then calculated the average rating, and stored it in the rating field of the Book model.
Currently, when multiple users rate a book, the average rating of the book becomes equal to the last user who has rated it.
There was a problem hiding this comment.
No that is not the case i have used an array for storing them with the username and have checked it also
There was a problem hiding this comment.
Yeah, you've used array but the array can't be stored anywhere in the database. The array will not persist when you close the server and then re-start it. So, after you restart the Django server, all the user ratings for a book would be gone, and then when a user will rate the book, it will be considered as if the rating is inserted in an empty array.
An array isn't persistent. There's no proper field like an array to store data in models. For that, we create another Model using some ForeignKey like relation.
As an example, check the admin panel, you won't find the array to be stored anywhere.
| 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.
| ratinginp=int(ratinginp) | ||
| ratinginp=max(0,ratinginp) | ||
| ratinginp=min(10,ratinginp) |
There was a problem hiding this comment.
You should not adjust the value of rating to lie between 0 and 10. Rather you should alert the user in case he rates the book beyond the allowed range.
|
Points have been updated! 🎉 |
CSoC Task 2 Submission
I have completed the following tasks