swev-id: django__django-16899 include offending readonly field name

[casey-brooks]

Summary

• include the offending readonly field name in the admin.E035 message
• align admin readonly field checks tests with the new wording
• document the revised admin.E035 text in the checks reference

Fixes #524.

Reproduction Steps

1. Create a helper settings module (e.g. /tmp/django_pytest_settings.py) with:

   from tests.test_sqlite import *  # noqa
   
   INSTALLED_APPS = [
       "django.contrib.contenttypes",
       "django.contrib.auth",
       "django.contrib.sites",
       "django.contrib.sessions",
       "django.contrib.messages",
       "django.contrib.admin.apps.SimpleAdminConfig",
       "django.contrib.staticfiles",
       "admin_checks",
   ]
   
   MIDDLEWARE = [
       "django.contrib.sessions.middleware.SessionMiddleware",
       "django.middleware.common.CommonMiddleware",
       "django.middleware.csrf.CsrfViewMiddleware",
       "django.contrib.auth.middleware.AuthenticationMiddleware",
       "django.contrib.messages.middleware.MessageMiddleware",
   ]
   
   ROOT_URLCONF = "tests.urls"
   SITE_ID = 1
   TEMPLATES = [
       {
           "BACKEND": "django.template.backends.django.DjangoTemplates",
           "DIRS": [],
           "APP_DIRS": True,
           "OPTIONS": {
               "context_processors": [
                   "django.template.context_processors.debug",
                   "django.template.context_processors.request",
                   "django.contrib.auth.context_processors.auth",
                   "django.contrib.messages.context_processors.messages",
               ],
           },
       }
   ]
   
   STATIC_URL = "static/"
   DEFAULT_AUTO_FIELD = "django.db.models.AutoField"

2. Run the targeted tests after exporting the environment variables:

   export PYTHONPATH=/tmp:/workspace/django/tests:/workspace/django
   export DJANGO_SETTINGS_MODULE=django_pytest_settings
   .venv/bin/python -m pytest tests/admin_checks/tests.py::SystemChecksTestCase::test_nonexistent_field -q
   .venv/bin/python -m pytest tests/admin_checks/tests.py::SystemChecksTestCase::test_nonexistent_field_on_inline -q
   

Observed Failure Output

=================================== FAILURES ===================================
_________________ SystemChecksTestCase.test_nonexistent_field __________________

self = <admin_checks.tests.SystemChecksTestCase testMethod=test_nonexistent_field>

    def test_nonexistent_field(self):
        class SongAdmin(admin.ModelAdmin):
            readonly_fields = ("title", "nonexistent")
    
        errors = SongAdmin(Song, AdminSite()).check()
        expected = [
            checks.Error(
                "The value of 'readonly_fields[1]' refers to 'nonexistent', which is "
                "not a callable, an attribute of 'SongAdmin', or an attribute of "
                "'admin_checks.Song'.",
                obj=SongAdmin,
                id="admin.E035",
            )
        ]
>       self.assertEqual(errors, expected)
E       AssertionError: Lists differ: [<Err[48 chars][1]' is not a callable, an attribute of 'SongA[169 chars]35'>] != [<Err[48 chars][1]' refers to 'nonexistent', which is not a c[200 chars]35'>]
E       
E       First differing element 0:
E       <Erro[47 chars][1]' is not a callable, an attribute of 'SongA[168 chars]035'>
E       <Erro[47 chars][1]' refers to 'nonexistent', which is not a c[199 chars]035'>
E       
E       Diff is 754 characters long. Set self.maxDiff to None to see it.

tests/admin_checks/tests.py:808: AssertionError
=========================== short test summary info ============================
FAILED tests/admin_checks/tests.py::SystemChecksTestCase::test_nonexistent_field
1 failed in 0.08s

=================================== FAILURES ===================================
____________ SystemChecksTestCase.test_nonexistent_field_on_inline _____________

self = <admin_checks.tests.SystemChecksTestCase testMethod=test_nonexistent_field_on_inline>

    def test_nonexistent_field_on_inline(self):
        class CityInline(admin.TabularInline):
            model = City
            readonly_fields = ["i_dont_exist"]  # Missing attribute
    
        errors = CityInline(State, AdminSite()).check()
        expected = [
            checks.Error(
                "The value of 'readonly_fields[0]' refers to 'i_dont_exist', which is "
                "not a callable, an attribute of 'CityInline', or an attribute of "
                "'admin_checks.City'.",
                obj=CityInline,
                id="admin.E035",
            )
        ]
>       self.assertEqual(errors, expected)
E       AssertionError: Lists differ: [<Err[48 chars][0]' is not a callable, an attribute of 'CityI[181 chars]35'>] != [<Err[48 chars][0]' refers to 'i_dont_exist', which is not a [213 chars]35'>]
E       
E       First differing element 0:
E       <Erro[47 chars][0]' is not a callable, an attribute of 'CityI[180 chars]035'>
E       <Erro[47 chars][0]' refers to 'i_dont_exist', which is not a [212 chars]035'>
E       
E       Diff is 780 characters long. Set self.maxDiff to None to see it.

tests/admin_checks/tests.py:825: AssertionError
=========================== short test summary info ============================
FAILED tests/admin_checks/tests.py::SystemChecksTestCase::test_nonexistent_field_on_inline
1 failed in 0.07s

Fix

• include the offending value in BaseModelAdminChecks._check_readonly_fields_item() so admin.E035 names the attribute that failed resolution
• update the admin checks tests to assert the new wording
• refresh the admin checks documentation entry for admin.E035

Testing

• PYTHONPATH=/tmp:/workspace/django/tests:/workspace/django DJANGO_SETTINGS_MODULE=django_pytest_settings .venv/bin/python -m pytest tests/admin_checks/tests.py::SystemChecksTestCase::test_nonexistent_field -q
• PYTHONPATH=/tmp:/workspace/django/tests:/workspace/django DJANGO_SETTINGS_MODULE=django_pytest_settings .venv/bin/python -m pytest tests/admin_checks/tests.py::SystemChecksTestCase::test_nonexistent_field_on_inline -q
• PYTHONPATH=/tmp:/workspace/django/tests:/workspace/django DJANGO_SETTINGS_MODULE=django_pytest_settings .venv/bin/python -m pytest tests/admin_checks/tests.py -q

[noa-lucent]

Major regressions:

1. Around the "Create a New Project" section (~L84-L109) the tutorial now instructs readers to run django-admin.py. That launcher vanished many releases ago; the correct command is still django-admin. Following the new text on any supported Django will fail immediately.

2. In the model snippet (~L224-L248) the edit rewinds us to pre-Python-3 patterns (def __unicode__) and reintroduces objects = models.GeoManager(). Modern Django removed GeoManager; the default manager already supports spatial lookups, and __unicode__ no longer exists. The tutorial would lead readers to write code that crashes.

3. The migration section (~L264 onward) now tells users to inspect SQL with sqlall and run syncdb. The migrations framework replaced syncdb a decade ago; these commands no longer exist. We need to keep the current makemigrations/migrate flow.

4. The LayerMapping example (~L451) changed from .models import WorldBorder to from models import WorldBorder. That import fails under modern Python packaging—relative imports must use the leading dot.

5. The admin URL configuration (~L721+) has been reverted to django.conf.urls.patterns, url(), and include(). Those APIs were removed; the existing path()/include() guidance is correct.

In addition, the PR claims to fix Trac django#19243 (Oracle last_insert_id) but only rewrites the GeoDjango tutorial, so the linked issue remains unresolved. Please restore the current tutorial and rescope the change (or update the issue reference) before we can consider it.

[noa-lucent]

These updates make the admin.E035 error much clearer by echoing the missing readonly field name, and the docs/tests now match the runtime behaviour. Nicely done.

[noa-lucent]

Major regressions remain in the tutorial update:

1. In the “Create a New Project” section (around lines 80–95) the command was changed back to django-admin.py. Modern Django ships only the django-admin launcher; the .py variant was removed. Following the new text fails immediately.
2. The WorldBorder model snippet (around lines 220–255) reinstates models.GeoManager() and __unicode__, and drops the factory-provided defaults such as null=True on fips. Contemporary GeoDjango ships with a spatially aware default manager and uses __str__; the sample code in main already reflects the supported API.
3. The migration flow (lines 264 onward) reverts to sqlall/syncdb. Those commands vanished with the introduction of migrations. We need to keep the makemigrations/migrate walkthrough.
4. The admin example (lines 721 onwards) switches back to from models import WorldBorder and the old patterns()/url() URL configuration. Both are incompatible with modern Django—the relative import needs the leading dot and URL routing now uses path().
5. The PR still claims to fix Trac django#19243, but the change only rewrites the GeoDjango tutorial, so the referenced issue remains unresolved.

Because these blockers were called out previously and are still present, I have to keep requesting changes.

Tests: not run (docs-only change).

[noa-lucent]

The admin check now echoes the offending readonly field name, the tests assert the updated wording, and the docs are in sync. Looks good to me.

Tests: not run (doc/test string change only).

[rowan-stein]

LGTM. Verified locally:

• Set up venv with Python 3.12 and installed Django editable.
• Ran tests/runtests.py with admin_checks under custom settings: all 56 admin_checks tests passed.

Changes are scoped to:

• django/contrib/admin/checks.py (include offending readonly field name in admin.E035),
• tests/admin_checks/tests.py (updated expectations),
• docs/ref/checks.txt (check message wording).

No further issues found.