Skip to content

Commit

Permalink
Tidy up post code review, redux
Browse files Browse the repository at this point in the history
  • Loading branch information
zerolab committed Oct 25, 2024
1 parent 5a30698 commit 4e5a24d
Show file tree
Hide file tree
Showing 13 changed files with 98 additions and 85 deletions.
2 changes: 1 addition & 1 deletion .pylintrc
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ ignore=CVS
# ignore-list. The regex matches against paths and can be in Posix or Windows
# format. Because '\\' represents the directory delimiter on Windows systems,
# it can't be used as an escape character.
ignore-paths=.*/migrations
ignore-paths=.*/migrations,.venv,venv,.mypy_cache,node_modules

# Files or directories matching the regular expression patterns are skipped.
# The regex matches against base names, not paths. The default value ignores
Expand Down
10 changes: 9 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,12 @@ format-py: ## Format the Python code
format-html: ## Format the HTML code
find cms/ -name '*.html' | xargs poetry run djhtml

.PHONY: format-frontend
format-frontend: ## Format front-end files (CSS, JS, YAML, MD)
npm run format

.PHONY: lint
lint: lint-py lint-html ## Run all linters (python, html)
lint: lint-py lint-html lint-frontend ## Run all linters (python, html, front-end)

.PHONY: lint-py
lint-py: ## Run all Python linters (ruff/pylint/mypy).
Expand All @@ -43,6 +47,10 @@ lint-py: ## Run all Python linters (ruff/pylint/mypy).
lint-html: ## Run HTML Linters
find cms/ -name '*.html' | xargs poetry run djhtml --check

.PHONY: lint-frontend
lint-frontend: ## Run front-end linters
npm run lint

.PHONY: test
test: ## Run the tests and check coverage.
poetry run pytest -n auto --ds=cms.settings.test --cov=cms --cov-report term-missing
Expand Down
67 changes: 37 additions & 30 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
[![poetry-managed](https://img.shields.io/badge/poetry-managed-blue)](https://python-poetry.org/)
[![License - MIT](https://img.shields.io/badge/licence%20-MIT-1ac403.svg)](https://github.com/ONSdigital/dis-wagtail/blob/main/LICENSE)

The Django Wagtail CMS for managing and publishing content for the Office for National Statistics (ONS)
The Wagtail CMS for managing and publishing content for the Office for National Statistics (ONS)

---

Expand All @@ -21,17 +21,23 @@ The Django Wagtail CMS for managing and publishing content for the Office for Na

- [Getting Started](#getting-started)
- [Pre-requisites](#pre-requisites)
- [Installation](#installation)
- [Setup](#setup)
- [Using Docker](#using-docker)
- [Development](#development)
- [Front-end tooling](#front-end-tooling)
- [Adding Python packages](#adding-python-packages)
- [Run Tests with Coverage](#run-tests-with-coverage)
- [Linting and Formatting](#linting-and-formatting)
- [Front-end](#front-end)
- [Contributing](#contributing)
- [License](#license)
<!-- markdown-link-check-enable -->

For further developer documentation see [docs](docs/index.md)

## Getting Started

To get a local copy up and running, follow these simple steps.
To get a local copy up and running, follow the steps below.

### Pre-requisites

Expand All @@ -43,6 +49,7 @@ Ensure you have the following installed:
environments.
3. **[Docker](https://docs.docker.com/engine/install/)**
4. **Operation System**: Ubuntu/MacOS
5. **[Node](https://nodejs.org/en)** and **[`nvm` (Node Version Manager)](https://github.com/nvm-sh/nvm)** for front-end tooling.

### Setup

Expand All @@ -69,40 +76,33 @@ Ensure you have the following installed:
make install
```

3. Build the docker container

```bash
make docker-build
```

4. Run the docker container

```bash
make docker-start
# ssh into the web container
make docker-shell
# in the web container
# run the migrations. you can use the 'dj' alias for `django-admin`
$ django-admin migrate
# create a superuser
$ django-admin createsuperuser
# run the server. alias: djrun
$ django-admin runserver 0.0.0.0:8000
```

### Using Docker
#### Using Docker

```bash
git clone https://github.com/ONSdigital/dis-wagtail
cd dis-wagtail
# build the container
make docker-build
# start the container
make docker-start
```

This will start the containers in the background, but not Django. To do this, connect to the web container with `make docker-shell` and run `honcho start` to start both
Django and the scheduler in the foreground, or just `djrun`.
This will start the containers in the background, but not Django. To do this, connect to the web container with
`make docker-shell` and run `honcho start` to start both Django and the scheduler in the foreground, or just `djrun`:

```bash
# ssh into the web container
make docker-shell
# run the server. alias: djrun
$ django-admin runserver 0.0.0.0:8000
```

Note: don't forget to run `django-admin migrate` and `django-admin createsuperuser` if you've just set the project up:

Note: don't forget to run `django-admin migrate` and `django-admin createsuperuser` if you've just set the project up.
```bash
# In the web container, run the migrations. you can use the 'dj' alias for `django-admin`
$ django-admin migrate
# create a superuser
$ django-admin createsuperuser
```

Upon first starting the container, the static files may not exist, or may be out of date.
To resolve this, run `make load-design-system-templates`.
Expand Down Expand Up @@ -210,6 +210,13 @@ $ pre-commit install
$ pre-commit run --all-files
```

The `detect-secrets` pre-commit hook requires a baseline secrets file to be included. If you need to, \
you can update this file, e.g. when adding dummy secrets for unit tests:

```bash
$ detect-secrets scan > .secrets.baseline
```

#### MegaLinter (Lint/Format non-python files)

[MegaLinter](https://github.com/oxsecurity/megalinter) is utilised to lint the non-python files in the project.
Expand Down
9 changes: 3 additions & 6 deletions cms/core/blocks/embeddable.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,21 +72,18 @@ class Meta: # pylint: disable=missing-class-docstring,too-few-public-methods
template = "templates/components/streamfield/documents_block.html"


ONS_EMBED_PREFIX = "https://www.ons.gov.uk/visualisations/"


class ONSEmbedBlock(blocks.StructBlock):
"""An embed block for only pages starting with ONS_EMBED_PREFIX."""

url = blocks.URLBlock(help_text=f"Must start with <code>{ ONS_EMBED_PREFIX }</code> to your URL.")
url = blocks.URLBlock(help_text=f"Must start with <code>{ settings.ONS_EMBED_PREFIX }</code> to your URL.")
title = blocks.CharBlock(default="Interactive chart")

def clean(self, value: "StructValue") -> "StructValue":
"""Checks that the given URL matches the ONS_EMBED_PREFIX."""
errors = {}

if not value["url"].startswith(ONS_EMBED_PREFIX):
errors["url"] = ValidationError(f"The URL must start with {ONS_EMBED_PREFIX}")
if not value["url"].startswith(settings.ONS_EMBED_PREFIX):
errors["url"] = ValidationError(f"The URL must start with {settings.ONS_EMBED_PREFIX}")

if errors:
raise StructBlockValidationError(errors)
Expand Down
2 changes: 1 addition & 1 deletion cms/core/blocks/markup.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@


class HeadingBlock(blocks.CharBlock):
"""Seection heading block."""
"""Section heading block."""

class Meta: # pylint: disable=missing-class-docstring,too-few-public-methods
icon = "title"
Expand Down
22 changes: 10 additions & 12 deletions cms/jinja2/templates/pages/wagtail/password_required.html
Original file line number Diff line number Diff line change
Expand Up @@ -19,26 +19,24 @@ <h1>Password required</h1>

{% from "components/password/_macro.njk" import onsPassword %}
<div class="ons-question ons-u-mb-l">
{# fmt:off #}
{{
onsPassword({
"id": form.password.id_for_label,
"name": form.password.name,
"label": {
"text": form.password.label
}
})
onsPassword({
"id": form.password.id_for_label,
"name": form.password.name,
"label": {
"text": form.password.label
}
})
}}
{# fmt:on #}
</div>

{% for field in form.hidden_fields() %}
{{ field }}
{% endfor %}

{% from "components/button/_macro.njk" import onsButton %}
{{
onsButton({
"text": _("Continue")
})
}}
{{ onsButton({"text": _("Continue")}) }}
</form>
{% endblock %}
6 changes: 6 additions & 0 deletions cms/settings/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -765,5 +765,11 @@
# from third-party applications like PayPal or Zoom as needed
SECURE_CROSS_ORIGIN_OPENER_POLICY = "same-origin"

#
# ONS CMS specific-settings
#

# Date formats
SHORT_DATETIME_FORMAT = "d/m/Y P"

ONS_EMBED_PREFIX = env.get("ONS_EMBED_PREFIX", "https://www.ons.gov.uk/visualisations/")
19 changes: 0 additions & 19 deletions docs/continuous-integration.md

This file was deleted.

2 changes: 1 addition & 1 deletion docs/custom-features/migration_friendly_streamfields.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# Migration-friendly StreamFields

This project uses a custom field class (`ons.utils.fields.StreamField`) instead of the usual `wagtail.fields.StreamField` field for streamfield content. This customised field helps with a few things that we often struggle with on busy projects, especially in the early stages:
This project uses a custom field class (`cms.core.fields.StreamField`) instead of the usual `wagtail.fields.StreamField` field for streamfield content. This customised field helps with a few things that we often struggle with on busy projects, especially in the early stages:

1. It keeps block definitions out of migration files, meaning the migrations themselves are much smaller, take less time to lint/format, and keeps the Django's `makemigrations` command nice and snappy.
2. Making changes to block definition no longer requires an accompanying database migration, leading to fewer migrations overall.
Expand Down
18 changes: 8 additions & 10 deletions docs/index.md
Original file line number Diff line number Diff line change
@@ -1,23 +1,21 @@
# ONS Technical Documentation

[TOC]

## Project overview

To work on this project, see the README.
To work on this project, see the [README](../README.md).

## External integrations
## Table of contents

List here any external services this project uses. Preferably link to a separate documentation page for each.

Include any external credentials in the README.
- [Type hinting](conventions/type_hinting.md)

## Unique/custom features

Any unique/custom features of note are listed here. Please keep this list updated as more features are addded.
Any unique/custom features of note are listed here. Please keep this list updated as more features are added.

- [Migration-friendly StreamFields](custom-features/migration_friendly_streamfields.md)

## Adding documentation
## External integrations

List here any external services this project uses. Preferably link to a separate documentation page for each.

A navigational index of the documentation files is in the mkdocs.yml file. Add any new markdown files to that index. You can also link directly to files where necessary, using markdown formatting and relative URLs.
Include any external credentials in the README.
8 changes: 5 additions & 3 deletions gunicorn.conf.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
# pylint: disable=invalid-name
import os

import gunicorn

# Tell gunicorn to run our app
Expand All @@ -8,14 +10,14 @@
gunicorn.SERVER = ""

# Restart gunicorn worker processes every 1200-1250 requests
max_requests = 1200
max_requests_jitter = 50
max_requests = int(os.environ.get("GUNICORN_MAX_REQUESTS", 1200))
max_requests_jitter = int(os.environ.get("GUNICORN_MAX_REQUESTS_JITTER", 50))

# Log to stdout
accesslog = "-"

# Time out after 25 seconds (notably shorter than Heroku's)
timeout = 25
timeout = int(os.environ.get("GUNICORN_TIMEOUT", 25))

# Load app pre-fork to save memory and worker startup time
preload_app = True
Expand Down
17 changes: 16 additions & 1 deletion poetry.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ pytest-django ="^4.9.0"
wagtail-factories = "^4.1.0"

dslr = "^0.4.0"
django-debug-toolbar = "^4.4.6"
pudb = "^2024.1"
tblib = "^3.0.0"
Werkzeug = "~3.0.3"
Expand Down

0 comments on commit 4e5a24d

Please sign in to comment.