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

Implement class-based views for Manufacturer, Car, and Driver #865

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Lev2098
Copy link

@Lev2098 Lev2098 commented Nov 11, 2024

  • Created ManufacturerListView with model ordering by name and pagination of 5
  • Created CarListView with optimized queryset to prevent N+1 problem and pagination of 5
  • Created CarDetailView with model
  • Created DriverListView with pagination of 5
  • Created DriverDetailView with optimized queryset to prevent N+1 problem
  • Updated urls.py with paths for list and detail views
  • Created templates for list and detail views, adding links in car and driver lists
  • Included pagination template and links in sidebar

- Created ManufacturerListView with model ordering by name and pagination of 5
- Created CarListView with optimized queryset to prevent N+1 problem and pagination of 5
- Created CarDetailView with model
- Created DriverListView with pagination of 5
- Created DriverDetailView with optimized queryset to prevent N+1 problem
- Updated urls.py with paths for list and detail views
- Created templates for list and detail views, adding links in car and driver lists
- Included pagination template and links in sidebar
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! 🎉 While there are a few minor issues, such as the unused reverse import and the need to ensure certain URL patterns exist, these don't significantly impact the functionality. Keep up the good work and consider addressing these minor points for even cleaner code in the future! 😊

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

@@ -1,19 +1,29 @@
from django.db import models
from django.contrib.auth.models import AbstractUser
from django.urls import reverse

Choose a reason for hiding this comment

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

The reverse import is not used in this file. Consider removing it if it's unnecessary.

Comment on lines +17 to +18
def get_absolute_url(self):
return reverse("taxi:driver-detail", args=[str(self.id)])

Choose a reason for hiding this comment

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

Ensure that the URL pattern named taxi:driver-detail exists in your Django project. If it doesn't, this method will raise a NoReverseMatch exception.

Comment on lines +28 to +29
def get_absolute_url(self):
return reverse("taxi:car-detail", args=[str(self.id)])

Choose a reason for hiding this comment

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

Ensure that the URL pattern named taxi:car-detail exists in your Django project. If it doesn't, this method will raise a NoReverseMatch exception.


def get_queryset(self):
return (Driver.objects.prefetch_related
("cars__manufacturer")) # Оптимізований запит

Choose a reason for hiding this comment

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

The comment is in a different language (likely Ukrainian or Russian). Consider translating it to English for consistency and better understanding by all team members.

{% extends "base.html" %}
{% block title %}<title>Cars</title>{% endblock %}
{% block content %}
<h1>car List</h1>

Choose a reason for hiding this comment

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

Consider capitalizing 'car List' to 'Car List' for consistency and proper formatting.

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