Amethyst - Abby Goodman #129

argoodm · 2023-05-18T00:22:22Z

No description provided.

…t constriant

…ete dependcies

ameerrah9 · 2023-05-22T22:01:29Z

app/models/goal.py

 class Goal(db.Model):
-    goal_id = db.Column(db.Integer, primary_key=True)
+    goal_id = db.Column(db.Integer, primary_key=True, autoincrement=True)
+    title = db.Column(db.String)
+    tasks = db.relationship("Task", back_populates="goal", lazy=True)


Great job completing Task List Abby! Your code looks clean and easily readable. I want to point out the good variable naming and your code is DRY. Great use of helper methods in your models! Excellent work using blue prints & creating RESTful CRUD routes for each model.

ameerrah9 · 2023-05-22T22:02:24Z

app/models/goal.py

+    def from_dict(cls, goal_data):
+        new_goal = Goal(
+            title=goal_data["title"]
+        )
+
+        return new_goal


I noticed you created this class method but didn't make use of it

ameerrah9 · 2023-05-22T22:03:25Z

app/models/task.py

 class Task(db.Model):
-    task_id = db.Column(db.Integer, primary_key=True)
+    task_id = db.Column(db.Integer, primary_key=True, autoincrement=True)
+    title = db.Column(db.String) #when refactoring change to varchar
+    description = db.Column(db.String) # when refacoring change to varvhar
+    completed_at = db.Column(db.DateTime, default=None, nullable=True) 
+    goal_id = db.Column(db.Integer, db.ForeignKey('goal.goal_id'))
+    goal = db.relationship("Goal", back_populates="tasks")


Nice approach creating this relationship to your Goal model. Why not use lazy here?

ameerrah9 · 2023-05-22T22:04:08Z

app/models/task.py

+    def from_dict(cls, task_data):
+        new_task = Task(
+            title=task_data["title"],
+            description=task_data["description"],
+            completed_at=task_data["completed_at"]
+        )
+
+        return new_task


I noticed you created this class method but didn't make use of it

ameerrah9 · 2023-05-22T22:04:26Z

app/models/task.py

+            "id":self.task_id,
+            "title":self.title,
+            "description":self.description,
+            "is_complete": True if self.completed_at else False}}


ameerrah9 · 2023-05-22T22:05:06Z

app/__init__.py

+    from .routes import task_list_bp
+    app.register_blueprint(task_list_bp)
+    from .routes import goals_bp
+    app.register_blueprint(goals_bp)


Great work registering these blueprints ✅

ameerrah9 · 2023-05-22T22:05:15Z

app/__init__.py

@@ -15,8 +15,10 @@ def create_app(test_config=None):
    app.config["SQLALCHEMY_TRACK_MODIFICATIONS"] = False

    if test_config is None:
-        app.config["SQLALCHEMY_DATABASE_URI"] = os.environ.get(
-            "SQLALCHEMY_DATABASE_URI")
+        app.config["SQLALCHEMY_DATABASE_URI"] = os.environ.get("RENDER_DB_URI")


ameerrah9 · 2023-05-22T22:10:41Z

app/__init__.py

@@ -30,5 +32,9 @@ def create_app(test_config=None):
    migrate.init_app(app, db)


Great job! We need to pass our instance of our app & the instance of our SQLALchemy db to connect the db and migrate to our Flask app

ameerrah9 · 2023-05-22T22:14:52Z

app/routes.py

+import requests
+import json
+from dotenv import load_dotenv


Great work! We need to import load_dotenv to set up environment variables in our .env file

ameerrah9 · 2023-05-22T22:15:29Z

app/routes.py

+from app.models.task import Task
+from app.models.goal import Goal
+from flask import Blueprint, jsonify, make_response, request, abort


👍🏾 These imports help a great deal when it comes to our request & response cycle

ameerrah9 · 2023-05-22T22:16:30Z

app/routes.py

+    try:
+        task_id = int(task_id)
+    except:
+        abort(make_response({"message": f"Task {task_id} invalid"}, 400))
+    task = Task.query.get(task_id)
+    if not task:
+        abort(make_response({"details": "Invalid Data"}, 404))
+    return task


Nice approach to handling an invalid task 😁

ameerrah9 · 2023-05-22T22:17:14Z

app/routes.py

+    try:
+        goal_id = int(goal_id)
+    except:
+        abort(make_response({"message": f"Goal {goal_id} invalid"}, 400))
+    goal = Goal.query.get(goal_id)
+    if not goal:
+        abort(make_response({"details": "Invalid Data"}, 404))
+    return goal


Nice approach to handling an invalid goal 😁 There's a bunch of repeated code here, any ideas of how to refactor?

ameerrah9 · 2023-05-22T22:18:06Z

app/routes.py

+        title = request_body["title"],
+        description = request_body["description"],
+        # completed_at = request_body["completed_at"]


It’s good practice to avoid committing commented out code when submitting a PR.

ameerrah9 · 2023-05-22T22:21:36Z

app/routes.py

Great job using db.session.<method-name>. Session gives us access to the follow:

db is our app’s instance of SQLAlchemy.

session represents our active database connection.

By referencing db.session we can use SQLAlchemy’s methods to perform tasks like:

committing a change to a model

storing a new record of a model

deleting a record.

By referencing db.session.add() you are able to use the SQLAlchemy method to store a new record of the task model

ameerrah9 · 2023-05-22T22:27:29Z

app/routes.py

+    task = validate_task(task_id)
+    task.completed_at = datetime.now()
+    #slack implementation
+    url = "https://slack.com/api/chat.postMessage"
+    payload = json.dumps({
+    "channel": "C0581AUJACV",
+    "text": (f"Someone just completed the task {task.title}")
+    })
+    headers = {
+    'Authorization': os.environ.get("SLACK_API_TOKEN"),
+    'Content-Type': 'application/json'
+    }
+    response = requests.request("POST", url, headers=headers, data=payload)
+    print(response.text)
+    db.session.commit()
+    return task.to_dict(), 200


This function is pretty packed, for readability I suggest using more spacing in between lines. Something like this:

def update_task_to_complete(task_id): task = validate_task(task_id) task.completed_at = datetime.now() #slack implementation url = "https://slack.com/api/chat.postMessage" payload = json.dumps({ "channel": "C0581AUJACV", "text": (f"Someone just completed the task {task.title}") }) headers = { 'Authorization': os.environ.get("SLACK_API_TOKEN"), 'Content-Type': 'application/json' } response = requests.request("POST", url, headers=headers, data=payload) print(response.text) db.session.commit() return task.to_dict(), 200

argoodm added 13 commits May 14, 2023 10:22

added post route and also recreated database with correct completed a…

b45b8ae

…t constriant

get all tasks complete and also figured out completed_at and is_compl…

57c5306

…ete dependcies

completed GET portion of CRUD

955f257

tests completed and CRUD completed

21836be

wave 2 completed, fixed wave 1 errors

ecd3436

mark_complete endpoint done

2102e20

tests updates, goals added, wave 3 no bugs

3573193

fixed tests wave 5

115ffc1

Slack api implementation

7494df7

fix model errors

41f4466

cleaned up test file comments

1b08b34

All passing! All done

fefdf05

render_db_attachement

ea1d5bd

ameerrah9 reviewed May 22, 2023

View reviewed changes

ameerrah9 changed the title ~~pull request~~ Amethyst - Abby Goodman May 22, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Amethyst - Abby Goodman #129

Amethyst - Abby Goodman #129

argoodm commented May 18, 2023

ameerrah9 May 22, 2023

ameerrah9 May 22, 2023

ameerrah9 May 22, 2023

ameerrah9 May 22, 2023

ameerrah9 May 22, 2023

ameerrah9 May 22, 2023

ameerrah9 May 22, 2023

ameerrah9 May 22, 2023

ameerrah9 May 22, 2023

ameerrah9 May 22, 2023

ameerrah9 May 22, 2023

ameerrah9 May 22, 2023

ameerrah9 May 22, 2023

ameerrah9 May 22, 2023

ameerrah9 May 22, 2023

		@@ -30,5 +32,9 @@ def create_app(test_config=None):
		migrate.init_app(app, db)

Amethyst - Abby Goodman #129

Are you sure you want to change the base?

Amethyst - Abby Goodman #129

Conversation

argoodm commented May 18, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment