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

Add files via upload#9

Open
arul-ashri wants to merge 3 commits into
COPS-IITBHU:masterfrom
arul-ashri:master
Open

Add files via upload#9
arul-ashri wants to merge 3 commits into
COPS-IITBHU:masterfrom
arul-ashri:master

Conversation

@arul-ashri

@arul-ashri arul-ashri commented May 6, 2020

Copy link
Copy Markdown

CSoC Task 2 Submission

I have completed the following tasks

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

@arul-ashri arul-ashri marked this pull request as ready for review May 7, 2020 18:14
@arul-ashri arul-ashri closed this May 7, 2020
@arul-ashri arul-ashri reopened this May 7, 2020

@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 @arul-ashri ! Will update the points later!

Comment thread store/models.py
Comment on lines +34 to +39
class BookRating(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)
ratinguser = models.FloatField(default=0)
def __str__(self):
return f'{self.book.title}'

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 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 thread authentication/views.py
firstname=request.POST['firstname']
lastname=request.POST['lastname']
email=request.POST['email']
except:

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.

As a good coding practice, whenever you use try-except block, capture only the exceptions which you want to catch (IndexError, IntegrityError, etc.)

Comment thread store/views.py

def bookDetailView(request, bid):
template_name = 'store/book_detail.html'
book1=Book.objects.get(pk=bid)

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 +108 to +115
book_id = request.POST.get("bid")
book=BookCopy.objects.get(id=book_id)
if(book):
message="success"
book.status=True
book.borrower=None
book.borrow_date=None
book.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.

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 +126 to +133
book_id = request.POST.get("bid")
rating_value = request.POST.get("rating")
book1=Book.objects.get(pk=book_id)
value= BookRating.objects.filter(book__exact=book1,user__exact=request.user)
if(value.count()>0):
currentuserRating= BookRating.objects.filter(book__exact=book1,user__exact=request.user).get()
currentuserRating.ratinguser=rating_value
currentuserRating.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've not put a backend validation on the rating, so the user can easily put invalid values of rating by editing the JS code.

@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 commented May 9, 2020

Copy link
Copy Markdown
Member

About the originality points, your submission resembles a lot with PR #3.
(authentication/views.py is exactly copied and a large portion of store/views.py is also matching)

@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