[change] Update theme customization paths and enhance collectstatic logic#579
[change] Update theme customization paths and enhance collectstatic logic#579
Conversation
…ogic Bug: Previously, when the OPENWISP_ADMIN_THEME_LINKS setting included any CSS or JavaScript file paths starting with /static/, the OpenWISP utilities logic would strip the /static/ prefix. As a result, Django couldn't recognize these custom static files, preventing them from being collected and served. Additionally, static files were previously mounted directly in the Nginx container, bypassing Django’s static collection workflow entirely. Fix: Mount custom theme assets inside the dashboard container and integrate them into Django’s staticfiles pipeline, ensuring they are processed by collectstatic and correctly served via Nginx.
📝 WalkthroughWalkthroughThe changes introduce a customization mechanism for OpenWISP static files (CSS and themes) by implementing change detection and recovery workflows. The docker-compose configuration is updated to separate theme customization into Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error)
✅ Passed checks (2 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/user/customization.rst`:
- Around line 190-191: Update the docs/user/customization.rst entry that
currently points to "customization/custom/maintenance.html": change the
documented maintenance file path to the actual mounted location
("customization/nginx/maintenance.html") and add a brief note that
docker-compose mounts ./customization/nginx into /opt/openwisp/public/custom so
placing maintenance.html there enables maintenance mode.
In `@images/common/collectstatic.py`:
- Around line 79-87: The call to run_collectstatic() should include the --clear
flag only when static_changed is true so stale files in STATIC_ROOT are removed;
modify the branch that checks pip_changed or static_changed to call
run_collectstatic("--clear") when static_changed is true and run_collectstatic()
without the flag otherwise, leaving the existing
redis_connection.set("pip_freeze_hash", current_pip_hash) and
redis_connection.set("static_custom_hash", current_static_hash) logic intact;
locate the conditional surrounding pip_changed/static_changed and the
run_collectstatic() invocation to implement this change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: efe678e1-ba46-4982-b0eb-1c57e626ba95
📒 Files selected for processing (4)
docker-compose.ymldocs/user/customization.rstimages/common/collectstatic.pyimages/common/openwisp/settings.py
📜 Review details
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2026-01-06T11:55:13.672Z
Learnt from: pandafy
Repo: openwisp/docker-openwisp PR: 549
File: docker-compose.yml:85-85
Timestamp: 2026-01-06T11:55:13.672Z
Learning: In docker-compose.yml files, avoid leaving CELERY_SERVICE_NETWORK_MODE as an empty string; an empty value is ignored and Docker Compose falls back to default networking. Do not rely on setting CELERY_SERVICE_NETWORK_MODE to 'bridge' for celery services, as this will not affect their networking. If specific networking is required for celery services, define and attach explicit networks in the compose file and reference them on the celery services.
Applied to files:
docker-compose.yml
📚 Learning: 2026-01-06T11:56:48.600Z
Learnt from: pandafy
Repo: openwisp/docker-openwisp PR: 549
File: docker-compose.yml:85-85
Timestamp: 2026-01-06T11:56:48.600Z
Learning: In docker-openwisp projects, ensure CELERY_SERVICE_NETWORK_MODE is set to an empty string "" (which Docker Compose treats as unset/null). This allows containers to connect via the Compose default network with correct service name DNS resolution. Using "bridge" as the value disables service name resolution and breaks communication between celery, dashboard, postgres, and redis. Apply this guideline to docker-compose.yml files in the repository and any similar Compose files where CELERY services rely on service name DNS.
Applied to files:
docker-compose.yml
🔇 Additional comments (6)
docker-compose.yml (2)
32-32: Mounting theme assets intodashboardis the right integration point.This routes custom files through Django’s staticfiles pipeline instead of serving them out-of-band.
132-132: Splitting Nginx-only custom files intocustomization/nginxlooks good.Keeping
/opt/openwisp/public/customseparate fromstatic_customavoids mixing Nginx overrides with Django-managed assets.images/common/openwisp/settings.py (1)
287-287:STATICFILES_DIRSnow points at the custom theme root correctly.This makes
customization/theme/custom/...resolvable as/static/custom/...duringcollectstatic.images/common/collectstatic.py (1)
29-51: The directory hash helper is deterministic.Sorting
dirsandfilesbefore hashing keeps change detection stable across restarts.docs/user/customization.rst (2)
27-27: Addingcustomization/nginxto the bootstrap commands is useful.It matches the new bind mount and makes the directory layout explicit for first-time setups.
88-105: The revised theme workflow instructions match the new container layout.Pointing users at
customization/theme/custom/...and rerunningcollectstaticondashboardis consistent with the compose change.
| To enable maintenance mode, create a ``maintenance.html`` file in the | ||
| ``customization/custom/maintenance.html`` created above. |
There was a problem hiding this comment.
The maintenance-mode path is documented incorrectly.
docker-compose.yml now mounts ./customization/nginx into /opt/openwisp/public/custom, so following customization/custom/maintenance.html will not enable the feature.
✏️ Suggested doc fix
-To enable maintenance mode, create a ``maintenance.html`` file in the
-``customization/custom/maintenance.html`` created above.
+To enable maintenance mode, create a ``maintenance.html`` file at
+``customization/nginx/maintenance.html``.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| To enable maintenance mode, create a ``maintenance.html`` file in the | |
| ``customization/custom/maintenance.html`` created above. | |
| To enable maintenance mode, create a ``maintenance.html`` file at | |
| ``customization/nginx/maintenance.html``. |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/user/customization.rst` around lines 190 - 191, Update the
docs/user/customization.rst entry that currently points to
"customization/custom/maintenance.html": change the documented maintenance file
path to the actual mounted location ("customization/nginx/maintenance.html") and
add a brief note that docker-compose mounts ./customization/nginx into
/opt/openwisp/public/custom so placing maintenance.html there enables
maintenance mode.
| if pip_changed or static_changed: | ||
| print( | ||
| "Changes in Python dependencies or static_custom detected," | ||
| " running collectstatic..." | ||
| ) | ||
| run_collectstatic() | ||
| redis_connection.set("pip_freeze_hash", current_pip_hash) | ||
| try: | ||
| redis_connection.set("pip_freeze_hash", current_pip_hash) | ||
| redis_connection.set("static_custom_hash", current_static_hash) |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does Django's collectstaticcommand remove files fromSTATIC_ROOTwhen the source file is deleted, or is the--clear option required to remove stale files?
💡 Result:
collectstatic does not automatically remove files from STATIC_ROOT when you delete them from the source locations. On subsequent runs, it mainly copies files that are newer than what’s already in STATIC_ROOT, so deleted/removed files can be left behind as stale files. [1]
To remove those stale files, you should run collectstatic --clear (or otherwise wipe STATIC_ROOT) so Django clears the existing files first and then recopies what currently exists. [1]
🏁 Script executed:
# First, let's look at the file to understand the context and structure
cat -n images/common/collectstatic.pyRepository: openwisp/docker-openwisp
Length of output: 4100
Use --clear flag when static files change to prevent serving deleted assets.
Django's collectstatic command does not remove files from STATIC_ROOT when they are deleted from the source directory—it only copies newer files. This means deleting or renaming a file under static_custom leaves the old file in openwisp_static, and Nginx continues serving an asset that no longer exists in the source tree.
To fix this, pass --clear=static_changed when calling run_collectstatic() so the --clear flag is used only when static files change, ensuring stale files are removed.
🛠️ Suggested fix
-def run_collectstatic():
+def run_collectstatic(clear=False):
try:
- subprocess.run(
- [sys.executable, "manage.py", "collectstatic", "--noinput"], check=True
- )
+ command = [sys.executable, "manage.py", "collectstatic", "--noinput"]
+ if clear:
+ command.append("--clear")
+ subprocess.run(command, check=True)
except subprocess.CalledProcessError as e:
print(f"Error running 'collectstatic': {e}", file=sys.stderr)
sys.exit(1)
@@
- run_collectstatic()
+ run_collectstatic(clear=static_changed)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@images/common/collectstatic.py` around lines 79 - 87, The call to
run_collectstatic() should include the --clear flag only when static_changed is
true so stale files in STATIC_ROOT are removed; modify the branch that checks
pip_changed or static_changed to call run_collectstatic("--clear") when
static_changed is true and run_collectstatic() without the flag otherwise,
leaving the existing redis_connection.set("pip_freeze_hash", current_pip_hash)
and redis_connection.set("static_custom_hash", current_static_hash) logic
intact; locate the conditional surrounding pip_changed/static_changed and the
run_collectstatic() invocation to implement this change.
Checklist
Description of Changes
Bug:
Previously, when the OPENWISP_ADMIN_THEME_LINKS setting included any CSS or JavaScript file paths starting with /static/, the OpenWISP utilities logic would strip the /static/ prefix. As a result, Django couldn't recognize these custom static files, preventing them from being collected and served. Additionally, static files were previously mounted directly in the Nginx container, bypassing Django’s static collection workflow entirely.
Fix:
Mount custom theme assets inside the dashboard container and integrate them into Django’s staticfiles pipeline, ensuring they are processed by collectstatic and correctly served via Nginx.