Skip to content

Complete1#1340

Open
KiporenkoMaksym wants to merge 2 commits intomate-academy:masterfrom
KiporenkoMaksym:develop
Open

Complete1#1340
KiporenkoMaksym wants to merge 2 commits intomate-academy:masterfrom
KiporenkoMaksym:develop

Conversation

@KiporenkoMaksym
Copy link

Screen_1

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.

Review Summary

I am requesting changes because there is still one core requirement missing in your configuration that affects how templates are discovered. Functionally, your index view, URL routing, and templates (including base.html and includes/sidebar.html) are correctly implemented and meet the main task goals, which is great. However, in taxi_service/settings.py, the TEMPLATES[0]["DIRS"] list is empty; per the task, you must tell Django where to find project-level templates, for example by adding BASE_DIR / "templates" to that list so the engine searches the project templates/ directory as required.

Critical fixes needed:

  1. Templates directory configuration (blocker): In taxi_service/settings.py, locate the TEMPLATES setting (around the existing "DIRS": [] line) and change it so it includes your project templates folder, e.g. "DIRS": [BASE_DIR / "templates"]. This directly addresses the requirement to configure the template engine to look in the project-level templates/ directory and ensures future templates placed there will be found.
  2. Minor but checklist-related: In taxi/templates/taxi/index.html, add a single trailing newline after {% endblock %} so the file ends with exactly one blank line [CHECKLIST ITEM #1].

Once you update the template directories in settings and add that final newline, your solution will fully align with both the functional requirements and the checklist; the core implementation is solid, so you are very close—nice work so far.


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

Important

Fix the issues, commit the changes, and then re-request my review using the Re-request review button2 in the Reviewers tab.

Tip

If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag3.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.

Caution

Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.

Footnotes

  1. Rate AI review example

  2. Re-request AI review example

  3. Add AI review comments example

<li>Number of manufacturers: {{ num_manufacturers }}</li>
</ul>

{% endblock %}

Choose a reason for hiding this comment

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

This violates checklist item #1: 'Ensure each file ends with a single blank line'. Add a single newline at the end of this file (after the {% endblock %}) so the file ends with one blank line.


STATIC_URL = "static/"

STATICFILES_DIRS = [

Choose a reason for hiding this comment

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

TEMPLATES['DIRS'] is empty. This violates the requirement: "Edit settings so that engine knows where to look for template source files." Add the project templates directory, for example:

TEMPLATES = [
    {
        "BACKEND": "django.template.backends.django.DjangoTemplates",
        "DIRS": [BASE_DIR / "templates"],
        "APP_DIRS": True,
        ...
    },
]

Place this change so the engine will find templates in the project-level templates/ directory.

Copy link

@vsmutok vsmutok 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!

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