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

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

Solution #406

wants to merge 6 commits into from

Conversation

9rosLove
Copy link

No description provided.

@9rosLove 9rosLove closed this Sep 25, 2023
@9rosLove 9rosLove reopened this Sep 25, 2023
@9rosLove
Copy link
Author

9rosLove commented Sep 25, 2023

cars
car create
car
car update
car delete
manufactures
manufacture creat
manufacture update
manufacture delete

Copy link

@MarianKovalyshyn MarianKovalyshyn left a comment

Choose a reason for hiding this comment

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

Looks good, but for a better user experience you can apply my notes.

taxi/views.py Outdated
model = Car
fields = "__all__"
success_url = reverse_lazy("taxi:car-list")
template_name = "taxi/car_form.html"

Choose a reason for hiding this comment

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

I think you have redundant redefinitions of 'template_name'

{% csrf_token %}
<input type="submit" value="Yes, delete" class="btn btn-danger">
</form>
{% endblock %}

Choose a reason for hiding this comment

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

You forget about 'Cancel' button. It was stated in 'checklist'

taxi/views.py Outdated

class CarDeleteView(LoginRequiredMixin, generic.DeleteView):
model = Car
template_name = "taxi/car_confirm_delete.html"
Copy link

Choose a reason for hiding this comment

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

do we really need template_name?

@9rosLove 9rosLove requested a review from Sofbog September 29, 2023 06:57
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.

3 participants