From 4328b9e38510655a8d14f85ed82e4c28e8d9e804 Mon Sep 17 00:00:00 2001 From: Lance Edgar Date: Tue, 10 Oct 2023 10:54:16 -0500 Subject: [PATCH] Show full version history within the "view" page avoid full page loads when navigating version history --- tailbone/diffs.py | 28 ++- tailbone/grids/core.py | 12 +- tailbone/static/css/layout.css | 13 +- tailbone/templates/base.mako | 157 ++++++------ tailbone/templates/grids/buefy.mako | 19 +- tailbone/templates/master/edit.mako | 3 +- tailbone/templates/master/view.mako | 255 ++++++++++++++++++-- tailbone/templates/master/view_version.mako | 7 +- tailbone/views/master.py | 134 +++++++++- 9 files changed, 498 insertions(+), 130 deletions(-) diff --git a/tailbone/diffs.py b/tailbone/diffs.py index 431c2efe..1c73635a 100644 --- a/tailbone/diffs.py +++ b/tailbone/diffs.py @@ -136,6 +136,9 @@ class VersionDiff(Diff): """ def __init__(self, version, *args, **kwargs): + self.version = version + self.mapper = sa.inspect(continuum.parent_class(type(self.version))) + self.version_mapper = sa.inspect(type(self.version)) self.title = kwargs.pop('title', None) if 'nature' not in kwargs: @@ -146,10 +149,31 @@ class VersionDiff(Diff): else: kwargs['nature'] = 'new' + if 'fields' not in kwargs: + kwargs['fields'] = self.get_default_fields() + + if not args: + old_data = {} + new_data = {} + for field in kwargs['fields']: + if version.previous: + old_data[field] = getattr(version.previous, field) + new_data[field] = getattr(version, field) + args = (old_data, new_data) + super().__init__(*args, **kwargs) - self.version = version - self.mapper = sa.inspect(continuum.parent_class(type(self.version))) + def get_default_fields(self): + fields = sorted(self.version_mapper.columns.keys()) + + unwanted = [ + 'transaction_id', + 'end_transaction_id', + 'operation_type', + ] + + return [field for field in fields + if field not in unwanted] def render_version_value(self, field, value, version): text = HTML.tag('span', c=[repr(value)], diff --git a/tailbone/grids/core.py b/tailbone/grids/core.py index e42f8714..dc1a5af0 100644 --- a/tailbone/grids/core.py +++ b/tailbone/grids/core.py @@ -1334,6 +1334,7 @@ class Grid(object): context['grid'] = self context['request'] = self.request context.setdefault('allow_save_defaults', True) + context.setdefault('view_click_handler', self.get_view_click_handler()) return render(template, context) def render_buefy(self, template='/grids/buefy.mako', **kwargs): @@ -1374,6 +1375,10 @@ class Grid(object): context.setdefault('paginated', False) if context['paginated']: context.setdefault('per_page', 20) + context['view_click_handler'] = self.get_view_click_handler() + return render(template, context) + + def get_view_click_handler(self): # locate the 'view' action # TODO: this should be easier, and/or moved elsewhere? @@ -1388,11 +1393,8 @@ class Grid(object): view = action break - context['view_click_handler'] = None - if view and view.click_handler: - context['view_click_handler'] = view.click_handler - - return render(template, context) + if view: + return view.click_handler def set_filters_sequence(self, filters, only=False): """ diff --git a/tailbone/static/css/layout.css b/tailbone/static/css/layout.css index cc4d0015..bdf35410 100644 --- a/tailbone/static/css/layout.css +++ b/tailbone/static/css/layout.css @@ -61,13 +61,14 @@ header .level .theme-picker { display: inline-flex; } -#content-title { - padding: 0.3rem; -} - #content-title h1 { - font-size: 2rem; - margin-left: 1rem; + margin-bottom: 0; + margin-right: 1rem; + max-width: 50%; + overflow: hidden; + padding: 0 0.3rem; + text-overflow: ellipsis; + white-space: nowrap; } /****************************** diff --git a/tailbone/templates/base.mako b/tailbone/templates/base.mako index 0e767353..8558eeb7 100644 --- a/tailbone/templates/base.mako +++ b/tailbone/templates/base.mako @@ -426,17 +426,22 @@ ## Page Title % if capture(self.content_title): -
-
-
-
-

-
+
+
+ +

+

+ +
${self.render_instance_header_title_extras()}
-
+ +
${self.render_instance_header_buttons()}
+
% endif @@ -634,76 +639,60 @@ ## TODO: is there a better way to check if viewing parent? % if parent_instance is Undefined: % if master.editable and instance_editable and master.has_perm('edit'): -
- - -
+ + % endif % if master.cloneable and master.has_perm('clone'): -
- - -
+ + % endif % if master.deletable and instance_deletable and master.has_perm('delete'): -
- - -
+ + % endif % else: ## viewing row % if instance_deletable and master.has_perm('delete_row'): -
- - -
+ + % endif % endif % elif master and master.editing: % if master.viewable and master.has_perm('view'): -
- - -
+ + % endif % if master.deletable and instance_deletable and master.has_perm('delete'): -
- - -
+ + % endif % elif master and master.deleting: % if master.viewable and master.has_perm('view'): -
- - -
+ + % endif % if master.editable and instance_editable and master.has_perm('edit'): -
- - -
+ + % endif % endif @@ -711,40 +700,32 @@ <%def name="render_prevnext_header_buttons()"> % if show_prev_next is not Undefined and show_prev_next: % if prev_url: -
- - Older - -
+ + Older + % else: -
- - Older - -
+ + Older + % endif % if next_url: -
- - Newer - -
+ + Newer + % else: -
- - Newer - -
+ + Newer + % endif % endif diff --git a/tailbone/templates/grids/buefy.mako b/tailbone/templates/grids/buefy.mako index 5b21b42a..6fdcf77d 100644 --- a/tailbone/templates/grids/buefy.mako +++ b/tailbone/templates/grids/buefy.mako @@ -254,7 +254,12 @@ % if column['field'] in grid.raw_renderers: ${grid.raw_renderers[column['field']]()} % elif grid.is_linked(column['field']): - + + % else: % endif @@ -274,6 +279,9 @@ % if action.click_handler: @click.prevent="${action.click_handler}" % endif + % if action.target: + target="${action.target}" + % endif > ${action.render_icon()|n} ${action.render_label()|n} @@ -533,7 +541,7 @@ ## TODO: i noticed buefy docs show using `async` keyword here, ## so now i am too. knowing nothing at all of if/how this is ## supposed to improve anything. we shall see i guess - async loadAsyncData(params, callback) { + async loadAsyncData(params, success, failure) { if (params === undefined || params === null) { params = new URLSearchParams(this.getBasicParams()) @@ -551,14 +559,17 @@ this.lastItem = data.last_item this.loading = false this.checkedRows = this.locateCheckedRows(data.checked_rows) - if (callback) { - callback() + if (success) { + success() } }) .catch((error) => { this.data = [] this.total = 0 this.loading = false + if (failure) { + failure() + } throw error }) }, diff --git a/tailbone/templates/master/edit.mako b/tailbone/templates/master/edit.mako index f1bc7318..a03912e6 100644 --- a/tailbone/templates/master/edit.mako +++ b/tailbone/templates/master/edit.mako @@ -1,7 +1,8 @@ ## -*- coding: utf-8; -*- <%inherit file="/master/form.mako" /> -<%def name="title()">Edit: ${instance_title} +<%def name="title()">${index_title} » ${instance_title} » Edit +<%def name="content_title()">Edit: ${instance_title} ${parent.body()} diff --git a/tailbone/templates/master/view.mako b/tailbone/templates/master/view.mako index e6d0c8de..b5930664 100644 --- a/tailbone/templates/master/view.mako +++ b/tailbone/templates/master/view.mako @@ -8,7 +8,6 @@ <%def name="render_instance_header_title_extras()"> - % if master.touchable and master.has_perm('touch'): % endif + % if expose_versions: + + {{ viewingHistory ? "View Current" : "View History" }} + + % endif <%def name="object_helpers()"> @@ -46,9 +52,6 @@ ## TODO: either make this configurable, or just lose it. ## nobody seems to ever find it useful in practice. ##
  • ${h.link_to("Permalink for this {}".format(model_title), action_url('view', instance))}
    • - % if master.has_versions and request.rattail_config.versioning_enabled() and request.has_perm('{}.versions'.format(permission_prefix)): -
  • ${h.link_to("Version History", action_url('versions', instance))}
    • - % endif <%def name="render_row_grid_tools()"> @@ -69,14 +72,152 @@ % endif +<%def name="render_this_page_component()"> + ## TODO: should override this in a cleaner way! too much duplicate code w/ parent template + + + + <%def name="render_this_page()"> - ${parent.render_this_page()} - % if master.has_rows: -
    - % if rows_title: -

    ${rows_title}

    - % endif - ${self.render_row_grid_component()} +
    + + ## render main form + ${parent.render_this_page()} + + ## render row grid + % if master.has_rows: +
    + % if rows_title: +

    ${rows_title}

    + % endif + ${self.render_row_grid_component()} + % endif +
    + + % if expose_versions: +
    + +
    +

    Version History

    +

    + + + View as separate page + +

    +
    + + + + + +
    +
    +
    + +
    + +
    + +
    +     + +
    +     + +
    +     + +
    +     + +
    +     +
    + +
    + +
    + + Older + + + Newer + +
    + + + + + {{ viewVersionShowAllFields ? "Show Diffs Only" : "Show All Fields" }} + +
    + +
    + +
    + +

    + {{ version.model_title }} +

    + + + + + + + + + + + + + + + + +
    field nameold valuenew value
    {{ field }}
    + +
    + +
    + +
    +
    +     +
    % endif @@ -90,12 +231,79 @@ ${rows_grid.render_buefy(allow_save_defaults=False, tools=capture(self.render_row_grid_tools))|n} % endif ${parent.render_this_page_template()} + % if expose_versions: + ${versions_grid.render_buefy()|n} + % endif + + +<%def name="modify_this_page_vars()"> + ${parent.modify_this_page_vars()} + % if expose_versions: + + % endif <%def name="modify_whole_page_vars()"> ${parent.modify_whole_page_vars()} - % if master.touchable and master.has_perm('touch'): - - % endif + % endif + + % if expose_versions: + WholePageData.viewingHistory = false + % endif + + <%def name="finalize_this_page_vars()"> ${parent.finalize_this_page_vars()} - % if master.has_rows: - % endif diff --git a/tailbone/templates/master/view_version.mako b/tailbone/templates/master/view_version.mako index d29a3496..6417dfb7 100644 --- a/tailbone/templates/master/view_version.mako +++ b/tailbone/templates/master/view_version.mako @@ -45,13 +45,18 @@
    ${transaction.meta.get('comment') or ''}
    +
    + +
    ${transaction.id}
    +
    +
    % for diff in version_diffs: -

    ${diff.title}

    +

    ${diff.title}

    ${diff.render_html()} % endfor
    diff --git a/tailbone/views/master.py b/tailbone/views/master.py index 167bdace..21418521 100644 --- a/tailbone/views/master.py +++ b/tailbone/views/master.py @@ -1172,6 +1172,12 @@ class MasterView(View): context['rows_grid'] = grid context['rows_grid_tools'] = HTML(self.make_row_grid_tools(instance) or '').strip() + context['expose_versions'] = (self.has_versions + and self.request.rattail_config.versioning_enabled() + and self.has_perm('versions')) + if context['expose_versions']: + context['versions_grid'] = self.make_revisions_grid(instance, empty_data=True) + return self.render_to_response('view', context) def image(self): @@ -1300,7 +1306,7 @@ class MasterView(View): return cls.version_grid_key return '{}.history'.format(cls.get_route_prefix()) - def get_version_data(self, instance): + def get_version_data(self, instance, order_by=True): """ Generate the base data set for the version grid. """ @@ -1308,7 +1314,9 @@ class MasterView(View): transaction_class = continuum.transaction_class(model_class) query = model_transaction_query(self.Session(), instance, model_class, child_classes=self.normalize_version_child_classes()) - return query.order_by(transaction_class.issued_at.desc()) + if order_by: + query = query.order_by(transaction_class.issued_at.desc()) + return query def get_version_child_classes(self): """ @@ -1330,6 +1338,114 @@ class MasterView(View): classes.append(cls) return classes + def make_revisions_grid(self, obj, empty_data=False): + route_prefix = self.get_route_prefix() + row_url = lambda txn, i: self.request.route_url(f'{route_prefix}.version', + uuid=obj.uuid, + txnid=txn.id) + + kwargs = { + 'component': 'versions-grid', + 'ajax_data_url': self.get_action_url('revisions_data', obj), + 'sortable': True, + 'default_sortkey': 'changed', + 'default_sortdir': 'desc', + 'main_actions': [ + self.make_action('view', icon='eye', url='#', + click_handler='viewRevision(props.row)'), + self.make_action('view_separate', url=row_url, target='_blank', + icon='external-link-alt', ), + ], + } + + if empty_data: + + # TODO: surely there is a better way to have empty initial + # data..? but so much logic depends on a query, can't + # just pass empty list here + txn_class = continuum.transaction_class(self.get_model_class()) + meta_class = continuum.versioning_manager.transaction_meta_cls + kwargs['data'] = self.Session.query(txn_class)\ + .outerjoin(meta_class, + meta_class.transaction_id == txn_class.id)\ + .filter(txn_class.id == -1) + + else: + kwargs['data'] = self.get_version_data(obj, order_by=False) + + grid = self.make_version_grid(**kwargs) + + grid.set_joiner('user', lambda q: q.outerjoin(self.model.User)) + grid.set_sorter('user', self.model.User.username) + + grid.set_link('remote_addr') + + grid.append('id') + grid.set_label('id', "TXN ID") + grid.set_link('id') + + return grid + + def revisions_data(self): + """ + AJAX view to fetch revision data for current instance. + """ + txnid = self.request.GET.get('txnid') + if txnid: + # return single txn data + + app = self.get_rattail_app() + obj = self.get_instance() + cls = self.get_model_class() + txn_cls = continuum.transaction_class(cls) + route_prefix = self.get_route_prefix() + + transactions = model_transaction_query( + self.Session(), obj, cls, + child_classes=self.normalize_version_child_classes()) + + txn = transactions.filter(txn_cls.id == txnid).first() + if not txn: + return self.notfound() + + older = transactions.filter(txn_cls.issued_at <= txn.issued_at)\ + .filter(txn_cls.id != txnid)\ + .order_by(txn_cls.issued_at.desc())\ + .first() + newer = transactions.filter(txn_cls.issued_at >= txn.issued_at)\ + .filter(txn_cls.id != txnid)\ + .order_by(txn_cls.issued_at)\ + .first() + + version_diffs = [] + for version in self.get_relevant_versions(txn, obj): + diff = self.make_version_diff(version) + version_diffs.append(diff.as_struct()) + + changed_raw = app.render_datetime(app.localtime(txn.issued_at, from_utc=True)) + changed_ago = app.render_time_ago(app.make_utc() - txn.issued_at) + + changed_by = str(txn.user) + if self.request.has_perm('users.view'): + changed_by = tags.link_to(changed_by, self.request.route_url('users.view', uuid=txn.user.uuid)) + + return { + 'txnid': txn.id, + 'changed': f"{changed_raw} ({changed_ago})", + 'changed_by': changed_by, + 'remote_addr': txn.remote_addr, + 'comment': txn.meta.get('comment'), + 'versions': version_diffs, + 'url': self.request.route_url(f'{route_prefix}.version', uuid=obj.uuid, txnid=txnid), + 'prev_txnid': older.id if older else None, + 'next_txnid': newer.id if newer else None, + } + + else: # no txnid, return grid data + obj = self.get_instance() + grid = self.make_revisions_grid(obj) + return grid.get_buefy_data() + def view_version(self): """ View showing diff details of a particular object version. @@ -4829,10 +4945,10 @@ class MasterView(View): def make_diff(self, old_data, new_data, **kwargs): return diffs.Diff(old_data, new_data, **kwargs) - def make_version_diff(self, version, old_data, new_data, **kwargs): + def make_version_diff(self, version, *args, **kwargs): if 'title' not in kwargs: kwargs['title'] = self.title_for_version(version) - return diffs.VersionDiff(version, old_data, new_data, **kwargs) + return diffs.VersionDiff(version, *args, **kwargs) ############################## # Configuration Views @@ -5576,6 +5692,16 @@ class MasterView(View): route_name='{}.version'.format(route_prefix), permission='{}.versions'.format(permission_prefix)) + # revisions data (AJAX) + config.add_route(f'{route_prefix}.revisions_data', + f'{instance_url_prefix}/revisions-data', + request_method='GET') + config.add_view(cls, attr='revisions_data', + route_name=f'{route_prefix}.revisions_data', + permission=f'{permission_prefix}.versions', + renderer='json') + + @classmethod def _defaults_edit_help(cls, config, **kwargs): route_prefix = cls.get_route_prefix()