Skip to content

Commit

Permalink
Merge branch 'release' into 'master'
Browse files Browse the repository at this point in the history
Release 69.11 prep

See merge request buckinghamshire-council/bc!673
  • Loading branch information
nickmoreton committed Nov 27, 2023
2 parents ab99ede + 9f26012 commit 58e6352
Show file tree
Hide file tree
Showing 20 changed files with 170 additions and 79 deletions.
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,13 @@

## Unreleased

## 69.11 (2023-11-27)

- Add a template for the RedirectPage type
- Add a check for empty columns in the PieChartBlock
- Add a Retry to the BucksMapsClient
- Upgrade Wagtail to 5.0

## 69.10 (2023-10-24)

Compare: <https://git.torchbox.com/buckinghamshire-council/bc/compare/69.9...69.10>
Expand Down
19 changes: 17 additions & 2 deletions bc/area_finder/client.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
from django.conf import settings

import requests
from requests import Session
from requests.adapters import HTTPAdapter
from urllib3.util import Retry


class BucksMapsClient:
Expand Down Expand Up @@ -43,7 +45,20 @@ class BucksMapsClient:
}

def _post(self, data):
response = requests.post(
# Set up a session with retries for handling Sentry ConnectionError in production
# for further information see documentation at docs/postcode-lookup.md
self.session = Session()

retries = Retry(
total=3,
backoff_factor=0.1,
status_forcelist=[502, 503, 504, 598],
allowed_methods={"POST"},
)

self.session.mount(self.base_url, adapter=HTTPAdapter(max_retries=retries))

response = self.session.post(
self.base_url,
data=data,
timeout=settings.BUCKS_MAPS_CLIENT_API_TIMEOUT_SECONDS,
Expand Down
9 changes: 9 additions & 0 deletions bc/area_finder/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -363,3 +363,12 @@ def test_timeout(self, mock_client):
self.assertEqual(resp.headers["content-type"], "application/json")
json_response = resp.json()
self.assertIn("error", json_response)

@mock.patch("bc.area_finder.views.BucksMapsClient")
def test_connectionerror(self, mock_client):
mock_client().query_postcode.side_effect = ConnectionError
resp = self.client.get(self.url + "?postcode=W1A+1AA")
self.assertEqual(resp.status_code, 400)
self.assertEqual(resp.headers["content-type"], "application/json")
json_response = resp.json()
self.assertIn("error", json_response)
2 changes: 1 addition & 1 deletion bc/area_finder/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ def area_finder(request):

try:
resp = client.query_postcode(formatted_postcode)
except (HTTPError, Timeout):
except (HTTPError, Timeout, ConnectionError):
return JsonResponse(
{"error": "Request failed, try again"}, status=status.HTTP_400_BAD_REQUEST
)
Expand Down
4 changes: 2 additions & 2 deletions bc/migration_utils/handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ def handle_page(page, attrs, mapper):


def handle_revision(revision, attrs, mapper):
revision_content = json.loads(revision.content_json)
revision_content = revision.content

should_save = False
for attr in attrs:
Expand All @@ -57,7 +57,7 @@ def handle_revision(revision, attrs, mapper):

if mapped:
revision_content[attr] = json.dumps(stream_data)
revision.content_json = json.dumps(revision_content)
revision.content = revision_content

if should_save:
revision.save()
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{% extends base_page_template %}

{% block content %}
<div class="wrapper wrapper--flex">
<div class="section section--main">
<h1 class="heading heading--xxl">Redirect Page preview</h1>
<p class="section__text">This is only place holder content.</p>
<p>A redirect page is not shown to a site visitor because the redirect will take place before it's displayed.</p>
</div>
</div>
{% endblock %}
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
context:
base_page_template: 'patterns/base_page.html'
3 changes: 1 addition & 2 deletions bc/search/tests/test_search_promotions.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
from django.test import TestCase, override_settings
from django.urls import reverse

from wagtail.contrib.search_promotions.models import SearchPromotion
from wagtail.contrib.search_promotions.models import Query, SearchPromotion
from wagtail.models import Page
from wagtail.search.models import Query

from bc.home.models import HomePage
from bc.standardpages.tests.fixtures import InformationPageFactory
Expand Down
3 changes: 1 addition & 2 deletions bc/search/tests/test_search_queries.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,9 @@
from django.test import RequestFactory, TestCase, override_settings
from django.urls import reverse

from wagtail.contrib.search_promotions.models import SearchPromotion
from wagtail.contrib.search_promotions.models import Query, SearchPromotion
from wagtail.models import Page, Site
from wagtail.search.backends import get_search_backend
from wagtail.search.models import Query

from bc.home.models import HomePage
from bc.home.tests.fixtures import HomePageFactory
Expand Down
2 changes: 1 addition & 1 deletion bc/search/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@
from django.views.decorators.csrf import csrf_exempt
from django.views.generic.base import View

from wagtail.contrib.search_promotions.models import Query
from wagtail.models import Page, Site
from wagtail.search.models import Query

import backoff
import elasticsearch
Expand Down
4 changes: 4 additions & 0 deletions bc/standardpages/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,10 @@ def get_context(self, request, *args, **kwargs):


class RedirectPage(BasePage):
# This page type returns a redirect, the template is never served on the frontend.
# In the Wagtail admin, without a template it will throw an error when the preview is opened.
template = "patterns/pages/standardpages/redirect_page.html"

internal_page = models.ForeignKey(
"wagtailcore.Page",
null=True,
Expand Down
4 changes: 0 additions & 4 deletions bc/static_src/vendor/wagtail_transfer/css/modal.css

This file was deleted.

32 changes: 23 additions & 9 deletions bc/utils/blocks.py
Original file line number Diff line number Diff line change
Expand Up @@ -457,19 +457,33 @@ def render(self, value, context=None):
# Get total value
total_value = 0
for row in cleaned_data:
if is_number(row[1]):
total_value += float(row[1])
try:
# The preview will update when key presses are made, so we need to
# handle the case where the second column has no value.
# When the second column is completed, the preview will update again
# and the total value will be correct.
if is_number(row[1]):
total_value += float(row[1])
except IndexError:
pass # Fails silently

# Use percentage values for the chart
series_data = []
for row in cleaned_data:
if is_number(row[1]):
series_value = round(float(row[1]) / total_value * 100, 1)
else:
series_value = row[1]
series = {"name": row[0], "y": series_value}
series_data.append(series)
row.append(f"{series_value}%")
try:
# The preview will update when key presses are made, so we need to
# handle the case where the second column has no value.
# When the second column is completed, the preview will update again
# and the total value will be correct.
if is_number(row[1]):
series_value = round(float(row[1]) / total_value * 100, 1)
else:
series_value = row[1]
series = {"name": row[0], "y": series_value}
series_data.append(series)
row.append(f"{series_value}%")
except IndexError:
pass # Fails silently

new_value = {"chart": {"type": "pie"}, "series": [{"data": series_data}]}

Expand Down
3 changes: 2 additions & 1 deletion bc/utils/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

from wagtail import blocks
from wagtail.admin.panels import FieldPanel, MultiFieldPanel
from wagtail.admin.widgets.slug import SlugInput
from wagtail.contrib.settings.models import BaseSiteSetting, register_setting
from wagtail.fields import RichTextField, StreamField
from wagtail.models import Orderable, Page
Expand Down Expand Up @@ -345,7 +346,7 @@ class Meta:
[
MultiFieldPanel(
[
FieldPanel("slug"),
FieldPanel("slug", widget=SlugInput),
FieldPanel("seo_title"),
FieldPanel("show_in_menus"),
FieldPanel("search_description"),
Expand Down
2 changes: 1 addition & 1 deletion bc/utils/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ def is_number(s):
try:
float(s)
return True
except (ValueError, TypeError):
except (IndexError, ValueError, TypeError):
return False


Expand Down
11 changes: 0 additions & 11 deletions bc/utils/wagtail_hooks.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
from django.templatetags.static import static
from django.urls import path, reverse
from django.utils.html import format_html
from django.utils.safestring import mark_safe

from wagtail import hooks
Expand Down Expand Up @@ -154,16 +153,6 @@ def register_big_text_feature(features):
features.register_converter_rule("contentstate", feature_name, db_conversion)


# This style has been added so that the close icon in the modal dialog is visible e.g. not white.
# It can be removed entirely once the package is updated to be compatible with wagtail 4+.
@hooks.register("insert_global_admin_css")
def wagtail_transfer_admin_fix_css():
return format_html(
'<link rel="stylesheet" href="{}">',
static("vendor/wagtail_transfer/css/modal.css"),
)


@hooks.register("insert_editor_js")
def editor_js():
return mark_safe(
Expand Down
4 changes: 4 additions & 0 deletions docs/postcode-lookup.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,3 +17,7 @@ If all the results for a query match a single district council (most queries), w
The Python client is at `bc.area_finder.client.BucksMapsClient`. The timeout for the upstream API can be configured with the environment variable:

- `BUCKS_MAPS_CLIENT_API_TIMEOUT_SECONDS` (must be parsable as a `float`; defaults to `10`)
- Retries are implemented to handle a Sentry ConnectionError see in production.
- `backoff_factor`: Increase in sleep time between retries (10%).
- `status_forcelist`: List of status codes to retry on.
- `allowed_methods`: Set of uppercased HTTP method verbs for retries.
22 changes: 14 additions & 8 deletions docs/upgrading.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,6 @@

This document describes aspects of the system which should be given particular attention when upgrading Wagtail or its dependencies.

## Known technical debt

Wagtail transfer isn't currently completely compatible with the wagtail version use here. v4.1

There is a admin hook in place bc/utils/wagtail_hooks.py (wagtail_transfer_admin_fix_css) to inject some temporary css.

Once Wagtail transfer is made compatible the template can be removed.

## Critical paths

The following areas of functionality are critical paths for the site which don't have full automated tests and should be checked manually.
Expand All @@ -34,3 +26,17 @@ As well as testing the critical paths, these areas of functionality should be ch
- This could be code which you know should be checked and possibly removed - e.g. because you've patched something until a fix is merged in a subsequent release.
- Any previous fixes which may need to be updated/reapplied on subsequent upgrades
- Technical debt which could be affected by an upgrade.

## Forked Wagtail package dependencies

As much as possible, we want to use the official releases available on PyPI for the Wagtail package dependencies.

However, in certain situations, critical fixes and upgrades may be pending approval, merging, or release.
A temporary solution is to fork the package dependency, tag the working branch, and use the tag in the pyproject file.

The following packages are forked at the time of the latest upgrade (Wagtail 5.0):

- `wagtail-django-recaptcha`
- `wagtail-transfer`

Please note that it is important to replace the usage of the git tags in the pyproject.toml file with the official release version from PyPI as soon as it becomes available. This ensures that we maintain compatibility with the official releases and benefit from any subsequent updates and improvements provided by the original package maintainers.
Loading

0 comments on commit 58e6352

Please sign in to comment.