diff --git a/CHANGELOG.md b/CHANGELOG.md index dbba860..7e13a5c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,17 @@ All notable changes to wuttaweb will be documented in this file. The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/) and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html). +## v0.25.1 (2025-12-20) + +### Fix + +- add `WebDiff` class now that `Diff` lives in wuttjamaican +- expose fallback key for email settings +- expose transaction comment for version history +- show display text for related objects, in version diff +- discard non-declared field values for grid vue data +- prevent error in DateTime schema type if no widget/request set + ## v0.25.0 (2025-12-17) ### Feat diff --git a/pyproject.toml b/pyproject.toml index 1cd984b..17e3c94 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -6,7 +6,7 @@ build-backend = "hatchling.build" [project] name = "WuttaWeb" -version = "0.25.0" +version = "0.25.1" description = "Web App for Wutta Framework" readme = "README.md" authors = [{name = "Lance Edgar", email = "lance@wuttaproject.org"}] @@ -44,13 +44,13 @@ dependencies = [ "pyramid_tm", "waitress", "WebHelpers2", - "WuttJamaican[db]>=0.26.0", + "WuttJamaican[db]>=0.27.0", "zope.sqlalchemy>=1.5", ] [project.optional-dependencies] -continuum = ["Wutta-Continuum>=0.2.2"] +continuum = ["Wutta-Continuum>=0.3.0"] docs = ["Sphinx", "furo", "sphinxcontrib-programoutput"] tests = ["pylint", "pytest", "pytest-cov", "tox"] diff --git a/src/wuttaweb/diffs.py b/src/wuttaweb/diffs.py index 9747fd0..cb59ed5 100644 --- a/src/wuttaweb/diffs.py +++ b/src/wuttaweb/diffs.py @@ -29,60 +29,20 @@ import sqlalchemy as sa from pyramid.renderers import render from webhelpers2.html import HTML +from wuttjamaican.diffs import Diff -class Diff: + +class WebDiff(Diff): """ - Represent / display a basic "diff" between two data records. + Simple diff class for the web app. - You must provide both the "old" and "new" data records, when - constructing an instance of this class. Then call - :meth:`render_html()` to display the diff table. - - :param old_data: Dict of "old" data record. - - :param new_data: Dict of "new" data record. - - :param fields: Optional list of field names. If not specified, - will be derived from the data records. - - :param nature: What sort of diff is being represented; must be one - of: ``("create", "update", "delete")`` - - :param old_color: Background color to display for "old/deleted" - field data, when applicable. - - :param new_color: Background color to display for "new/created" - field data, when applicable. + This is based on the + :class:`~wuttjamaican:wuttjamaican.diffs.Diff` class; it just + tweaks :meth:`render_html()` to use the web template lookup + engine. """ - def __init__( # pylint: disable=too-many-arguments,too-many-positional-arguments - self, - old_data: dict, - new_data: dict, - fields: list = None, - nature="update", - old_color="#ffebe9", - new_color="#dafbe1", - ): - self.old_data = old_data - self.new_data = new_data - self.columns = ["field name", "old value", "new value"] - self.fields = fields or self.make_fields() - self.nature = nature - self.old_color = old_color - self.new_color = new_color - - def make_fields(self): # pylint: disable=missing-function-docstring - return sorted(set(self.old_data) | set(self.new_data), key=lambda x: x.lower()) - - def old_value(self, field): # pylint: disable=missing-function-docstring - return self.old_data.get(field) - - def new_value(self, field): # pylint: disable=missing-function-docstring - return self.new_data.get(field) - - def values_differ(self, field): # pylint: disable=missing-function-docstring - return self.new_value(field) != self.old_value(field) + cell_padding = None def render_html(self, template="/diff.mako", **kwargs): """ @@ -98,69 +58,25 @@ class Diff: """ context = kwargs context["diff"] = self - return HTML.literal(render(template, context)) - - def render_field_row(self, field): # pylint: disable=missing-function-docstring - is_diff = self.values_differ(field) - - td_field = HTML.tag("td", class_="field", c=field) - - td_old_value = HTML.tag( - "td", - c=self.render_old_value(field), - **self.get_old_value_attrs(is_diff), - ) - - td_new_value = HTML.tag( - "td", - c=self.render_new_value(field), - **self.get_new_value_attrs(is_diff), - ) - - return HTML.tag("tr", c=[td_field, td_old_value, td_new_value]) - - def render_old_value(self, field): # pylint: disable=missing-function-docstring - value = self.old_value(field) - return repr(value) - - def render_new_value(self, field): # pylint: disable=missing-function-docstring - value = self.new_value(field) - return repr(value) - - def get_old_value_attrs( # pylint: disable=missing-function-docstring - self, is_diff - ): - attrs = {} - if self.nature == "update" and is_diff: - attrs["style"] = f"background-color: {self.old_color};" - elif self.nature == "delete": - attrs["style"] = f"background-color: {self.old_color};" - return attrs - - def get_new_value_attrs( # pylint: disable=missing-function-docstring - self, is_diff - ): - attrs = {} - if self.nature == "create": - attrs["style"] = f"background-color: {self.new_color};" - elif self.nature == "update" and is_diff: - attrs["style"] = f"background-color: {self.new_color};" - return attrs + html = render(template, context) + return HTML.literal(html) -class VersionDiff(Diff): +class VersionDiff(WebDiff): """ - Special diff class, for use with version history views. Note that - while based on :class:`Diff`, this class uses a different - signature for the constructor. + Special diff class for use with version history views. While + based on :class:`WebDiff`, this class uses a different signature + for the constructor. - :param version: Reference to a Continuum version record (object). + :param config: The app :term:`config object`. + + :param version: Reference to a Continuum version record object. :param \\**kwargs: Remaining kwargs are passed as-is to the - :class:`Diff` constructor. + :class:`WebDiff` constructor. """ - def __init__(self, version, **kwargs): + def __init__(self, config, version, **kwargs): import sqlalchemy_continuum as continuum # pylint: disable=import-outside-toplevel from wutta_continuum.util import ( # pylint: disable=import-outside-toplevel render_operation_type, @@ -195,7 +111,7 @@ class VersionDiff(Diff): old_data[field] = getattr(version.previous, field) new_data[field] = getattr(version, field) - super().__init__(old_data, new_data, **kwargs) + super().__init__(config, old_data, new_data, **kwargs) def get_default_fields(self): # pylint: disable=missing-function-docstring fields = sorted(self.version_mapper.columns.keys()) @@ -208,17 +124,76 @@ class VersionDiff(Diff): return [field for field in fields if field not in unwanted] - def render_version_value(self, value): # pylint: disable=missing-function-docstring - return HTML.tag("span", c=[repr(value)], style="font-family: monospace;") + def render_version_value(self, version, field, value): + """ + Render the cell value HTML for a given version + field. + + This method is used to render both sides of the diff (old + + new values). It will just render the field value using a + monospace font by default. However: + + If the field is involved in a mapper relationship (i.e. it is + the "foreign key" to a related table), the logic here will + also (try to) traverse that show display text for the related + object (if found). + + :param version: Reference to the Continuum version object. + + :param field: Name of the field, as string. + + :param value: Raw value for the field, as obtained from the + version object. + + :returns: Rendered cell value as HTML literal + """ + # first render normal span; this is our fallback but also may + # be embedded within a more complex result. + text = HTML.tag("span", c=[repr(value)], style="font-family: monospace;") + + # loop thru all mapped relationship props + for prop in self.mapper.relationships: + + # we only want singletons + if prop.uselist: + continue + + # loop thru columns for prop + # nb. there should always be just one colum for a + # singleton prop, but technically a list is used, so no + # harm in looping i assume.. + for col in prop.local_columns: + + # we only want the matching column + if col.name != field: + continue + + # grab "related version" reference via prop key. this + # would be like a UserVersion for instance. + if ref := getattr(version, prop.key): + + # grab "related object" reference. this would be + # like a User for instance. + if ref := getattr(ref, "version_parent", None): + + # render text w/ related object as bold string + style = ( + "margin-left: 2rem; font-style: italic; font-weight: bold;" + ) + return HTML.tag( + "span", + c=[text, HTML.tag("span", c=[str(ref)], style=style)], + ) + + return text def render_old_value(self, field): if self.nature == "create": return "" value = self.old_value(field) - return self.render_version_value(value) + return self.render_version_value(self.version.previous, field, value) def render_new_value(self, field): if self.nature == "delete": return "" value = self.new_value(field) - return self.render_version_value(value) + return self.render_version_value(self.version, field, value) diff --git a/src/wuttaweb/forms/schema.py b/src/wuttaweb/forms/schema.py index 37db3e9..e002c0b 100644 --- a/src/wuttaweb/forms/schema.py +++ b/src/wuttaweb/forms/schema.py @@ -31,6 +31,7 @@ import colander import sqlalchemy as sa from wuttjamaican.conf import parse_list +from wuttjamaican.util import localtime from wuttaweb.db import Session from wuttaweb.forms import widgets @@ -38,28 +39,38 @@ from wuttaweb.forms import widgets class WuttaDateTime(colander.DateTime): """ - Custom schema type for ``datetime`` fields. + Custom schema type for :class:`~python:datetime.datetime` fields. This should be used automatically for - :class:`sqlalchemy:sqlalchemy.types.DateTime` columns unless you - register another default. + :class:`~sqlalchemy:sqlalchemy.types.DateTime` ORM columns unless + you register another default. This schema type exists for sake of convenience, when working with the Buefy datepicker + timepicker widgets. + + It also follows the datetime handling "rules" as outlined in + :doc:`wuttjamaican:narr/datetime`. On the Python side, values + should be naive/UTC datetime objects. On the HTTP side, values + will be ISO-format strings representing aware/local time. """ def serialize(self, node, appstruct): if not appstruct: return colander.null - request = node.widget.request - config = request.wutta_config - app = config.get_app() + # nb. request should be present when it matters + if node.widget and node.widget.request: + request = node.widget.request + config = request.wutta_config + app = config.get_app() + appstruct = app.localtime(appstruct) + else: + # but if not, fallback to config-less logic + appstruct = localtime(appstruct) - dt = app.localtime(appstruct) if self.format: - return dt.strftime(self.format) - return dt.isoformat() + return appstruct.strftime(self.format) + return appstruct.isoformat() def deserialize( # pylint: disable=inconsistent-return-statements self, node, cstruct @@ -72,6 +83,7 @@ class WuttaDateTime(colander.DateTime): "%Y-%m-%dT%I:%M %p", ] + # nb. request is always assumed to be present here request = node.widget.request config = request.wutta_config app = config.get_app() diff --git a/src/wuttaweb/grids/base.py b/src/wuttaweb/grids/base.py index 81c92ae..f0f45a1 100644 --- a/src/wuttaweb/grids/base.py +++ b/src/wuttaweb/grids/base.py @@ -2390,6 +2390,9 @@ class Grid: # pylint: disable=too-many-instance-attributes,too-many-public-meth # convert record to new dict record = self.object_to_dict(record) + # discard non-declared fields + record = {field: record[field] for field in record if field in self.columns} + # make all values safe for json record = make_json_safe(record, warn=False) diff --git a/src/wuttaweb/templates/master/view_version.mako b/src/wuttaweb/templates/master/view_version.mako index 361055e..8e838e8 100644 --- a/src/wuttaweb/templates/master/view_version.mako +++ b/src/wuttaweb/templates/master/view_version.mako @@ -24,6 +24,10 @@ ${transaction.id} + + ${transaction.meta.get("comment", "")} + +
diff --git a/src/wuttaweb/views/email.py b/src/wuttaweb/views/email.py index 981faff..b71ce51 100644 --- a/src/wuttaweb/views/email.py +++ b/src/wuttaweb/views/email.py @@ -63,6 +63,7 @@ class EmailSettingView(MasterView): # pylint: disable=abstract-method form_fields = [ "key", + "fallback_key", "description", "subject", "sender", @@ -92,9 +93,11 @@ class EmailSettingView(MasterView): # pylint: disable=abstract-method def normalize_setting(self, setting): # pylint: disable=empty-docstring """ """ key = setting.__name__ + setting = setting(self.config) return { "key": key, - "description": setting.__doc__, + "fallback_key": setting.fallback_key or "", + "description": setting.get_description() or "", "subject": self.email_handler.get_auto_subject( key, rendered=False, setting=setting ), @@ -158,8 +161,12 @@ class EmailSettingView(MasterView): # pylint: disable=abstract-method f = form super().configure_form(f) + # fallback_key + f.set_readonly("fallback_key") + # description f.set_readonly("description") + f.set_widget("description", "notes") # replyto f.set_required("replyto", False) @@ -247,11 +254,12 @@ class EmailSettingView(MasterView): # pylint: disable=abstract-method if self.viewing: setting = context["instance"] context["setting"] = setting + context["has_html_template"] = self.email_handler.get_auto_body_template( - setting["key"], "html" + setting["key"], "html", fallback_key=setting["fallback_key"] ) context["has_txt_template"] = self.email_handler.get_auto_body_template( - setting["key"], "txt" + setting["key"], "txt", fallback_key=setting["fallback_key"] ) return super().render_to_response(template, context) @@ -269,11 +277,15 @@ class EmailSettingView(MasterView): # pylint: disable=abstract-method mode = self.request.params.get("mode", "html") if mode == "txt": - body = self.email_handler.get_auto_txt_body(key, context) + body = self.email_handler.get_auto_txt_body( + key, context, fallback_key=setting.fallback_key + ) self.request.response.content_type = "text/plain" else: # html - body = self.email_handler.get_auto_html_body(key, context) + body = self.email_handler.get_auto_html_body( + key, context, fallback_key=setting.fallback_key + ) self.request.response.text = body return self.request.response diff --git a/src/wuttaweb/views/master.py b/src/wuttaweb/views/master.py index aa24c89..57baaff 100644 --- a/src/wuttaweb/views/master.py +++ b/src/wuttaweb/views/master.py @@ -1145,6 +1145,7 @@ class MasterView(View): # pylint: disable=too-many-public-methods "issued_at", "user", "remote_addr", + "comment", ] def get_version_grid_data(self, instance): @@ -1197,6 +1198,14 @@ class MasterView(View): # pylint: disable=too-many-public-methods # remote_addr g.set_label("remote_addr", "IP Address") + # comment + g.set_renderer("comment", self.render_version_comment) + + def render_version_comment( # pylint: disable=missing-function-docstring,unused-argument + self, txn, key, value + ): + return txn.meta.get("comment", "") + def view_version(self): # pylint: disable=too-many-locals """ View to show diff details for a particular object version. @@ -1260,7 +1269,7 @@ class MasterView(View): # pylint: disable=too-many-public-methods ) version_diffs = [ - VersionDiff(version) + VersionDiff(self.config, version) for version in self.get_relevant_versions(txn, instance) ] diff --git a/tests/forms/test_schema.py b/tests/forms/test_schema.py index a01ce35..6c587fb 100644 --- a/tests/forms/test_schema.py +++ b/tests/forms/test_schema.py @@ -62,6 +62,13 @@ class TestWuttaDateTime(WebTestCase): ) self.assertEqual(result, "2024-12-11 02:33 PM") + # missing widget/request/config + typ = mod.WuttaDateTime() + node = colander.SchemaNode(typ) + result = typ.serialize(node, datetime.datetime(2024, 12, 11, 22, 33)) + # nb. not possible to know which timezone is system-local + self.assertTrue(result.startswith("2024-12-")) + def test_deserialize(self): tzlocal = get_timezone_by_name("America/Los_Angeles") with patch.object(self.app, "get_timezone", return_value=tzlocal): diff --git a/tests/grids/test_base.py b/tests/grids/test_base.py index b0927e6..0087e93 100644 --- a/tests/grids/test_base.py +++ b/tests/grids/test_base.py @@ -1867,7 +1867,19 @@ class TestGrid(WebTestCase): context = grid.get_vue_context() self.assertEqual(context, {"data": [{"foo": "bar"}], "row_classes": {}}) + # non-declared columns are discarded + mydata = [ + {"foo": "a", "bar": "b", "baz": "c"}, + ] + grid = self.make_grid(columns=["bar"], data=mydata) + context = grid.get_vue_context() + self.assertEqual(context, {"data": [{"bar": "b"}], "row_classes": {}}) + # if grid has actions, that list may be supplemented + mydata = [ + {"foo": "bar"}, + ] + grid = self.make_grid(columns=["foo"], data=mydata) grid.actions.append(mod.GridAction(self.request, "view", url="/blarg")) context = grid.get_vue_context() self.assertIsNot(context["data"], mydata) diff --git a/tests/test_diffs.py b/tests/test_diffs.py index ee76314..00c78a9 100644 --- a/tests/test_diffs.py +++ b/tests/test_diffs.py @@ -4,114 +4,10 @@ from wuttaweb import diffs as mod from wuttaweb.testing import WebTestCase, VersionWebTestCase -# nb. using WebTestCase here only for mako support in render_html() -class TestDiff(WebTestCase): +class TestWebDiff(WebTestCase): def make_diff(self, *args, **kwargs): - return mod.Diff(*args, **kwargs) - - def test_constructor(self): - old_data = {"foo": "bar"} - new_data = {"foo": "baz"} - diff = self.make_diff(old_data, new_data, fields=["foo"]) - self.assertEqual(diff.fields, ["foo"]) - - def test_make_fields(self): - old_data = {"foo": "bar"} - new_data = {"foo": "bar", "baz": "zer"} - # nb. this calls make_fields() - diff = self.make_diff(old_data, new_data) - # TODO: should the fields be cumulative? or just use new_data? - self.assertEqual(diff.fields, ["baz", "foo"]) - - def test_values(self): - old_data = {"foo": "bar"} - new_data = {"foo": "baz"} - diff = self.make_diff(old_data, new_data) - self.assertEqual(diff.old_value("foo"), "bar") - self.assertEqual(diff.new_value("foo"), "baz") - - def test_values_differ(self): - old_data = {"foo": "bar"} - new_data = {"foo": "baz"} - diff = self.make_diff(old_data, new_data) - self.assertTrue(diff.values_differ("foo")) - - old_data = {"foo": "bar"} - new_data = {"foo": "bar"} - diff = self.make_diff(old_data, new_data) - self.assertFalse(diff.values_differ("foo")) - - def test_render_values(self): - old_data = {"foo": "bar"} - new_data = {"foo": "baz"} - diff = self.make_diff(old_data, new_data) - self.assertEqual(diff.render_old_value("foo"), "'bar'") - self.assertEqual(diff.render_new_value("foo"), "'baz'") - - def test_get_old_value_attrs(self): - - # no change - old_data = {"foo": "bar"} - new_data = {"foo": "bar"} - diff = self.make_diff(old_data, new_data, nature="update") - self.assertEqual(diff.get_old_value_attrs(False), {}) - - # update - old_data = {"foo": "bar"} - new_data = {"foo": "baz"} - diff = self.make_diff(old_data, new_data, nature="update") - self.assertEqual( - diff.get_old_value_attrs(True), - {"style": f"background-color: {diff.old_color};"}, - ) - - # delete - old_data = {"foo": "bar"} - new_data = {} - diff = self.make_diff(old_data, new_data, nature="delete") - self.assertEqual( - diff.get_old_value_attrs(True), - {"style": f"background-color: {diff.old_color};"}, - ) - - def test_get_new_value_attrs(self): - - # no change - old_data = {"foo": "bar"} - new_data = {"foo": "bar"} - diff = self.make_diff(old_data, new_data, nature="update") - self.assertEqual(diff.get_new_value_attrs(False), {}) - - # update - old_data = {"foo": "bar"} - new_data = {"foo": "baz"} - diff = self.make_diff(old_data, new_data, nature="update") - self.assertEqual( - diff.get_new_value_attrs(True), - {"style": f"background-color: {diff.new_color};"}, - ) - - # create - old_data = {} - new_data = {"foo": "bar"} - diff = self.make_diff(old_data, new_data, nature="create") - self.assertEqual( - diff.get_new_value_attrs(True), - {"style": f"background-color: {diff.new_color};"}, - ) - - def test_render_field_row(self): - old_data = {"foo": "bar"} - new_data = {"foo": "baz"} - diff = self.make_diff(old_data, new_data) - row = diff.render_field_row("foo") - self.assertIn("", row) - self.assertIn("'bar'", row) - self.assertIn(f'style="background-color: {diff.old_color};"', row) - self.assertIn("'baz'", row) - self.assertIn(f'style="background-color: {diff.new_color};"', row) - self.assertIn("", row) + return mod.WebDiff(self.config, *args, **kwargs) def test_render_html(self): old_data = {"foo": "bar"} @@ -121,9 +17,9 @@ class TestDiff(WebTestCase): self.assertIn("", html) self.assertIn("'bar'", html) - self.assertIn(f'style="background-color: {diff.old_color};"', html) + self.assertIn(f'style="background-color: {diff.old_color}"', html) self.assertIn("'baz'", html) - self.assertIn(f'style="background-color: {diff.new_color};"', html) + self.assertIn(f'style="background-color: {diff.new_color}"', html) self.assertIn("", html) self.assertIn("", html) @@ -131,7 +27,7 @@ class TestDiff(WebTestCase): class TestVersionDiff(VersionWebTestCase): def make_diff(self, *args, **kwargs): - return mod.VersionDiff(*args, **kwargs) + return mod.VersionDiff(self.config, *args, **kwargs) def test_constructor(self): import sqlalchemy_continuum as continuum @@ -174,11 +70,15 @@ class TestVersionDiff(VersionWebTestCase): ["active", "person_uuid", "prevent_edit", "username", "uuid"], ) - def test_render_values(self): + def test_render_version_value(self): import sqlalchemy_continuum as continuum model = self.app.model - user = model.User(username="fred") + person = model.Person(full_name="Fred Flintstone") + self.session.add(person) + + # create, update, delete user + user = model.User(username="fred", person=person) self.session.add(user) self.session.commit() user.username = "freddie" @@ -191,32 +91,42 @@ class TestVersionDiff(VersionWebTestCase): versions = self.session.query(vercls).order_by(vercls.transaction_id).all() self.assertEqual(len(versions), 3) + # create (1st version) version = versions[0] diff = self.make_diff(version) self.assertEqual(diff.nature, "create") self.assertEqual(diff.render_old_value("username"), "") - self.assertEqual( - diff.render_new_value("username"), - ''fred'', - ) + self.assertIn("fred", diff.render_new_value("username")) + self.assertNotIn("freddie", diff.render_new_value("username")) + self.assertEqual(diff.render_old_value("person_uuid"), "") + # rendered person_uuid includes display name + html = diff.render_new_value("person_uuid") + self.assertIn(str(person.uuid), html) + self.assertIn("Fred Flintstone", html) + # update (2nd version) version = versions[1] diff = self.make_diff(version) self.assertEqual(diff.nature, "update") - self.assertEqual( - diff.render_old_value("username"), - ''fred'', - ) - self.assertEqual( - diff.render_new_value("username"), - ''freddie'', - ) + self.assertIn("fred", diff.render_old_value("username")) + self.assertNotIn("freddie", diff.render_old_value("username")) + self.assertIn("freddie", diff.render_new_value("username")) + # rendered person_uuid includes display name + html = diff.render_old_value("person_uuid") + self.assertIn(str(person.uuid), html) + self.assertIn("Fred Flintstone", html) + html = diff.render_new_value("person_uuid") + self.assertIn(str(person.uuid), html) + self.assertIn("Fred Flintstone", html) + # delete (3rd version) version = versions[2] diff = self.make_diff(version) self.assertEqual(diff.nature, "delete") - self.assertEqual( - diff.render_old_value("username"), - ''freddie'', - ) + self.assertIn("freddie", diff.render_old_value("username")) self.assertEqual(diff.render_new_value("username"), "") + # rendered person_uuid includes display name + html = diff.render_old_value("person_uuid") + self.assertIn(str(person.uuid), html) + self.assertIn("Fred Flintstone", html) + self.assertEqual(diff.render_new_value("person_uuid"), "") diff --git a/tests/views/test_master.py b/tests/views/test_master.py index 5807ca8..cbaeb62 100644 --- a/tests/views/test_master.py +++ b/tests/views/test_master.py @@ -2115,7 +2115,7 @@ class TestVersionedMasterView(VersionWebTestCase): view = self.make_view() self.assertEqual( view.get_version_grid_columns(), - ["id", "issued_at", "user", "remote_addr"], + ["id", "issued_at", "user", "remote_addr", "comment"], ) # custom