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

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
1 change: 1 addition & 0 deletions requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,4 @@ flake8-variables-names==0.0.5
pep8-naming==0.13.2
django-debug-toolbar==3.2.4
django-crispy-forms==1.14.0
crispy-bootstrap4
19 changes: 19 additions & 0 deletions taxi/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,12 @@
DriverListView,
DriverDetailView,
ManufacturerListView,
CarCreateView,
CarUpdateView,
CarDeleteView,
ManufacturerCreateView,
ManufacturerUpdateView,
ManufacturerDeleteView,
)

urlpatterns = [
Expand All @@ -22,6 +28,19 @@
path(
"drivers/<int:pk>/", DriverDetailView.as_view(), name="driver-detail"
),
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("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"),

]

app_name = "taxi"
41 changes: 41 additions & 0 deletions taxi/views.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from django.contrib.auth.decorators import login_required
from django.shortcuts import render
from django.urls import reverse_lazy
from django.views import generic
from django.contrib.auth.mixins import LoginRequiredMixin

Expand Down Expand Up @@ -34,6 +35,26 @@ class ManufacturerListView(LoginRequiredMixin, generic.ListView):
paginate_by = 5


class ManufacturerCreateView(LoginRequiredMixin, generic.CreateView):
model = Manufacturer
fields = "__all__"
success_url = reverse_lazy("taxi:manufacturer-list")
template_name = "taxi/manufacturer_form.html"


class ManufacturerUpdateView(LoginRequiredMixin, generic.UpdateView):
model = Manufacturer
fields = "__all__"
success_url = reverse_lazy("taxi:manufacturer-list")
template_name = "taxi/manufacturer_form.html"


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


class CarListView(LoginRequiredMixin, generic.ListView):
model = Car
paginate_by = 5
Expand All @@ -44,6 +65,26 @@ class CarDetailView(LoginRequiredMixin, generic.DetailView):
model = Car


class CarCreateView(LoginRequiredMixin, generic.CreateView):
model = Car
fields = "__all__"
success_url = reverse_lazy("taxi:car-list")
template_name = "taxi/car_form.html"


class CarUpdateView(LoginRequiredMixin, generic.UpdateView):
model = Car
fields = "__all__"
success_url = reverse_lazy("taxi:car-list")
template_name = "taxi/car_form.html"


class CarDeleteView(LoginRequiredMixin, generic.DeleteView):
model = Manufacturer

Choose a reason for hiding this comment

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

The CarDeleteView should use the Car model instead of the Manufacturer model. Ensure that the model is correctly specified to delete car instances.

Comment on lines +82 to +83

Choose a reason for hiding this comment

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

The CarDeleteView is incorrectly using the Manufacturer model. It should be using the Car model to delete car instances. Please change model = Manufacturer to model = Car.

success_url = reverse_lazy("taxi:car-list")
template_name = "taxi/car_delete.html"


class DriverListView(LoginRequiredMixin, generic.ListView):
model = Driver
paginate_by = 5
Expand Down
7 changes: 7 additions & 0 deletions taxi_service/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,9 @@
"django.contrib.staticfiles",
"debug_toolbar",
"taxi",
"crispy_bootstrap4",
"crispy_forms",

]

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

DEFAULT_AUTO_FIELD = "django.db.models.BigAutoField"

CRISPY_ALLOWED_TEMPLATE_PACKS = "bootstrap4"

CRISPY_TEMPLATE_PACK = "bootstrap4"
10 changes: 10 additions & 0 deletions templates/taxi/car_delete.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{% extends "base.html" %}
{% block content %}
<h1>Delete car:</h1>
<p>Are you sure you want to delete the car: {{ car }}?</p>

Choose a reason for hiding this comment

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

The template uses {{ car }} to display the car's information. Ensure that the context variable car is correctly passed from the view to this template. If the context variable is named differently, update it accordingly.

<p><i>All data related to this car will be also deleted!</i></p>
<form action="" method="post">
{% csrf_token %}
<input type="submit" value="Yes, delete" class="btn btn-danger">
</form>
{% endblock %}
2 changes: 2 additions & 0 deletions templates/taxi/car_detail.html
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,6 @@ <h1>Drivers</h1>
<li>{{ driver.username }} ({{ driver.first_name }} {{ driver.last_name }})</li>
{% endfor %}
</ul>
<a href="./update/" class="btn btn-primary">Update</a>
<a href="./delete/" class="btn btn-primary">Delete</a>
Comment on lines +12 to +13

Choose a reason for hiding this comment

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

Using relative URLs like ./update/ and ./delete/ can lead to issues if the URL structure changes. It's recommended to use the {% url %} template tag to dynamically generate these URLs based on the named URL patterns defined in urls.py. For example, {% url 'taxi:car-update' car.id %} for the update link.

{% endblock %}
10 changes: 10 additions & 0 deletions templates/taxi/car_form.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{% extends "base.html" %}
{% load crispy_forms_filters %}
{% block content %}
<h1>{{ objects|yesno:"Update,Create" }} car:</h1>

Choose a reason for hiding this comment

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

The use of objects|yesno:"Update,Create" seems incorrect. It should likely be object|yesno:"Update,Create" to determine whether the form is for updating or creating based on the presence of an object instance. Ensure that the context variable object is correctly passed to the template.

Choose a reason for hiding this comment

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

The use of {{ objects|yesno:"Update,Create" }} is incorrect because objects is not a standard context variable for determining form actions. Typically, you would check if the form instance has an id or use a context variable like is_update to decide between 'Update' and 'Create'. Ensure the correct context variable is used here.

<form action="" method="post" novalidate>
{% csrf_token %}
{{ form|crispy }}
<input class="btn btn-primary" type="submit" value="Submit">
</form>
{% endblock %}
2 changes: 2 additions & 0 deletions templates/taxi/car_list.html
Original file line number Diff line number Diff line change
Expand Up @@ -14,4 +14,6 @@ <h1>Car list</h1>
{% else %}
<p>There are no cars in taxi</p>
{% endif %}
<a href="./create/" class="m-md-2 btn btn-primary">Add new car</a>

Choose a reason for hiding this comment

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

Using a relative URL like ./create/ for the 'Add new car' link can lead to issues if the URL structure changes. It's recommended to use the {% url %} template tag to dynamically generate this URL based on the named URL pattern defined in urls.py. For example, {% url 'taxi:car-create' %}.


{% endblock %}
10 changes: 10 additions & 0 deletions templates/taxi/manufacturer_delete.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{% extends "base.html" %}
{% block content %}
<h1>Delete manufacturer:</h1>
<p>Are you sure you want to delete the manufacturer: {{ manufacturer }}?</p>

Choose a reason for hiding this comment

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

The template uses {{ manufacturer }} to display the manufacturer's information. Ensure that the context variable manufacturer is correctly passed from the view to this template. If the context variable is named differently, update it accordingly.

<p><i>All cars with this manufacturer will be also deleted!</i></p>
<form action="" method="post">
{% csrf_token %}
<input type="submit" value="Yes, delete" class="btn btn-danger">
</form>
{% endblock %}
10 changes: 10 additions & 0 deletions templates/taxi/manufacturer_form.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{% extends "base.html" %}
{% load crispy_forms_filters %}
{% block content %}
<h1>{{ objects|yesno:"Update,Create" }} manufacturer:</h1>

Choose a reason for hiding this comment

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

The use of objects|yesno:"Update,Create" seems incorrect. It should likely be object|yesno:"Update,Create" to determine whether the form is for updating or creating based on the presence of an object instance. Ensure that the context variable object is correctly passed to the template.

Choose a reason for hiding this comment

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

The use of {{ objects|yesno:"Update,Create" }} is incorrect because objects is not a standard context variable for determining form actions. Typically, you would check if the form instance has an id or use a context variable like is_update to decide between 'Update' and 'Create'. Ensure the correct context variable is used here.

<form action="" method="post" novalidate>
{% csrf_token %}
{{ form|crispy }}
<input class="btn btn-primary" type="submit" value="Submit">
</form>
{% endblock %}
9 changes: 9 additions & 0 deletions templates/taxi/manufacturer_list.html
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ <h1>Manufacturer List
<th>ID</th>
<th>Name</th>
<th>Country</th>
<th>Update</th>
<th>Delete</th>
</tr>

{% for manufacturer in manufacturer_list %}
Expand All @@ -23,11 +25,18 @@ <h1>Manufacturer List
<td>
{{ manufacturer.country }}
</td>
<td>
<a href="{{ manufacturer.id }}/update/" class="btn btn-primary">+</a>

Choose a reason for hiding this comment

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

The URL for the update link is hardcoded as {{ manufacturer.id }}/update/. It would be better to use the {% url %} template tag to generate the URL dynamically, ensuring it aligns with the URL patterns defined in urls.py. For example: {% url 'taxi:manufacturer-update' pk=manufacturer.id %}.

Choose a reason for hiding this comment

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

Using a relative URL like {{ manufacturer.id }}/update/ can lead to issues if the URL structure changes. It's recommended to use the {% url %} template tag to dynamically generate this URL based on the named URL pattern defined in urls.py. For example, {% url 'taxi:manufacturer-update' manufacturer.id %}.

</td>
<td>
<a href="{{ manufacturer.id }}/delete/" class="btn btn-danger">-</a>

Choose a reason for hiding this comment

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

The URL for the delete link is hardcoded as {{ manufacturer.id }}/delete/. It would be better to use the {% url %} template tag to generate the URL dynamically, ensuring it aligns with the URL patterns defined in urls.py. For example: {% url 'taxi:manufacturer-delete' pk=manufacturer.id %}.

Choose a reason for hiding this comment

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

Using a relative URL like {{ manufacturer.id }}/delete/ can lead to issues if the URL structure changes. It's recommended to use the {% url %} template tag to dynamically generate this URL based on the named URL pattern defined in urls.py. For example, {% url 'taxi:manufacturer-delete' manufacturer.id %}.

</td>
</tr>
{% endfor %}
</table>

{% else %}
<p>There are no manufacturers in the service.</p>
{% endif %}
<a href="./create/" class="m-md-2 btn btn-primary">Add new manufacturer</a>

Choose a reason for hiding this comment

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

Using a relative URL like ./create/ for the 'Add new manufacturer' link can lead to issues if the URL structure changes. It's recommended to use the {% url %} template tag to dynamically generate this URL based on the named URL pattern defined in urls.py. For example, {% url 'taxi:manufacturer-create' %}.

{% endblock %}
Loading