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 #835

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 11 additions & 5 deletions taxi/views.py
Original file line number Diff line number Diff line change
@@ -1,47 +1,53 @@
from django.contrib.auth.decorators import login_required
from django.contrib.auth.mixins import LoginRequiredMixin
from django.shortcuts import render
from django.views import generic

from .models import Driver, Car, Manufacturer


@login_required
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)
request.session["num_visits"] = num_visits + 1

context = {
"num_drivers": num_drivers,
"num_cars": num_cars,
"num_manufacturers": num_manufacturers,
"num_visits": num_visits
}

return render(request, "taxi/index.html", context=context)


class ManufacturerListView(generic.ListView):
class ManufacturerListView(LoginRequiredMixin, generic.ListView):
model = Manufacturer
context_object_name = "manufacturer_list"
template_name = "taxi/manufacturer_list.html"
paginate_by = 5


class CarListView(generic.ListView):
class CarListView(LoginRequiredMixin, generic.ListView):
model = Car
paginate_by = 5
queryset = Car.objects.select_related("manufacturer")

Choose a reason for hiding this comment

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

The queryset for CarListView uses select_related with manufacturer. Ensure that this is necessary for your use case, as it can impact performance by loading related objects in a single query. If you only need the manufacturer name or a few fields, this is fine; otherwise, consider if it's needed.



class CarDetailView(generic.DetailView):
class CarDetailView(LoginRequiredMixin, generic.DetailView):
model = Car


class DriverListView(generic.ListView):
class DriverListView(LoginRequiredMixin, generic.ListView):
model = Driver
paginate_by = 5


class DriverDetailView(generic.DetailView):
class DriverDetailView(LoginRequiredMixin, generic.DetailView):
model = Driver
queryset = Driver.objects.prefetch_related("cars__manufacturer")
4 changes: 3 additions & 1 deletion taxi_service/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
)

# SECURITY WARNING: don"t run with debug turned on in production!
DEBUG = True
DEBUG = False

Choose a reason for hiding this comment

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

The DEBUG setting is set to False, which is appropriate for production. However, ensure that ALLOWED_HOSTS is configured with the domain names or IP addresses that your application will be served from in production. Leaving it empty can cause security vulnerabilities.

Choose a reason for hiding this comment

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

The DEBUG setting is set to False, which is correct for a production environment. However, ensure that the ALLOWED_HOSTS list is properly configured to include the domain names or IP addresses that your application will serve.


ALLOWED_HOSTS = []

Choose a reason for hiding this comment

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

The ALLOWED_HOSTS list is empty. In a production environment, you should specify the domain names or IP addresses that your Django application can serve. This is crucial for security to prevent HTTP Host header attacks.

Choose a reason for hiding this comment

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

The ALLOWED_HOSTS list is currently empty. In a production environment, you should specify the domain names or IP addresses that your Django site can serve. This is a security measure to prevent HTTP Host header attacks.


Expand Down Expand Up @@ -133,3 +133,5 @@
# https://docs.djangoproject.com/en/4.0/ref/settings/#default-auto-field

DEFAULT_AUTO_FIELD = "django.db.models.BigAutoField"

LOGIN_REDIRECT_URL = "/"
1 change: 1 addition & 0 deletions taxi_service/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,4 +22,5 @@
urlpatterns = [
path("admin/", admin.site.urls),
path("", include("taxi.urls", namespace="taxi")),

Choose a reason for hiding this comment

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

The include function for taxi.urls specifies a namespace, but ensure that the taxi app's urls.py file defines an app_name variable. This is necessary for namespacing to work correctly in Django.

path("accounts/", include("django.contrib.auth.urls")),

Choose a reason for hiding this comment

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

The inclusion of Django's built-in authentication URLs is correct and allows for user authentication features such as login and logout.

] + static(settings.STATIC_URL, document_root=settings.STATIC_ROOT)

Choose a reason for hiding this comment

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

Serving static files using django.conf.urls.static.static is suitable for development but not recommended for production. In a production environment, it's better to serve static files using a web server like Nginx or Apache.

7 changes: 7 additions & 0 deletions templates/includes/sidebar.html
Original file line number Diff line number Diff line change
@@ -1,4 +1,11 @@
<ul class="sidebar-nav">
{% if user.is_authenticated %}
<li><a href="{% url "taxi:driver-detail" pk=user.id %}">User: {{ user.username }}</a></li>

Choose a reason for hiding this comment

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

The URL tag for taxi:driver-detail uses the user.id as the primary key. Ensure that the Driver model's primary key corresponds to the user.id. If the Driver model uses a different field as the primary key, this could lead to incorrect URL generation.

<li><a href="{% url "logout" %}">Logout</a></li>
{% else %}
<li><a href="{% url "login" %}">Login</a></li>
{% endif %}
<br>
<li><a href="{% url "taxi:index" %}">Home page</a></li>
<li><a href="{% url "taxi:manufacturer-list" %}">Manufacturers</a></li>
<li><a href="{% url "taxi:car-list" %}">Cars</a></li>
Expand Down
6 changes: 6 additions & 0 deletions templates/registration/logged_out.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{% extends "base.html" %}

{% block content %}
<p>Logged out!</p>
<a href="{% url 'login'%}">Click here to login again.</a>
{% endblock %}
16 changes: 16 additions & 0 deletions templates/registration/login.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
{% extends "base.html" %}

{% block content %}
<h1>Login</h1>

{% if form.errors %}
<p style="color: red">Invalid credentials</p>
{% endif %}

<form method="post" action="{% url 'login' %}">
{% csrf_token %}
{{ form.as_p }}
<input type="submit" value="Submit">
<input type="hidden" name="next" value="{{ next }}">
</form>
{% endblock %}
3 changes: 3 additions & 0 deletions templates/taxi/driver_list.html
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ <h1>Driver list</h1>
<li>
<a href="{% url "taxi:driver-detail" pk=driver.id %}">{{ driver.username }}</a>
({{ driver.first_name }} {{ driver.last_name }})
{% if driver.username == user.username%}
(Me)

Choose a reason for hiding this comment

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

The logic to identify the current user by comparing driver.id with user.id assumes that the Driver model's primary key is the same as the user.id. Ensure that this assumption is correct, as any discrepancy could lead to incorrect identification of the current user.

{% endif %}
</li>
{% endfor %}
</ul>
Expand Down
1 change: 1 addition & 0 deletions templates/taxi/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,5 @@ <h2>Dynamic content</h2>
<li><strong>Drivers:</strong> {{ num_drivers }}</li>
<li><strong>Manufacturers:</strong> {{ num_manufacturers }}</li>
</ul>
<p>You have visited this page {{ num_visits }} time{{ num_visits|pluralize }}.</p>
{% endblock %}
Loading