This commit will resolve all issues #69

harshit-soora · 2021-05-27T11:46:59Z

Fixes #68
This commit is for the issue that I have opened

Please follow the Pictures folder in my repo here to see the new changes

kumaraditya303 · 2021-05-28T10:48:25Z

Dockerfile

@@ -1,14 +0,0 @@
-FROM python:3.9.5


Don't remove Docker Image it is useful for deployment

kumaraditya303 · 2021-05-28T10:48:42Z

LICENSE

@@ -1,674 +0,0 @@
-                    GNU GENERAL PUBLIC LICENSE


LICENSE must be present

kumaraditya303 · 2021-05-28T10:49:14Z

Library/models.py

@@ -1,6 +1,6 @@
 from flask_login import UserMixin

-from Library_Management_System import db
+from Library import db


Any reason to rename the directory structure, the earlier one was more clear

Sorry for that, you can keep the original

It was just the project folder have same name as a folder inside(that have most of code) it so I felt it a bit confusing every time I have to keep track which folder I am in actually

Library/templates/admin_layout.html

kumaraditya303 · 2021-05-28T10:50:17Z

Library/templates/admin_layout.html

+          <div valign="bottom">
+               <hr style="border: 5px solid grey;border-radius: 5px;" />
+               <footer>
+                    <p>&copy; {{ year }} - Library Management System - Harshit Soora</p>


You can add your name on your repo, let it be intact for this repo

Ohh sorry I should have removed that before committing

kumaraditya303 · 2021-05-28T10:51:08Z

Library/templates/register.html

@@ -35,6 +38,18 @@ <h1 class='display-4 text-center'>Register</h1><br>
             </div>
         </div>
     </div>
+
+     <div class="input-group mb-3">


This is not safe, it should be implemented using a cli

Library/templates/show_students.html

kumaraditya303 · 2021-05-28T10:52:29Z

Library/views.py

        flash("Invalid Credentials!")
        return redirect(url_for("main.admin"))


 @main.route("/dashboard", methods=["GET"])
 @login_required
 def dashboard():
-    copies = Copy.query.filter_by(issued_by=current_user.id).all()
+    copies = db.session.query(Book.id, Book.name, Book.author, Copy.date_issued, Copy.date_return) \


What does this changes ?

The issue book page don't show the Author name and Book name, so we need to join both the tables from Book and Copy, where we join the two tables by book_id and search by user id

kumaraditya303 · 2021-05-28T10:53:11Z

Library/views.py

+class ShowStudentView(MethodView):
+    def get(self):
+        students = User.query.filter_by(admin = False).all()
+        print(students)


When you are finished implementing remove all the print statements

kumaraditya303 · 2021-05-28T10:53:45Z

Procfile

@@ -1 +0,0 @@
-web: gunicorn wsgi


This is required to deploy on heroku

kumaraditya303 · 2021-05-28T10:54:22Z

tests/test_view.py

@@ -1,120 +0,0 @@
-from config import TestConfig


Don't remove the tests, instead you should add test for your change!

harshit-soora · 2021-05-28T10:55:22Z

Hey Aditya I just need the project to host it locally from my machine so I skipped the docker and procfile Thanks and Regards

…

On Fri, May 28, 2021 at 4:18 PM Kumar Aditya ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In Dockerfile <#69 (comment)> : > @@ -1,14 +0,0 @@ -FROM python:3.9.5 Don't remove Docker Image it is useful for deployment — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#69 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AOPKEJR3JCSAXNO6RKNON33TP5YIRANCNFSM45UC7KDA> .

-- Harshit Soora Final Year Department of Computer Science and Engineering IIT Kharagpur

kumaraditya303 · 2021-05-28T10:57:05Z

Hey Aditya I just need the project to host it locally from my machine so I skipped the docker and procfile Thanks and Regards
…
On Fri, May 28, 2021 at 4:18 PM Kumar Aditya @.> wrote: @.* commented on this pull request. ------------------------------ In Dockerfile <#69 (comment)> : > @@ -1,14 +0,0 @@ -FROM python:3.9.5 Don't remove Docker Image it is useful for deployment — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#69 (review)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AOPKEJR3JCSAXNO6RKNON33TP5YIRANCNFSM45UC7KDA .
-- Harshit Soora Final Year Department of Computer Science and Engineering IIT Kharagpur

Okay, then you can just ignore those and host it locally without docker, there isn't need to remove Dockerfile

harshit-soora · 2021-05-28T10:58:47Z

Hello Aditya In Library/views.py <#69 (comment)> :

flash("Invalid Credentials!")

return redirect(url_for("main.admin")) @main.route("/dashboard", methods=["GET"]) @login_required def dashboard(): - copies = Copy.query.filter_by(issued_by=current_user.id).all() + copies = db.session.query(Book.id, Book.name, Book.author, Copy.date_issued, Copy.date_return) \ What does this changes ? The issue book page don't show the Author name and Book name, so we need to join both the tables from Book and Copy, where we join the two tables by book_id and search by user id Thanks and Regards

…

On Fri, May 28, 2021 at 4:24 PM Kumar Aditya ***@***.***> wrote: ***@***.**** requested changes on this pull request. ------------------------------ In LICENSE <#69 (comment)> : > @@ -1,674 +0,0 @@ - GNU GENERAL PUBLIC LICENSE LICENSE must be present ------------------------------ In Library/models.py <#69 (comment)> : > @@ -1,6 +1,6 @@ from flask_login import UserMixin -from Library_Management_System import db +from Library import db Any reason to rename the directory structure, the earlier one was more clear ------------------------------ In Library/templates/admin_layout.html <#69 (comment)> : > @@ -16,7 +16,9 @@ </head> <body - style="font-family: 'PT Sans', sans-serif;background-image: url(https://www.pixelstalk.net/wp-content/uploads/2016/08/Library-Desktop-Wallpaper-768x480.jpg);background-size: cover;"> + style="font-family: 'PT Sans', sans-serif;"> +  You can better remove the background image ------------------------------ In Library/templates/admin_layout.html <#69 (comment)> : > {% block content %}{% endblock %} - <hr style="border: 5px solid white;border-radius: 5px;" /> - <footer> - <p>© {{ year }} - Library Management System - Kumar Aditya</p> - </footer> + <div valign="bottom"> + <hr style="border: 5px solid grey;border-radius: 5px;" /> + <footer> + <p>© {{ year }} - Library Management System - Harshit Soora</p> You can add your name on your repo, let it be intact for this repo ------------------------------ In Library/templates/register.html <#69 (comment)> : > @@ -35,6 +38,18 @@ <h1 class='display-4 text-center'>Register</h1><br> </div> </div> </div> + + <div class="input-group mb-3"> This is not safe, it should be implemented using a cli ------------------------------ In Library/templates/show_students.html <#69 (comment)> : > + +{% if student %} + +<div class=' text-center container-fluid'> + <h1 class='display-4 text-center'>Students</h1><br> + + <table class="table table-dark table-bordered"> + <thead> + <th scope="col">Roll No</th> + <th scope="col">Name</th> + <th scope="col">Email</th> + </thead> + <tbody> + {% for stu in student %} + <tr> + <td scope='row'>{{ stu.id }}</td> Naming it students would be better ------------------------------ In Library/views.py <#69 (comment)> : > flash("Invalid Credentials!") return redirect(url_for("main.admin")) @main.route("/dashboard", methods=["GET"]) @login_required def dashboard(): - copies = Copy.query.filter_by(issued_by=current_user.id).all() + copies = db.session.query(Book.id, Book.name, Book.author, Copy.date_issued, Copy.date_return) \ What does this changes ? ------------------------------ In Library/views.py <#69 (comment)> : > return render_template("admin_dashboard.html", year=datetime.now().year) +class ShowStudentView(MethodView): + def get(self): + students = User.query.filter_by(admin = False).all() + print(students) When you are finished implementing remove all the print statements ------------------------------ In Procfile <#69 (comment)> : > @@ -1 +0,0 @@ -web: gunicorn wsgi This is required to deploy on heroku ------------------------------ In tests/test_view.py <#69 (comment)> : > @@ -1,120 +0,0 @@ -from config import TestConfig Don't remove the tests, instead you should add test for your change! — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#69 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AOPKEJXZBNJ7HPMVUZ26HRLTP5Y7JANCNFSM45UC7KDA> .

-- Harshit Soora Final Year Department of Computer Science and Engineering IIT Kharagpur

kumaraditya303 · 2021-05-28T11:07:18Z

Okay @harshit-soora when you are done implementing then you can request for review from github and I would review, Thanks for your contribution.

This commit will resolve all issues

532d28e

kumaraditya303 self-assigned this May 28, 2021

kumaraditya303 reviewed May 28, 2021

View reviewed changes

Dockerfile

@@ -1,14 +0,0 @@

FROM python:3.9.5

Copy link

Owner

kumaraditya303 May 28, 2021

Choose a reason for hiding this comment

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

Don't remove Docker Image it is useful for deployment

kumaraditya303 requested changes May 28, 2021

View reviewed changes

Merge branch 'master' into harshit

49bbf6f

kumaraditya303 closed this Aug 7, 2021

This commit will resolve all issues #69

This commit will resolve all issues #69

Uh oh!

Conversation

harshit-soora commented May 27, 2021 • edited by kumaraditya303 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

harshit-soora commented May 28, 2021 via email

Uh oh!

kumaraditya303 commented May 28, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

harshit-soora commented May 28, 2021 via email

Uh oh!

kumaraditya303 commented May 28, 2021

Uh oh!

Uh oh!

harshit-soora commented May 27, 2021 •

edited by kumaraditya303

Loading

kumaraditya303 commented May 28, 2021 •

edited

Loading