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

All tasks Done csoc-2020-task-2#4

Open
yashjain715-creater wants to merge 2 commits into
COPS-IITBHU:masterfrom
yashjain715-creater:master
Open

All tasks Done csoc-2020-task-2#4
yashjain715-creater wants to merge 2 commits into
COPS-IITBHU:masterfrom
yashjain715-creater:master

Conversation

@yashjain715-creater

Copy link
Copy Markdown

CSoC Task 2 Submission

I have completed the following tasks

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

@krashish8 krashish8 left a comment

Copy link
Copy Markdown
Member

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 @yashjain715-creater ! Will update the points later!

Comment thread store/models.py
Comment on lines +19 to +22
class rating(models.Model):
book = models.ForeignKey(Book, related_name='book', on_delete=models.CASCADE)
rate = models.FloatField(default=0.0)
user = models.ForeignKey(User, related_name='rate', null=True, blank=True, on_delete=models.SET_NULL)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The class name shall start with a capital letter.
The rating shall be 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 thread store/urls.py
Comment on lines +12 to +13
path('login/',loginView,name="loginView"),
path('logout/',loginView,name="logoutView"),

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You have created login/ and logout/ urls both here and in authentication.

Comment on lines +26 to +34
<ul class="sidebar-nav">
{% if user.is_authenticated %}
<li>User: {{ user.username }}</li>
<li><a href="{% url 'view-loaned' %}">My Borrowed</a></li>
<li><a href="{% url 'logoutView' %}">Logout</a></li>
{% else %}
<li><a href="{% url 'loginView' %}">Login</a></li>
{% endif %}
</ul>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Always prefer to use base.html to make the code DRY.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I always use base.html for here for only two HTML files I add upper base.html

Comment thread store/views.py
Comment on lines +55 to +70
if(title and author and genre):
books = Book.objects.filter(title=title,author=author,genre=genre)
elif(title and author):
books = Book.objects.filter(title=title,author=author)
elif(title and genre):
books = Book.objects.filter(title=title,genre=genre)
elif(author and genre):
books = Book.objects.filter(author=author,genre=genre)
elif(title):
books = Book.objects.filter(title=title)
elif(author):
books = Book.objects.filter(author=author)
elif(genre):
books = Book.objects.filter(genre=genre)
else:
books = Book.objects.all()

@krashish8 krashish8 May 9, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You could have done this in a simpler way.
Also, you should have used icontains here instead of an exact match during search.

Comment thread store/views.py
book_id = None # get the book id from post data

book_id = request.POST.get("bid") # get the book id from post data
book = Book.objects.get(id=book_id)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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.

Comment thread store/views.py
Comment on lines 134 to +146
def returnBookView(request):
pass
book_id = request.POST.get('bid')
print(book_id)
book = BookCopy.objects.get(id=book_id)
user = User.objects.get(username = request.user.username)
if(book!=0):
book.borrower = None
book.status = True
book.borrow_date = None
book.save()
response_data = {
'message': 'success',
}

Copy link
Copy Markdown
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 thread store/views.py
Comment on lines +159 to +166
rate = request.POST.get('rate')
bid = request.POST.get('bid')
book = Book.objects.get(id = bid)
user = User.objects.get(username = request.user.username)
rateuser = rating()
rateuser.user = user
rateuser.rate = rate
rateuser.book = book

Copy link
Copy Markdown
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.

Comment thread store/views.py
Comment on lines +168 to +169
availableRateUser.delete()
rateuser.save()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You could have updated the rating rather than deleting and then saving it.

@krashish8

Copy link
Copy Markdown
Member

Points updated! 🎉

@krashish8 krashish8 added the Judged The Pull Requests which are judged label May 9, 2020
@krashish8 krashish8 mentioned this pull request May 9, 2020
4 tasks
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.

3 participants