ackama / django-template

A template for initiating new Django projects
BSD 3-Clause "New" or "Revised" License
4 stars 0 forks source link

Correct way to add an app probably needs to be documented #25

Open wsot opened 9 months ago

wsot commented 9 months ago

🤔 Issue:

Because of the project structure, creating a new app requires (slightly) more than just using ./manage.py createapp <my app name> - specifically, updating the name in the new apps.py to include the parent package name. It is also not immediately clear that the parent package name should be included when adding the new app to INSTALLED_APPS.

This should probably be in the documentation.

(This uses the bug template because originally I thought I'd hit a catch-22 where I couldn't get both mypy and pytest working with the same app setup)

📝Steps to reproduce:

Once the template is set up, it is not obvious (to me) the correct way to create a new app and set it up.

For example, I create the project using:

django-admin startproject --template ../ackama/django-template/template/ --extension py,env,toml,yml myfunproject

I set up the basics:

cd myfunproject
poetry install
cp example.env .env
poetry shell

I run inv check and it passes type checking and 'test views'. (It fails playwright because of my Chromium, but that's irrelevant)

I then do a Django check.

cd src/myfunproject
./manage.py check

That comes out file as well (except staticfiles dir doesn't exist - again, not relevant)

Now I want to create an app to make some functionality. In the same folder as the ./manage.py file, I create the app:

./manage.py startapp myexcitingapp
# I could equally use `django-admin startapp myexcitingapp` - the effect is exactly the same.

Now I need to update the INSTALLED_APPS in main/settings.py to include the app. I can potentially here either include the parent package name (myfunproject) or not. i.e. I can do (including above and below line for context):

    # Local
    "myfunproject.myexcitingapp",
]

or

    # Local
    "myexcitingapp",
]

Other lines, like the root URL config include the prefix (i.e. ROOT_URLCONF = "myfunproject.main.urls"). Based on that, I'd expect to include it.

Both of them will pass a ./manage.py check.

In order to test which works, I update my myexcitingapp.py to contain:

from django.db import models

class MyModel(models.Model):
    my_text = models.CharField(max_length=100)

With that model in place, both values in the settings.py file will result in migrations being generated for the model. That is, both are being included.

However, if I go up two folders and run inv check again, both will fail the pytest stage with. If I use just "myexcitingapp", in the settings I get:

ModuleNotFoundError: No module named 'myexcitingapp'

If I instead use myfunproject.myexcitingapp in the settings.py then I get the error:

django.core.exceptions.ImproperlyConfigured: Cannot import 'myexcitingapp'. Check that 'myfunproject.myexcitingapp.apps.MyexcitingappConfig.name' is correct.

I need to go and edit the src/myfunproject/myexcitingapp/apps.py and update the app name from myexcitingapp to myfunproject.myexcitingapp.

Once I have done that, then the tests will pass and the ./manage.py check will also pass.

✅ Expected Result:

There are clear instructions on how to add a new app (i.e. what values need to go where in settings.py and apps.py

➕Additional Information:

wsot commented 9 months ago

The actual origin of this was me getting an error: RuntimeError: Model class myexcitingapp.models.MyModel doesn't declare an explicit app_label and isn't in an application in INSTALLED_APPS.

The reason I got the error, which took me a bit longer to track down than I would have liked, was that in my urls.py I had missed the parent package name - i.e.

urlpatterns = [
    path("myexcitingapp/", include("myexcitingapp.urls"),
] 

As soon as myexcitingapp/urls.py contained a FormView that used my model, it failed - but I didn't notice the timing relationship immediately.

I can imagine not being the last to clumsily trip over this, but I'm not sure how to prevent others clumsily tripping over this either. Maybe something in the docs so the error is super-searchable.