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

Completed all stages#1

Open
subodhk01 wants to merge 6 commits into
COPS-IITBHU:masterfrom
subodhk01:master
Open

Completed all stages#1
subodhk01 wants to merge 6 commits into
COPS-IITBHU:masterfrom
subodhk01:master

Conversation

@subodhk01

Copy link
Copy Markdown
Member

CSoC Task 2 Submission

I have completed the following tasks

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

@nishantwrp nishantwrp temporarily deployed to csoc-task2-master-78epvpe7hc5k May 1, 2020 11:22 Inactive
@nishantwrp

Copy link
Copy Markdown
Member

Hi @subodhk01, Your deployment failed, Please fix the issues.

@nishantwrp

nishantwrp commented May 1, 2020

Copy link
Copy Markdown
Member

Here is the log if it helps

           from jsonfield import JSONField

       ModuleNotFoundError: No module named 'jsonfield'

Make sure you are using pipenv to manage your dependencies.

@nishantwrp nishantwrp temporarily deployed to csoc-task2-master-g5gmsonuvuhi May 1, 2020 11:44 Inactive
@subodhk01

Copy link
Copy Markdown
Member Author

@nishantwrp What about new migrations?

@nishantwrp nishantwrp temporarily deployed to csoc-task2-master-g5gmsonuvuhi May 6, 2020 13:40 Inactive
@nishantwrp nishantwrp temporarily deployed to csoc-task2-master-g5gmsonuvuhi May 6, 2020 18:52 Inactive
@nishantwrp nishantwrp temporarily deployed to csoc-task2-master-g5gmsonuvuhi May 6, 2020 20:57 Inactive

@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 @subodhk01! I'll update the points later!

Comment thread authentication/views.py
Comment on lines +11 to +12
username = request.POST['username']
password = request.POST['password']

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 directly access 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 thread authentication/views.py
password1 = request.POST['password1']
password2 = request.POST['password2']
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/models.py
Comment on lines 11 to +12
rating = models.FloatField(default=0.0)
user_ratings = JSONField()

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.

Nice use of JSONField()!
Since the model fields rating and user_rating are linked with each other, you could have used a @property decorator, so that the rating field would even update if the user_ratings is updated from the backend.

Comment thread store/views.py
Comment on lines +100 to +106
bid = request.POST['bid']
book = BookCopy.objects.get(pk=bid)
book.status = True
book.borrower = None
book.borrow_date = None
book.save()
return JsonResponse( {"message":"Book successfully returned."} )

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
def rateBookview(request, bid):
if request.method == "POST":
book = Book.objects.get(pk=bid)
rating = request.POST['rating']

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 this value, so the user can simply edit the JS code you've written in the template and easily put invalid values of rating.

Comment thread store/views.py
@login_required
def rateBookview(request, bid):
if request.method == "POST":
book = 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 also 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.

@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