Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding UserMachineModel #58

Merged

Conversation

svensevenslow
Copy link
Contributor

Closes: #17

Changes Made

  1. Added UserMachine Model
  2. Made relevant changes for when a new user registers or new machine is created
  3. Updated the create_db files
  4. Made changes for the comments left in src/FlaskRTBCTF/ctf/routes.py

Other Information

  1. Running flake8 src/ --max-line-length=88 --show-source --statistics is throwing errors for src/FlaskRTBCTF/utils/init.py, which I assume can be ignored
  2. There is a typo in flake8 src/ --max-line-length=88 --show-source --statistics in CONTRIBUTING.md, I have corrected it for this branch atleast
  3. I have not utilised the get_all method in Machine class but have instead used query.all() directly wherever I need it. If the get_all method is preferred then I will use that and also add it to User class since it is required.

Let me know what you think @eshaan7

@@ -43,7 +43,7 @@ $ black .
```

```bash
$ flake8 src/ ---max-line-length=88 --show-source --statistics
$ flake8 src/ --max-line-length=88 --show-source --statistics
Copy link
Member

Choose a reason for hiding this comment

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

thankyou!

@@ -15,8 +16,20 @@ class Machine(db.Model):
ip = db.Column(db.String(64), nullable=False)
hardness = db.Column(db.String, nullable=False, default="Easy")

users = db.relationship(
Copy link
Member

Choose a reason for hiding this comment

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

don't need this. It should be a one way relationship.

@@ -128,6 +124,18 @@ def new_machine():
new_machine = Machine()
form.populate_obj(new_machine)
db.session.add(new_machine)
db.session.flush()

Users = User.query.all()
Copy link
Member

Choose a reason for hiding this comment

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

don't need to create this here.

db.session.flush()

Machines = Machine.query.all()
for machine in Machines:
Copy link
Member

Choose a reason for hiding this comment

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

we don't need to initialize rows for every User & Machine. It would be created only when user submits flag, right or wrong.


Machines = Machine.query.all()
for machine in Machines:
user_machine = UserMachine(
Copy link
Member

@eshaan7 eshaan7 Apr 29, 2020

Choose a reason for hiding this comment

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

same as stated above.

@@ -56,6 +58,12 @@
isAdmin=True,
)
db.session.add(admin_user)
db.session.flush()

admin_userMachine = UserMachine(
Copy link
Member

Choose a reason for hiding this comment

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

same as stated above. No need to initialize.

@eshaan7 eshaan7 merged commit f3b5f41 into abs0lut3pwn4g3:caching-dev Apr 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants