Skip to content

FIXME: Move format methods from Layout to utils #1835

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 4 additions & 69 deletions src/onegov/core/layout.py
Original file line number Diff line number Diff line change
@@ -1,21 +1,18 @@
from __future__ import annotations


import arrow
import babel.dates
import babel.numbers
import isodate
import numbers
import sedate

from datetime import datetime
from functools import cached_property
from functools import lru_cache
from onegov.core import utils
from onegov.core.templates import PageTemplate
from pytz import timezone

from typing import overload, Any, TypeVar, TYPE_CHECKING

if TYPE_CHECKING:
from chameleon import PageTemplateFile
from collections.abc import Callable, Collection, Iterable, Iterator
Expand Down Expand Up @@ -154,40 +151,8 @@ def csrf_protected_url(self, url: str) -> str:
return self.request.csrf_protected_url(url)

def format_date(self, dt: datetime | date | None, format: str) -> str:
""" Takes a datetime and formats it according to local timezone and
the given format.

"""
if dt is None:
return ''

if getattr(dt, 'tzinfo', None) is not None:
dt = self.timezone.normalize(
dt.astimezone(self.timezone) # type:ignore[attr-defined]
)

locale = self.request.locale
assert locale is not None, 'Cannot format date without a locale'
if format == 'relative':
adt = arrow.get(dt)

try:
return adt.humanize(locale=locale)
except ValueError:
return adt.humanize(locale=locale.split('_')[0])

fmt = getattr(self, format + '_format')
if fmt.startswith('skeleton:'):
return babel.dates.format_skeleton(
fmt.replace('skeleton:', ''),
datetime=dt,
fuzzy=False,
locale=locale
)
elif hasattr(dt, 'hour'):
return babel.dates.format_datetime(dt, format=fmt, locale=locale)
else:
return babel.dates.format_date(dt, format=fmt, locale=locale)
fmt = getattr(self, f'{format}_format', format)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would move this part to CoreRequest as well, we should never manually specify a fmt string, it should always be one of our pre-configured format options. If one of the applications overwrites the formats in their specific layout class, you would need to instead overwrite them in their specific request class, but I'm not sure if we do overwrite them anywhere.

return self.request.format_date(dt, fmt)

def isodate(self, date: datetime) -> str:
""" Returns the given date in the ISO 8601 format. """
Expand All @@ -197,43 +162,13 @@ def parse_isodate(self, string: str) -> datetime:
""" Returns the given ISO 8601 string as datetime. """
return isodate.parse_datetime(string)

@staticmethod
@lru_cache(maxsize=8)
def number_symbols(locale: str) -> tuple[str, str]:
""" Returns the locale specific number symbols. """

return (
babel.numbers.get_decimal_symbol(locale),
babel.numbers.get_group_symbol(locale)
)

def format_number(
self,
number: numbers.Number | Decimal | float | str | None,
decimal_places: int | None = None,
padding: str = ''
) -> str:
""" Takes the given numer and formats it according to locale.

If the number is an integer, the default decimal places are 0,
otherwise 2.

"""
if isinstance(number, str):
return number

if number is None:
return ''

if decimal_places is None:
if isinstance(number, numbers.Integral):
decimal_places = 0
else:
decimal_places = 2

decimal, group = self.number_symbols(self.request.locale)
result = '{{:{},.{}f}}'.format(padding, decimal_places).format(number)
return result.translate({ord(','): group, ord('.'): decimal})
return self.request.format_number(number, decimal_places, padding)

@property
def view_name(self) -> str | None:
Expand Down
87 changes: 85 additions & 2 deletions src/onegov/core/request.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,16 @@
from __future__ import annotations

import arrow
import babel.dates
import babel.numbers
import morepath
import numbers

import pytz
import ua_parser

from datetime import timedelta
from functools import cached_property
from datetime import timedelta, datetime, date
from functools import cached_property, lru_cache
from onegov.core.cache import instance_lru_cache
from onegov.core.custom import msgpack
from onegov.core.utils import append_query_param
Expand All @@ -29,6 +35,7 @@
if TYPE_CHECKING:
from _typeshed import SupportsItems
from collections.abc import Callable, Iterable, Iterator, Sequence
from decimal import Decimal
from dectate import Sentinel
from gettext import GNUTranslations
from markupsafe import Markup
Expand Down Expand Up @@ -874,3 +881,79 @@
""" Returns the chameleon template loader. """
registry = self.app.config.template_engine_registry
return registry._template_loaders['.pt']

def format_date(
self,
dt: datetime | date | None,
fmt: str,
timezone: pytz.BaseTzInfo | None = None
) -> str:
""" Takes a datetime and formats it according to local timezone and
the given format.
"""
if dt is None:
return ''

if getattr(dt, 'tzinfo', None) is not None:
timezone = timezone or pytz.timezone('Europe/Zurich')
dt = timezone.normalize(
dt.astimezone(timezone) # type:ignore[attr-defined]
)

locale = self.locale
assert locale is not None, 'Cannot format date without a locale'
if fmt == 'relative':
adt = arrow.get(dt)

try:
return adt.humanize(locale=locale)
except ValueError:
return adt.humanize(locale=locale.split('_')[0])

Check warning on line 911 in src/onegov/core/request.py

View check run for this annotation

Codecov / codecov/patch

src/onegov/core/request.py#L910-L911

Added lines #L910 - L911 were not covered by tests

if fmt.startswith('skeleton:'):
return babel.dates.format_skeleton(
fmt.replace('skeleton:', ''),
datetime=dt,
fuzzy=False,
locale=locale
)
elif hasattr(dt, 'hour'):
return babel.dates.format_datetime(dt, format=fmt, locale=locale)
else:
return babel.dates.format_date(dt, format=fmt, locale=locale)

def format_number(
self,
number: numbers.Number | Decimal | float | str | None,
decimal_places: int | None = None,
padding: str = ''
) -> str:
""" Takes the given numer and formats it according to locale.
If the number is an integer, the default decimal places are 0,
otherwise 2.
"""
if isinstance(number, str):
return number

if number is None:
return ''

if decimal_places is None:
if isinstance(number, numbers.Integral):
decimal_places = 0
else:
decimal_places = 2

decimal, group = self.number_symbols(self.locale)
result = '{{:{},.{}f}}'.format(padding, decimal_places).format(number)
return result.translate({ord(','): group, ord('.'): decimal})

@staticmethod
@lru_cache(maxsize=8)
def number_symbols(locale: str) -> tuple[str, str]:
""" Returns the locale specific number symbols. """

return (
babel.numbers.get_decimal_symbol(locale),
babel.numbers.get_group_symbol(locale)
)
21 changes: 5 additions & 16 deletions src/onegov/org/forms/newsletter.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@
from onegov.core.csv import convert_excel_to_csv, CSVFile
from onegov.form.fields import UploadField
from onegov.org.forms.fields import HtmlField
from onegov.org.utils import extract_categories_and_subcategories
from onegov.form.validators import FileSizeLimit
from onegov.form.validators import WhitelistedMimeType
from wtforms.fields import BooleanField
from onegov.core.layout import Layout
from onegov.file.utils import name_without_extension
from onegov.form import Form
from onegov.form.fields import ChosenSelectField
Expand All @@ -29,8 +29,6 @@

from typing import Any, TYPE_CHECKING

from onegov.org.utils import extract_categories_and_subcategories

if TYPE_CHECKING:
from collections.abc import Iterable, Callable
from onegov.core.csv import DefaultRow
Expand Down Expand Up @@ -85,10 +83,6 @@ def with_news(
news: Iterable[News]
) -> type[Self]:

# FIXME: using a layout just for format_date seems bad, we should
# probably extract these functions into util modules
layout = Layout(None, request)

choices = tuple(
(
str(item.id),
Expand All @@ -97,7 +91,7 @@ def with_news(
'<div class="date">{}</div>'
).format(
item.title,
layout.format_date(item.created, 'relative')
request.format_date(item.created, 'relative'),
)
)
for item in news
Expand Down Expand Up @@ -149,9 +143,6 @@ def with_occurrences(
occurrences: Iterable[Occurrence]
) -> type[Self]:

# FIXME: another use of layout for format_date
layout = Layout(None, request)

choices = tuple(
(
str(item.id),
Expand All @@ -160,7 +151,8 @@ def with_occurrences(
'<div class="date">{}</div>'
).format(
item.title,
layout.format_date(item.localized_start, 'datetime')
request.format_date(
item.localized_start, 'dd.MM.yyyy HH:mm')
)
)
for item in occurrences
Expand Down Expand Up @@ -201,9 +193,6 @@ def with_publications(
publications: Iterable[File]
) -> type[Self]:

# FIXME: another use of layout for format_date
layout = Layout(None, request)

choices = tuple(
(
str(item.id),
Expand All @@ -212,7 +201,7 @@ def with_publications(
'<div class="date">{}</div>'
).format(
name_without_extension(item.name),
layout.format_date(item.created, 'date')
request.format_date(item.created, 'dd.MM.yyyy')
)
)
for item in publications
Expand Down
2 changes: 1 addition & 1 deletion src/onegov/swissvotes/layouts/default.py
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,6 @@ def format_number(
# Fixes using "," for french locale instead of "." as for german
if locale == 'fr_CH':
locale = 'de_CH'
decimal, group = self.number_symbols(locale)
decimal, group = self.request.number_symbols(locale)
result = '{{:{},.{}f}}'.format(padding, decimal_places).format(number)
return result.translate({ord(','): group, ord('.'): decimal})
16 changes: 16 additions & 0 deletions tests/onegov/core/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import transaction
import yaml

from tests.onegov.org.conftest import create_org_app
from tests.shared import Client
from tests.shared.utils import create_app
from onegov.core.cli import command_group
Expand Down Expand Up @@ -174,3 +175,18 @@ def maildir_smtp_app(temporary_directory, maildir, smtp):
app.smtp = smtp

return app


@pytest.fixture(scope='function')
def org_app(request):
yield create_org_app(request, use_elasticsearch=False)
Comment on lines +180 to +182
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't use OrgApp in core tests, the tests here are supposed to pass with a CoreRequest without any org specific features, so you should create a basic Framework instance here like e.g. maildir_app above.



@pytest.fixture(scope='function')
def core_request(org_app):
yield org_app.request_class(environ={
'PATH_INFO': '/',
'SERVER_NAME': '',
'SERVER_PORT': '',
'SERVER_PROTOCOL': 'https'
}, app=org_app)
18 changes: 12 additions & 6 deletions tests/onegov/core/test_layout.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,10 @@ def test_batched():
]


def test_format_date():
def test_format_date(core_request):
layout = Layout(
model=object(),
request=Bunch(app=Bunch(version='1.0', sentry_dsn=None))
request=core_request
)

dt = replace_timezone(datetime(2015, 6, 17, 15, 0), 'Europe/Zurich')
Expand All @@ -41,10 +41,10 @@ def test_format_date():
assert layout.format_date(dt, 'day_long') == '17. Juni'


def test_format_number():
def test_format_number(core_request):
layout = Layout(
model=object(),
request=Bunch(app=Bunch(version='1.0', sentry_dsn=None))
request=core_request
)

layout.request.locale = 'de_CH'
Expand Down Expand Up @@ -78,10 +78,16 @@ def test_format_number():
assert layout.format_number(None) == ""


def test_relative_date():
def test_relative_date(core_request):
layout = Layout(
model=object(),
request=Bunch(locale='en', app=Bunch(version='1.0', sentry_dsn=None))
request=core_request
)

# default locale is de_CH
text = layout.format_date(utcnow() - timedelta(seconds=60 * 5), 'relative')
assert text == 'vor 5 Minuten'

core_request.locale = 'en_US'
text = layout.format_date(utcnow() - timedelta(seconds=60 * 5), 'relative')
assert text == '5 minutes ago'
Loading