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

Open
wants to merge 3 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
4 changes: 4 additions & 0 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@ jobs:
runs-on: ubuntu-latest
timeout-minutes: 10

env:
SECRET_KEY: "django-insecure-8ovil3xu6=eaoqd#-#&ricv159p0pypoh5_lgm*)-dfcjqe=yc"

Choose a reason for hiding this comment

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

It's a security issue to hardcode sensitive information like SECRET_KEY in your configuration files. Consider using GitHub Secrets to store this value securely and reference it in your workflow.

Choose a reason for hiding this comment

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

Security Concern: Hardcoding the SECRET_KEY directly in the workflow file is a security risk. Consider using GitHub Secrets to securely store and reference this value.

DEBUG: "False"

steps:
- name: Checkout repo
uses: actions/checkout@v2
Expand Down
Binary file modified requirements.txt
Binary file not shown.
14 changes: 14 additions & 0 deletions taxi/forms.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
from django import forms
from .models import Car, Manufacturer


class CarForm(forms.ModelForm):
class Meta:
model = Car
fields = "__all__"

Choose a reason for hiding this comment

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

Using fields = "__all__" will include all fields from the Car model in the form. Ensure that this is intentional and that no sensitive fields are exposed. Consider specifying only the fields you want to include explicitly.

Choose a reason for hiding this comment

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

Form Field Exposure: Using fields = "__all__" can unintentionally expose sensitive fields. Specify only the fields you want to include to ensure data security.



class ManufacturerForm(forms.ModelForm):
class Meta:
model = Manufacturer
fields = "__all__"

Choose a reason for hiding this comment

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

Similar to the CarForm, using fields = "__all__" for ManufacturerForm will include all fields from the Manufacturer model. Make sure this is what you want, and consider specifying fields explicitly to avoid exposing sensitive data.

Choose a reason for hiding this comment

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

Form Field Exposure: Similar to the CarForm, using fields = "__all__" here can expose sensitive fields. Specify only the necessary fields to include in the form.

26 changes: 25 additions & 1 deletion taxi/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,40 @@
DriverListView,
DriverDetailView,
ManufacturerListView,
CarCreateView,
ManufacturerCreateView,
ManufacturerUpdateView,
CarUpdateView,
ManufacturerDeleteView,
CarDeleteView,
)

urlpatterns = [
path("", index, name="index"),
path(
"manufacturers/",
ManufacturerListView.as_view(),
name="manufacturer-list",
name="manufacturer-list"
),
path(
"manufacturers/create/",
ManufacturerCreateView.as_view(),
name="manufacturer-create"
),
path(
"manufacturers/<int:pk>/update/",
ManufacturerUpdateView.as_view(),
name="manufacturer-update"
),
path(
"manufacturers/<int:pk>/delete/",
ManufacturerDeleteView.as_view(),
name="manufacturer-delete"
),
path("cars/", CarListView.as_view(), name="car-list"),
path("cars/create/", CarCreateView.as_view(), name="car-create"),
path("cars/<int:pk>/update/", CarUpdateView.as_view(), name="car-update"),
path("cars/<int:pk>/delete/", CarDeleteView.as_view(), name="car-delete"),
path("cars/<int:pk>/", CarDetailView.as_view(), name="car-detail"),
path("drivers/", DriverListView.as_view(), name="driver-list"),
path(
Expand Down
41 changes: 41 additions & 0 deletions taxi/views.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
from django.urls import reverse_lazy

from django.contrib.auth.decorators import login_required
from django.shortcuts import render
from django.views import generic
from django.contrib.auth.mixins import LoginRequiredMixin

from .forms import ManufacturerForm, CarForm
from .models import Driver, Car, Manufacturer


Expand Down Expand Up @@ -34,12 +37,50 @@ class ManufacturerListView(LoginRequiredMixin, generic.ListView):
paginate_by = 5


class ManufacturerCreateView(LoginRequiredMixin, generic.CreateView):
form_class = ManufacturerForm
success_url = reverse_lazy("taxi:manufacturer-list")
template_name = "taxi/manufacturer_form.html"


class ManufacturerUpdateView(LoginRequiredMixin, generic.UpdateView):
form_class = ManufacturerForm
success_url = reverse_lazy("taxi:manufacturer-list")
template_name = "taxi/manufacturer_form.html"
queryset = Manufacturer.objects.all()


class ManufacturerDeleteView(LoginRequiredMixin, generic.DeleteView):
model = Manufacturer
template_name = "taxi/manufacturer_confirm_delete.html"
success_url = reverse_lazy("taxi:manufacturer-list")


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

Choose a reason for hiding this comment

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

The use of select_related in CarListView is good for performance optimization by reducing database queries. Ensure that 'manufacturer' is a foreign key in the Car model.



class CarCreateView(LoginRequiredMixin, generic.CreateView):
form_class = CarForm
success_url = reverse_lazy("taxi:car-list")
template_name = "taxi/car_form.html"


class CarUpdateView(LoginRequiredMixin, generic.UpdateView):
form_class = CarForm
success_url = reverse_lazy("taxi:car-list")
template_name = "taxi/car_form.html"
queryset = Car.objects.all().select_related("manufacturer")


class CarDeleteView(LoginRequiredMixin, generic.DeleteView):
model = Car
template_name = "taxi/car_confirm_delete.html"
success_url = reverse_lazy("taxi:car-list")


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

Expand Down
14 changes: 9 additions & 5 deletions taxi_service/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,10 @@
For the full list of settings and their values, see
https://docs.djangoproject.com/en/4.0/ref/settings/
"""

import os
from pathlib import Path
from dotenv import load_dotenv
load_dotenv()

# Build paths inside the project like this: BASE_DIR / "subdir".
BASE_DIR = Path(__file__).resolve().parent.parent
Expand All @@ -20,12 +22,10 @@
# See https://docs.djangoproject.com/en/4.0/howto/deployment/checklist/

# SECURITY WARNING: keep the secret key used in production secret!
SECRET_KEY = (
"django-insecure-8ovil3xu6=eaoqd#-#&ricv159p0pypoh5_lgm*)-dfcjqe=yc"
)
SECRET_KEY = os.environ.get("SECRET_KEY")

# SECURITY WARNING: don"t run with debug turned on in production!
DEBUG = True
DEBUG = os.environ.get("DEBUG") == "True"

Choose a reason for hiding this comment

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

Ensure that the DEBUG setting is set to False in a production environment to avoid exposing sensitive information.


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 setting is currently empty. For production, make sure to specify the domain names or IP addresses that your Django site can serve.

Choose a reason for hiding this comment

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

Production Settings: The ALLOWED_HOSTS setting is empty. In a production environment, ensure to populate this with the appropriate domain names or IP addresses to prevent security vulnerabilities.


Expand All @@ -44,6 +44,8 @@
"django.contrib.staticfiles",
"debug_toolbar",
"taxi",
"crispy_forms",
"crispy_bootstrap4"
]

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

DEFAULT_AUTO_FIELD = "django.db.models.BigAutoField"

CRISPY_TEMPLATE_PACK = "bootstrap4"
11 changes: 11 additions & 0 deletions templates/taxi/car_confirm_delete.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{% extends "base.html" %}

{% block content %}
<h1>Delete Car</h1>

<p>Are you sure you want to delete <strong>{{ car }}</strong>?</p>
<form action="" method="post">
{% csrf_token %}
<input type="submit" value="Yes, delete" class="btn btn-danger">
</form>

Choose a reason for hiding this comment

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

Consider adding a cancel button or link to allow users to navigate away from the delete confirmation page without taking action. This improves user experience by providing an easy way to cancel the operation.

{% endblock %}
11 changes: 11 additions & 0 deletions templates/taxi/car_form.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{% extends "base.html" %}
{% load crispy_forms_filters %}
{% block content %}
<h1>Create Car</h1>
<form action="" method="post">
{% csrf_token %}
{{ form|crispy }}
{# {{ form.as_p }}#}
<input type="submit" value="Submit">

Choose a reason for hiding this comment

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

Consider adding Bootstrap classes to the submit button for styling consistency, such as class="btn btn-primary".

</form>
{% endblock %}
30 changes: 23 additions & 7 deletions templates/taxi/car_list.html
Original file line number Diff line number Diff line change
@@ -1,16 +1,32 @@
{% extends "base.html" %}

{% block content %}
<h1>Car list</h1>
<h1>Car list <a href="{% url "taxi:car-create" %}">+</a></h1>
{% if car_list %}
<ul>
<table class="table">
<tr>
<th>ID</th>
<th>Model</th>
<th>Update</th>
<th>Delete</th>
</tr>
{% for car in car_list %}
<li>
<a href="{% url "taxi:car-detail" pk=car.id %} ">{{ car.id }}</a>
{{ car.model }} ({{ car.manufacturer.name }})
</li>
<tr>
<td>
{{ car.id }}
</td>
<td>
<a href="{% url "taxi:car-detail" pk=car.id %} ">{{ car.model }} ({{ car.manufacturer.name }})</a>
</td>
<td>
<a href="{% url "taxi:car-update" pk=car.id %}">Update</a>
</td>
<td>
<a href="{% url "taxi:car-delete" pk=car.id %}">DELETE</a>
</td>
</tr>
{% endfor %}
</ul>
</table>
{% else %}
<p>There are no cars in taxi</p>
{% endif %}
Expand Down
12 changes: 12 additions & 0 deletions templates/taxi/manufacturer_confirm_delete.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
{% extends "base.html" %}

{% block content %}
<h1>Delete Manufacturer</h1>

<p>Are you sure you want to delete <strong>{{ manufacturer }}</strong>?</p>
<p><i>All cars from that manufacturer will be also deleted!</i></p>
<form action="" method="post">
{% csrf_token %}
<input type="submit" value="Yes, delete" class="btn btn-danger">
</form>

Choose a reason for hiding this comment

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

Consider adding a cancel button or link to allow users to navigate away from the delete confirmation page without taking action. This improves user experience by providing an easy way to cancel the operation.

{% endblock %}
11 changes: 11 additions & 0 deletions templates/taxi/manufacturer_form.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{% extends "base.html" %}
{% load crispy_forms_filters %}
{% block content %}
<h1>Create Manufacturer</h1>
<form action="" method="post">
{% csrf_token %}
{{ form|crispy }}
{# {{ form.as_p }}#}
<input type="submit" value="Submit">

Choose a reason for hiding this comment

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

Consider adding Bootstrap classes to the submit button for styling consistency, such as class="btn btn-primary".

</form>
{% endblock %}
16 changes: 12 additions & 4 deletions templates/taxi/manufacturer_list.html
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{% extends "base.html" %}

{% block content %}
<h1>Manufacturer List
<h1>Manufacturer List <a href="{% url "taxi:manufacturer-create" %}">+</a>

Choose a reason for hiding this comment

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

Consider adding Bootstrap classes to the '+' link to make it more visually appealing and consistent with other buttons or links in the application, such as class="btn btn-primary".

</h1>

{% if manufacturer_list %}
Expand All @@ -10,18 +10,26 @@ <h1>Manufacturer List
<th>ID</th>
<th>Name</th>
<th>Country</th>
<th>Update</th>
<th>Delete</th>
</tr>

{% for manufacturer in manufacturer_list %}
<tr>
<td>
{{ manufacturer.id }}
{{ manufacturer.id }}
</td>
<td>
{{ manufacturer.name }}
{{ manufacturer.name }}
</td>
<td>
{{ manufacturer.country }}
{{ manufacturer.country }}
</td>
<td>
<a href="{% url "taxi:manufacturer-update" pk=manufacturer.id %}">Update</a>
</td>
<td>
<a href="{% url "taxi:manufacturer-delete" pk=manufacturer.id %}">DELETE</a>
</td>
</tr>
{% endfor %}
Expand Down
Loading