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

Solution #862

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Solution #862

wants to merge 2 commits into from

Conversation

CrazyHilly
Copy link

No description provided.

Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

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

Great job on implementing the task requirements! 🎉 The code is mostly correct and functional. Just make sure to verify that the driver-detail URL resolves correctly using the appropriate primary key, and double-check that the num_visits variable behaves as expected on the first visit. Also, ensure that ignoring database files like db.sqlite3 and identifier.sqlite is intentional and doesn't lead to any data loss. Keep up the good work! 😊

✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨

@@ -7,3 +7,6 @@ venv/
.venv/
.pytest_cache/
**__pycache__/
db.sqlite3

Choose a reason for hiding this comment

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

Including db.sqlite3 in the .gitignore file is common practice to avoid committing local database files to the repository. Ensure this is intentional and that no important data is lost by ignoring this file.

@@ -7,3 +7,6 @@
.venv/
.pytest_cache/
**__pycache__/
db.sqlite3
identifier.sqlite

Choose a reason for hiding this comment

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

Similar to db.sqlite3, the identifier.sqlite file is also being ignored. Verify that this file does not contain any crucial data that should be version-controlled.

def index(request):
"""View function for the home page of the site."""

num_drivers = Driver.objects.count()
num_cars = Car.objects.count()
num_manufacturers = Manufacturer.objects.count()
num_visits = request.session.get("num_visits", 1)

Choose a reason for hiding this comment

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

Ensure that the num_visits variable works as expected. According to the checklist, when you visit the page for the first time, it should display: You have visited this page 1 time.

<li><a href="{% url "taxi:car-list" %}">Cars</a></li>
<li><a href="{% url "taxi:driver-list" %}">Drivers</a></li>
{% if user.is_authenticated %}
<li>Hi, <a href="{% url 'taxi:driver-detail' pk=user.id %}">{{ user.username }}</a>!</li>

Choose a reason for hiding this comment

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

Ensure that the driver-detail URL correctly resolves using user.id as the primary key. This is important for linking to the user's profile page as specified in the task requirements.

@@ -7,7 +7,10 @@ <h1>Driver list</h1>
{% for driver in driver_list%}
<li>
<a href="{% url "taxi:driver-detail" pk=driver.id %}">{{ driver.username }}</a>

Choose a reason for hiding this comment

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

Ensure that the driver-detail URL correctly resolves using driver.id as the primary key. This is important for linking to the driver's profile page as specified in the task requirements.

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