diff --git a/requirements.txt b/requirements.txt index 3a7cc6a..c0149db 100644 --- a/requirements.txt +++ b/requirements.txt @@ -306,7 +306,7 @@ sqlalchemy[asyncio]==1.4.31 # prefect # sqlalchemy-continuum # sqlalchemy-utils -sqlalchemy-continuum==1.3.14 +sqlalchemy-continuum==1.4.0 # via -r requirements.in sqlalchemy-utils==0.38.2 # via sqlalchemy-continuum diff --git a/tourist/__init__.py b/tourist/__init__.py index c5ad9d8..a5f4acc 100644 --- a/tourist/__init__.py +++ b/tourist/__init__.py @@ -141,45 +141,25 @@ def uwht_redirect(): @event.listens_for(db.session, "before_flush") def before_flush(session, flush_context, instances): - update_render_after_flush = False for instance in session.new | session.dirty: if not session.is_modified(instance): continue - if isinstance(instance, (tstore.Entity, tstore.EntityChild)): - update_render_after_flush = True if isinstance(instance, tstore.Entity): instance.validate() - for instance in session.deleted: - if isinstance(instance, (tstore.Entity, tstore.EntityChild)): - update_render_after_flush = True - - if update_render_after_flush: - session.info[UPDATE_RENDER_AFTER_FLUSH] = True - - @event.listens_for(db.session, "after_flush_postexec") - def after_flush_postexec(session, flush_context): - if session.info.get(UPDATE_RENDER_AFTER_FLUSH, False): - # ORM model objects that haven't been loaded from the database are slightly different - # from those populated from a form. In particular `region` is a str instead of - # geometry type. Instead of changing the render_factory to handle both types force - # objects used for the render to be refreshed from the database. - # expire_all() breaks some login tests so expire only pool/place/club objects. - for instance in session.identity_map.values(): - if isinstance(instance, tstore.Entity): - session.expire(instance) - new_cache_ids = [] - for new_cache in render_factory.yield_cache(): - session.add(new_cache) - new_cache_ids.append(new_cache.name) - # Remove rows in RenderCache not in new_cache_ids. This should be removed places. - session.query(tstore.RenderCache).filter(tstore.RenderCache.name.notin_( - new_cache_ids)).delete() - del session.info[UPDATE_RENDER_AFTER_FLUSH] - return app +def update_render_cache(session): + new_cache_ids = [] + for new_cache in render_factory.yield_cache(): + session.add(new_cache) + new_cache_ids.append(new_cache.name) + # Remove rows in RenderCache not in new_cache_ids. This should be removed places. + session.query(tstore.RenderCache).filter(tstore.RenderCache.name.notin_(new_cache_ids)).delete() + session.commit() + + def initialise_logger(app): """ Read environment config then initialise a 100MB rotating log """ log_dir = app.config['LOG_DIR'] diff --git a/tourist/models/render.py b/tourist/models/render.py index e865366..07f3c6b 100644 --- a/tourist/models/render.py +++ b/tourist/models/render.py @@ -101,6 +101,22 @@ class RecentlyUpdated: source_name: Optional[str] = None +@attrs.frozen() +class PlaceEntityChanges: + """Changes for an entity (place, club, pool) in a very crude format, but good enough for + debugging.""" + + @attrs.frozen() + class Change: + """A single version of an entity""" + timestamp: datetime.datetime + user: str + change: str + + entity_name: str + changes: List[Change] = attrs.field(factory=list) + + @attrs.frozen() class Place: id: int @@ -114,7 +130,10 @@ class Place: child_places: List[ChildPlace] parents: List[ChildPlace] comments: List[PlaceComment] = attrs.field(factory=list) + # recently_updated, only set for 'world' recently_updated: Optional[List[RecentlyUpdated]] = None + # changes for this place and all direct children. not set for 'world' + changes: Optional[List[PlaceEntityChanges]] = None @attrs.frozen() diff --git a/tourist/render_factory.py b/tourist/render_factory.py index c7258a3..dcc6ed7 100644 --- a/tourist/render_factory.py +++ b/tourist/render_factory.py @@ -2,7 +2,9 @@ import datetime import enum import io +import itertools from typing import List, Mapping +from typing import Union from sqlalchemy.util import IdentitySet @@ -59,6 +61,20 @@ def _build_render_pool(orm_pool: tstore.Pool) -> render.Pool: ) +def _build_changes(orm_entity: Union[tstore.Place, tstore.Club, tstore.Pool]) -> ( + render.PlaceEntityChanges): + changes = render.PlaceEntityChanges(entity_name=orm_entity.name) + + for v in orm_entity.versions: + user_email = None + if v.transaction.user: + user_email = v.transaction.user.email + changes.changes.append(render.PlaceEntityChanges.Change( + timestamp=v.transaction.issued_at, user=user_email, + change=str(v.changeset))) + return changes + + def _build_render_place(orm_place: tstore.Place, source_by_short_name: Mapping[str, render.ClubSource]) -> render.Place: children_geojson = orm_place.children_geojson_features if children_geojson: @@ -106,8 +122,14 @@ def _build_render_place(orm_place: tstore.Place, source_by_short_name: Mapping[s recently_updated.append(render.RecentlyUpdated( timestamp=source.sync_timestamp, path=place.path, place_name=place.name, source_name=source.name)) recently_updated.sort(key=lambda ru: ru.timestamp, reverse=True) + entity_changes = None else: recently_updated = None + entity_changes = [_build_changes(orm_place)] + for child in itertools.chain(orm_place.child_places, orm_place.child_pools, + orm_place.child_clubs): + entity_changes.append(_build_changes(child)) + return render.Place( id=orm_place.id, @@ -122,6 +144,7 @@ def _build_render_place(orm_place: tstore.Place, source_by_short_name: Mapping[s parents=parents, recently_updated=recently_updated, comments=comments, + changes=entity_changes, ) @@ -196,12 +219,6 @@ def _build_problems(all_places: List[tstore.Place], all_clubs: List[tstore.Club] return problems - - - - - - def yield_cache(): def get_all(cls): all_objects = IdentitySet(cls.query.all()) | tstore.db.session.dirty | tstore.db.session.new diff --git a/tourist/routes.py b/tourist/routes.py index 1cbab9d..120f090 100644 --- a/tourist/routes.py +++ b/tourist/routes.py @@ -126,6 +126,7 @@ def edit_club(club_id): form_delete_place_comments() flask.flash(f"Updated {club.name}") tstore.db.session.commit() + tourist.update_render_cache(tstore.db.session) return redirect(club.path) return render_template("edit_club.html", form=form, club=club) @@ -150,6 +151,7 @@ def edit_place(place_id): form_delete_place_comments() flask.flash(f"Updated {place.name}") tstore.db.session.commit() + tourist.update_render_cache(tstore.db.session) return redirect(place.path) return render_template("edit_place.html", form=form, place=place) @@ -179,6 +181,7 @@ def add_place_comment(place_id): place_comment.akismet_spam_status = tourist.get_comment_spam_status(place_comment) tstore.db.session.add(place_comment) tstore.db.session.commit() + tourist.update_render_cache(tstore.db.session) flask.flash(f"Comment added to {place.name}") else: flask.flash(f"Ignored empty comment for {place.name}") @@ -224,6 +227,7 @@ def delete_place(place_id): parent_path = place.parent.path delete_place_children_and_flash(place) tstore.db.session.commit() + tourist.update_render_cache(tstore.db.session) return redirect(parent_path) return render_template('delete_place.html', form=form, place=place) @@ -242,6 +246,7 @@ def delete_club(club_id): parent_path = club.parent.path delete_club_and_flash(club) tstore.db.session.commit() + tourist.update_render_cache(tstore.db.session) return redirect(parent_path) return render_template('delete_club.html', form=form, club=club) @@ -262,6 +267,7 @@ def delete_pool(pool_id): parent_path = pool.parent.path delete_pool_and_flash(pool) tstore.db.session.commit() + tourist.update_render_cache(tstore.db.session) return redirect(parent_path) return render_template('delete_pool.html', form=form, pool=pool) diff --git a/tourist/scripts/batchtool.py b/tourist/scripts/batchtool.py index 5c114be..01b112c 100644 --- a/tourist/scripts/batchtool.py +++ b/tourist/scripts/batchtool.py @@ -94,6 +94,7 @@ def replace_club_pool_links(write): if write: click.echo('Committing changes') tstore.db.session.commit() + tourist.update_render_cache(tstore.db.session) else: click.echo('Run with --write to commit changes') @@ -161,6 +162,7 @@ def incr_column(cls, column_name: str): if write: click.echo('Committing changes') tstore.db.session.commit() + tourist.update_render_cache(tstore.db.session) else: click.echo('Run with --write to commit changes') @@ -404,6 +406,7 @@ def transactioninsert1(initial_snapshot: str, change_log: str, output_path: str, if commit: click.echo('Committing changes') tstore.db.session.commit() + tourist.update_render_cache(tstore.db.session) else: click.echo('Run with --write to commit changes') @@ -435,5 +438,6 @@ def _find_empty(place: tstore.Place) -> Tuple[bool, List]: tstore.db.session.delete(place) click.echo('Committing changes') tstore.db.session.commit() + tourist.update_render_cache(tstore.db.session) else: click.echo('Run with --write to commit changes') diff --git a/tourist/scripts/scrape.py b/tourist/scripts/scrape.py index f767cdf..90535d7 100644 --- a/tourist/scripts/scrape.py +++ b/tourist/scripts/scrape.py @@ -736,6 +736,7 @@ def extract_gbfeed(uk_place: tstore.Place, feed: GbUwhFeed, fetch_timestamp: dat tstore_source.place_id = uk_place.id tstore.db.session.commit() + tourist.update_render_cache(tstore.db.session) return [] @@ -874,6 +875,7 @@ def comment_command(): print(f"Comment added to {place.short_name}:\n{comment}") tstore.db.session.add_all(comment_to_extract.keys()) tstore.db.session.commit() + tourist.update_render_cache(tstore.db.session) for comment, extract in comment_to_extract.items(): tstore.db.session.refresh(comment) assert comment.id diff --git a/tourist/scripts/sync.py b/tourist/scripts/sync.py index 9fd2872..ab1801d 100644 --- a/tourist/scripts/sync.py +++ b/tourist/scripts/sync.py @@ -9,6 +9,7 @@ import click from prefect.deployments import Deployment +import tourist from tourist.models import tstore, attrib from geoalchemy2.shape import to_shape import attr @@ -226,6 +227,7 @@ def run(self, jsons_iterable: Iterable[str]): print('Updated fields ' + ','.join(self.updater.updated_fields)) tstore.db.session.add_all(self.updater.to_add) tstore.db.session.commit() + tourist.update_render_cache(tstore.db.session) @sync_cli.command('import_jsonl') diff --git a/tourist/templates/place.html b/tourist/templates/place.html index cf68dc9..99febd1 100644 --- a/tourist/templates/place.html +++ b/tourist/templates/place.html @@ -143,11 +143,23 @@

{{ pool.name }} {% if current_user.edit_granted -%} {{ place_comments(place.comments) }} +

Add in {{ place.name }}:

place add club add pool add +
+

History

+ {% else %} {% if place.comments -%}
There are comments about this place. Login to view and handle them.
{% endif %} diff --git a/tourist/tests/test_basic.py b/tourist/tests/test_basic.py index f8e57e2..42cd380 100644 --- a/tourist/tests/test_basic.py +++ b/tourist/tests/test_basic.py @@ -215,6 +215,7 @@ def add_some_entities(test_app): markdown='Some palace', id=1) tstore.db.session.add_all([world, country, metro, club, pool]) tstore.db.session.commit() + tourist.update_render_cache(tstore.db.session) def add_and_return_edit_granted_user(test_app): @@ -269,6 +270,7 @@ def test_list(test_app): ) tstore.db.session.add_all([metro]) tstore.db.session.commit() + tourist.update_render_cache(tstore.db.session) with test_app.test_client() as c: response = c.get('/tourist/list') @@ -284,6 +286,7 @@ def test_club_with_bad_pool_link(test_app): club.markdown += " * [[badpoollink]]\n" tstore.db.session.add(club) tstore.db.session.commit() + tourist.update_render_cache(tstore.db.session) with test_app.test_client() as c: response = c.get('/tourist/place/metro') @@ -393,6 +396,7 @@ def test_delete_pool(test_app): club.markdown = 'Club Foo has no pool' tstore.db.session.add(club) tstore.db.session.commit() + tourist.update_render_cache(tstore.db.session) with test_app.app_context(), test_app.test_client(user=user) as c: c.get('/tourist/delete/pool/1') # GET to create the CSRF token @@ -563,5 +567,6 @@ def test_add_delete_place_comment(test_app, mocker: MockerFixture): tstore.db.session.delete(place.child_clubs[0]) tstore.db.session.delete(place) tstore.db.session.commit() + tourist.update_render_cache(tstore.db.session) # Check that comment_id was implicitly deleted by cascade. assert tstore.PlaceComment.query.get(comment_id) is None diff --git a/tourist/tests/test_login.py b/tourist/tests/test_login.py index dc575c2..d1ee05a 100644 --- a/tourist/tests/test_login.py +++ b/tourist/tests/test_login.py @@ -1,3 +1,4 @@ +import tourist from tourist.models import tstore from tourist.tests.conftest import no_expire_on_commit @@ -13,6 +14,7 @@ def test_heavy(test_app): with no_expire_on_commit(): tstore.db.session.add_all([world, au, user_plain, user_edit_granted]) tstore.db.session.commit() + tourist.update_render_cache(tstore.db.session) with test_app.test_client() as c: response = c.get('/tourist/') diff --git a/tourist/tests/test_render_factory.py b/tourist/tests/test_render_factory.py index 77b79e7..ce3b954 100644 --- a/tourist/tests/test_render_factory.py +++ b/tourist/tests/test_render_factory.py @@ -1,5 +1,6 @@ from geoalchemy2 import WKTElement +import tourist from tourist import render_factory from tourist.models import tstore @@ -34,6 +35,7 @@ def test_geojson(test_app): markdown='', entrance=point1) tstore.db.session.add_all([world, country, metro_with_pool, metro_no_pool, pool, poolgeo]) tstore.db.session.commit() + tourist.update_render_cache(tstore.db.session) with test_app.app_context(): collection = render_factory._build_geojson_feature_collection(tstore.Place.query.all(), diff --git a/tourist/tests/test_scrape.py b/tourist/tests/test_scrape.py index ef98a7f..a5e0202 100644 --- a/tourist/tests/test_scrape.py +++ b/tourist/tests/test_scrape.py @@ -149,6 +149,7 @@ def add_uk(test_app): tstore.db.session.add_all([world, uk, north, london, poolden, poolgeo2]) tstore.db.session.commit() + tourist.update_render_cache(tstore.db.session) def test_extract_gbuwh_short(test_app): @@ -276,6 +277,7 @@ def add_canada(test_app): region=some_region, markdown='') tstore.db.session.add_all([world, ca, cabc, caon]) tstore.db.session.commit() + tourist.update_render_cache(tstore.db.session) def test_load_and_comment(test_app): diff --git a/tourist/tests/test_sync.py b/tourist/tests/test_sync.py index f4d7712..ffe168e 100644 --- a/tourist/tests/test_sync.py +++ b/tourist/tests/test_sync.py @@ -1,6 +1,7 @@ import pytest from geoalchemy2 import WKTElement +import tourist from tourist.models import tstore from tourist.scripts import sync @@ -12,6 +13,7 @@ def test_get_sorted_entities_with_duplicate_short_name(test_app): club = tstore.Club(name='Club Name', short_name='bad_name', parent=place) tstore.db.session.add_all([place, pool, club]) tstore.db.session.commit() + tourist.update_render_cache(tstore.db.session) with pytest.raises(ValueError, match="Duplicates"): sync.get_sorted_entities() @@ -25,4 +27,5 @@ def test_output_place(test_app): cc = tstore.Place(name='CC', short_name='cc', region=some_region, markdown='', parent=world) tstore.db.session.add_all([world, cc]) tstore.db.session.commit() + tourist.update_render_cache(tstore.db.session) sync._output_place(place_short_name=['world', 'cc'])