Skip to content

Ada C24 Crow Tatiana D.#40

Open
FluenceMind wants to merge 16 commits intoAda-C24:mainfrom
FluenceMind:main
Open

Ada C24 Crow Tatiana D.#40
FluenceMind wants to merge 16 commits intoAda-C24:mainfrom
FluenceMind:main

Conversation

@FluenceMind
Copy link
Copy Markdown

No description provided.

This PR implements Wave 5 functionality for the Task List API.

✔ Added `Goal` model with `to_dict()` and `from_dict()`
✔ Implemented full CRUD routes for `/goals`
✔ Introduced shared helpers: `validate_model` and `create_from_dict_or_400`
✔ Ensured correct error bodies for both Goal and Task
✔ All Wave 5 tests passing (15/15)
Wave 6: goal-task relationship, CRUD updates, all tests passing
Wave 7: Refactor routes, add route utilities, deploy preparation
Copy link
Copy Markdown

@mikellewade mikellewade left a comment

Choose a reason for hiding this comment

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

@FluenceMind, there are a number of refactors you need to make on this project. In general, your syntax for most of the project is older syntax and you need to update it. I left a number of comments directing you to the necessary refactors. Once you've made them please reach out to me via Slack DM to get your grade changed from a Red to a Green.

Comment thread app/__init__.py Outdated
Comment on lines +20 to +24
from app.models.task import Task
from app.models.goal import Goal

from app.routes.task_routes import bp as tasks_bp
from app.routes.goal_routes import goal_bp
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@FluenceMind, I'm not sure as to why these imports were moved to here. I think that they are better served being at the top of the file.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Absolutely - I agree! I'm not sure how those imports ended up down there… they must have gone on a little adventure. I’ll move them back to the top of the file.

Comment thread app/__init__.py
Comment on lines +26 to +27
app.register_blueprint(tasks_bp)
app.register_blueprint(goal_bp)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Comment thread app/models/goal.py Outdated
Comment on lines +4 to +7
id = db.Column(db.Integer, primary_key=True)
title = db.Column(db.String, nullable=False)

tasks = db.relationship("Task", back_populates="goal")
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Perfect! You are making a relationship attribute on the Goal model. This attribute is going to be a list of Task models. You then use relationship with back_populates to tell SQLAlchemy to sync this attribute with relationship attribute called goal on the Task model.

Comment thread app/models/goal.py Outdated
Comment on lines +4 to +5
id = db.Column(db.Integer, primary_key=True)
title = db.Column(db.String, nullable=False)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@FluenceMind, this is old syntax for establishing the columns on our models. You will need to use the current/one demonstrated in class to establish the relationship: https://docs.sqlalchemy.org/en/20/orm/declarative_styles.html#using-a-declarative-base-class

Please refactor this code with the required changes. Once updated, please let me know via Slack DM.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I refactored this using the updated declarative style.

Comment thread app/models/task.py Outdated
Comment on lines +5 to +10
id = db.Column(db.Integer, primary_key=True)
title = db.Column(db.String, nullable=False)
description = db.Column(db.String, nullable=True)
completed_at = db.Column(db.DateTime, nullable=True)

goal_id = db.Column(db.Integer, db.ForeignKey("goal.id"), nullable=True)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@FluenceMind, this is old syntax for establishing the columns on our models. You will need to use the current/one demonstrated in class to establish the relationship: https://docs.sqlalchemy.org/en/20/orm/declarative_styles.html#using-a-declarative-base-class

Please refactor this code with the required changes. Once updated, please let me know via Slack DM.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I refactored this using the updated declarative style.

Comment thread tests/test_wave_01.py
Comment on lines +212 to +213
assert response.is_json
assert response_body == {"details": "Task 1 not found"}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Comment thread tests/test_wave_01.py
Comment on lines +234 to +235
assert response.is_json
assert response_body == {"details": "Task 1 not found"}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Comment thread tests/test_wave_05.py Outdated
assert response.status_code == 204

get_response = client.get("/goals/1")
get_body = get_response.get_json()
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Instead of checking this way, you should leverage the db query, this is so your test won't also be dependent on the GET and if it is implemented correctly.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Updated the test

Comment thread tests/test_wave_07.py
Comment on lines +46 to +50
response = e.value.get_response()
assert response.status_code == 404

response_body = response.get_json()
assert response_body == {"details": "Task 4 not found"}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Comment thread tests/test_wave_07.py
Comment on lines +144 to +148
response = e.value.get_response()
assert response.status_code == 400

response_body = response.get_json()
assert response_body == {"details": "Invalid data"}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants