From 01afc47826af8975502cf8e284c1e10ab06b1bbc Mon Sep 17 00:00:00 2001 From: Andy Beverley Date: Sun, 4 Jan 2026 22:39:16 +0000 Subject: [PATCH 1/6] Performance - link latest record version (D1098) This commit improves performance by linking the current version of a record to its main stub, thereby making retrieval faster for records with lots of versions. The version retrieval functionality is retained for the purpose of historical view, which operates as it previously did. This commit also includes the addition of curval subfields to the y-axis of graphs (back-end only) which was needed to test some of the changes. It also includes sorting for grouping, which was needed for keeping the grouping tests working as before. --- lib/GADS/Role/Presentation/Chronology.pm | 14 ++++++++++ .../Role/Presentation/Chronology/Curval.pm | 27 +++++++++++++++++++ lib/GADS/Role/Presentation/Chronology/Date.pm | 13 +++++++++ .../Role/Presentation/Chronology/Person.pm | 17 ++++++++++++ .../Role/Presentation/Chronology/String.pm | 13 +++++++++ lib/GADS/Role/Presentation/Chronology/Tree.pm | 15 +++++++++++ 6 files changed, 99 insertions(+) create mode 100644 lib/GADS/Role/Presentation/Chronology.pm create mode 100644 lib/GADS/Role/Presentation/Chronology/Curval.pm create mode 100644 lib/GADS/Role/Presentation/Chronology/Date.pm create mode 100644 lib/GADS/Role/Presentation/Chronology/Person.pm create mode 100644 lib/GADS/Role/Presentation/Chronology/String.pm create mode 100644 lib/GADS/Role/Presentation/Chronology/Tree.pm diff --git a/lib/GADS/Role/Presentation/Chronology.pm b/lib/GADS/Role/Presentation/Chronology.pm new file mode 100644 index 000000000..4bebdcc7d --- /dev/null +++ b/lib/GADS/Role/Presentation/Chronology.pm @@ -0,0 +1,14 @@ +package GADS::Role::Presentation::Chronology; + +use Moo::Role; + +sub chronology { + my ($self) = @_; + + return $self->presentation if $self->can('presentation'); + + # placeholder for now - this will be replaced with a more sensible implementation + return { id => $self->id, }; +} + +1; diff --git a/lib/GADS/Role/Presentation/Chronology/Curval.pm b/lib/GADS/Role/Presentation/Chronology/Curval.pm new file mode 100644 index 000000000..a064b721c --- /dev/null +++ b/lib/GADS/Role/Presentation/Chronology/Curval.pm @@ -0,0 +1,27 @@ +package GADS::Role::Presentation::Chronology::Curval; + +use strict; +use warnings; + +use Moo::Role; + +use Data::Dump qw(pp); + +around chronology => sub { + my ( $orig, $self ) = @_; + + my $chronology = $self->$orig; + + for my $f (@{ $chronology->{html_form}}) { + delete $f->{topics} if exists $f->{topics}; + } + + my $result = {$chronology->{column_name} => []}; + for my $l (@{ $chronology->{links}}) { + $result->{$chronology->{column_name}} = [ map {+{$_->{data}->{column_name}=>$_->{data}->{value}}} @{$l->{presentation}->{columns}}]; + } + + $result; +}; + +1; diff --git a/lib/GADS/Role/Presentation/Chronology/Date.pm b/lib/GADS/Role/Presentation/Chronology/Date.pm new file mode 100644 index 000000000..a1be1cf66 --- /dev/null +++ b/lib/GADS/Role/Presentation/Chronology/Date.pm @@ -0,0 +1,13 @@ +package GADS::Role::Presentation::Chronology::Date; + +use Moo::Role; + +around chronology => sub { + my ( $orig, $self ) = @_; + + my $chronology = $self->$orig; + + return { $chronology->{column_name} => $chronology->{value} }; +}; + +1; diff --git a/lib/GADS/Role/Presentation/Chronology/Person.pm b/lib/GADS/Role/Presentation/Chronology/Person.pm new file mode 100644 index 000000000..4ef2473fe --- /dev/null +++ b/lib/GADS/Role/Presentation/Chronology/Person.pm @@ -0,0 +1,17 @@ +package GADS::Role::Presentation::Chronology::Person; + +use Moo::Role; + +around chronology => sub { + my ($orig, $self) = @_; + + my $chronology = $self->$orig; + + return { + name => $chronology->{column_name}, + text => $chronology->{text}, + details => $chronology->{details}, + }; +}; + +1; \ No newline at end of file diff --git a/lib/GADS/Role/Presentation/Chronology/String.pm b/lib/GADS/Role/Presentation/Chronology/String.pm new file mode 100644 index 000000000..1af618605 --- /dev/null +++ b/lib/GADS/Role/Presentation/Chronology/String.pm @@ -0,0 +1,13 @@ +package GADS::Role::Presentation::Chronology::String; + +use Moo::Role; + +around chronology => sub { + my ( $orig, $self ) = @_; + + my $chronology = $self->$orig; + + return { $chronology->{column_name} => $chronology->{value} }; +}; + +1; diff --git a/lib/GADS/Role/Presentation/Chronology/Tree.pm b/lib/GADS/Role/Presentation/Chronology/Tree.pm new file mode 100644 index 000000000..0758a144a --- /dev/null +++ b/lib/GADS/Role/Presentation/Chronology/Tree.pm @@ -0,0 +1,15 @@ +package GADS::Role::Presentation::Chronology::Tree; + +use Moo::Role; + +around chronology => sub { + my ($orig, $self) = @_; + + my $chronology = $self->$orig; + + return { + $chronology->{column_name} => $chronology->{value}, + }; +}; + +1; From 481e5993a8405d326873ab753d5072d46ca87dd3 Mon Sep 17 00:00:00 2001 From: Andy Beverley Date: Fri, 6 Feb 2026 17:27:42 +0000 Subject: [PATCH 2/6] Updated to default to "all" view on view all button click (D795) --- src/frontend/css/stylesheets/base/_global.scss | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/frontend/css/stylesheets/base/_global.scss b/src/frontend/css/stylesheets/base/_global.scss index df36878f3..9ae417eb4 100644 --- a/src/frontend/css/stylesheets/base/_global.scss +++ b/src/frontend/css/stylesheets/base/_global.scss @@ -110,6 +110,9 @@ table.table-bordered { @extend .pb-2; @extend .border-bottom } + +.text-primary { + color: $brand-secundary !important; } .text-primary { From 994769911777d429221e56e3c0700160a2527ab0 Mon Sep 17 00:00:00 2001 From: Andy Beverley Date: Fri, 6 Feb 2026 17:37:00 +0000 Subject: [PATCH 3/6] Fix syntax error from manual merge --- src/frontend/css/stylesheets/base/_global.scss | 1 + 1 file changed, 1 insertion(+) diff --git a/src/frontend/css/stylesheets/base/_global.scss b/src/frontend/css/stylesheets/base/_global.scss index 9ae417eb4..274608976 100644 --- a/src/frontend/css/stylesheets/base/_global.scss +++ b/src/frontend/css/stylesheets/base/_global.scss @@ -110,6 +110,7 @@ table.table-bordered { @extend .pb-2; @extend .border-bottom } +} .text-primary { color: $brand-secundary !important; From 87785d750dce5cfc30a4770af1a7a0d4a336f3e1 Mon Sep 17 00:00:00 2001 From: Dave Roberts Date: Fri, 6 Feb 2026 11:27:15 +0000 Subject: [PATCH 4/6] Initial iteration of chronology --- lib/GADS/API.pm | 30 ++- lib/GADS/Chronology.pm | 24 +++ lib/GADS/Datum/Rag.pm | 1 + lib/GADS/Record.pm | 183 +++++++++++++++--- lib/GADS/Role/Presentation/Chronology.pm | 14 -- .../Role/Presentation/Chronology/Curval.pm | 27 --- lib/GADS/Role/Presentation/Chronology/Date.pm | 13 -- .../Role/Presentation/Chronology/Person.pm | 17 -- .../Role/Presentation/Chronology/String.pm | 13 -- lib/GADS/Role/Presentation/Chronology/Tree.pm | 15 -- lib/GADS/Role/Presentation/Datum/Person.pm | 24 +-- src/frontend/components/button/_button.scss | 7 +- .../components/button/lib/component.ts | 6 + .../lib/load-more-chronology-button.test.ts | 25 +++ .../button/lib/load-more-chronology-button.ts | 75 +++++++ .../components/button/lib/rename-button.ts | 2 +- .../form-group/autosave/lib/component.js | 2 +- .../components/select-all/lib/component.ts | 1 - src/frontend/js/lib/util/common.ts | 6 + .../lib/util/errorHandler/lib/errorHandler.ts | 1 - .../js/lib/util/filedrag/lib/filedrag.ts | 2 +- t/003_chronology.t | 20 +- views/chronology.tt | 39 +--- views/chronology/chronology.tt | 41 ++++ 24 files changed, 401 insertions(+), 187 deletions(-) create mode 100644 lib/GADS/Chronology.pm delete mode 100644 lib/GADS/Role/Presentation/Chronology.pm delete mode 100644 lib/GADS/Role/Presentation/Chronology/Curval.pm delete mode 100644 lib/GADS/Role/Presentation/Chronology/Date.pm delete mode 100644 lib/GADS/Role/Presentation/Chronology/Person.pm delete mode 100644 lib/GADS/Role/Presentation/Chronology/String.pm delete mode 100644 lib/GADS/Role/Presentation/Chronology/Tree.pm create mode 100644 src/frontend/components/button/lib/load-more-chronology-button.test.ts create mode 100644 src/frontend/components/button/lib/load-more-chronology-button.ts create mode 100644 views/chronology/chronology.tt diff --git a/lib/GADS/API.pm b/lib/GADS/API.pm index 43a484215..a6e65f81e 100644 --- a/lib/GADS/API.pm +++ b/lib/GADS/API.pm @@ -570,7 +570,7 @@ post '/api/settings/logo' => require_login sub { my $filecheck = GADS::Filecheck->instance; error __x"Files of mimetype {mimetype} are not allowed", mimetype => $filecheck->get_filetype($file) unless $filecheck->is_image($file); - + $site->update({ site_logo => $file->content }); content_type 'application/json'; @@ -584,6 +584,32 @@ post '/api/settings/logo' => require_login sub { ); }; +get '/api/chronology/:id' => require_login sub { + my $user = logged_in_user; + my $current_id = route_parameters->get('id'); + my $page = query_parameters->get('page') // 1; + + my $record = GADS::Record->new( + user => $user, + schema => schema, + ); + + $record->find_chronology_id($current_id, page => $page); + + my $layout = $record->layout; + my $base_url = request->base; + my $last_page = $record->last_chronology_page; + + return template 'chronology/chronology.tt' => { + record => $record, + layout_obj => $layout, + page => $page, + last_page => $last_page == $page ? 1 : 0, + }, { + layout => undef, # Do not render page header, footer etc + }; +}; + sub _post_dashboard_widget { my $layout = shift; my $user = logged_in_user; @@ -666,7 +692,7 @@ sub _put_dashboard_widget_edit { my $widget = _get_widget_write(route_parameters->get('id'), route_parameters->get('dashboard_id'), $layout, $user); my $body = from_json(request->body); - + $widget->title($body->{'title'}); $widget->static(query_parameters->get('static') ? 1 : 0) if $widget->dashboard->is_shared; diff --git a/lib/GADS/Chronology.pm b/lib/GADS/Chronology.pm new file mode 100644 index 000000000..ab90984bc --- /dev/null +++ b/lib/GADS/Chronology.pm @@ -0,0 +1,24 @@ +=comment +I _really_ don't like this - I have to work out some way to share the last chronology record between requests, +and this is the only way I can think of to do it without some sort of odd persistence somewhere. +It's a singleton object that just holds the last chronology record, and the API can set it when it finds a chronology, +and the Record class can get it when it needs to find the next page of a chronology. +=cut +package GADS::Chronology; + +use strict; +use warnings; + +use Moo; + +with 'MooX::Singleton'; + +has last_record => ( + is => 'rw', +); + +sub clear { + shift->last_record(undef); +} + +1; diff --git a/lib/GADS/Datum/Rag.pm b/lib/GADS/Datum/Rag.pm index 2e708ad45..78b8b0dfc 100644 --- a/lib/GADS/Datum/Rag.pm +++ b/lib/GADS/Datum/Rag.pm @@ -122,6 +122,7 @@ sub grade { shift->as_grade } sub as_grade { my $self = shift; + return "invalid" unless defined $self->_value_single; return $mapping{ $self->_value_single }; } diff --git a/lib/GADS/Record.pm b/lib/GADS/Record.pm index d6fb7f5f2..b9e896e1e 100644 --- a/lib/GADS/Record.pm +++ b/lib/GADS/Record.pm @@ -24,6 +24,7 @@ use DateTime::Format::Strptime qw( ); use DBIx::Class::ResultClass::HashRefInflator; use GADS::AlertSend; use GADS::Config; +use GADS::Chronology; use GADS::Datum::Autocur; use GADS::Datum::Calc; use GADS::Datum::Count; @@ -55,6 +56,8 @@ use MooX::Types::MooseLike::Base qw(Maybe Bool Int ArrayRef HashRef); use MooX::Types::MooseLike::DateTime qw/DateAndTime/; use namespace::clean; +use JSON qw/to_json/; + with 'GADS::DateTime'; with 'GADS::Role::Presentation::Record'; @@ -724,7 +727,7 @@ sub find_chronology_id or error __x"Record ID {id} not found", id => $current_id; my $instance_id = $current->instance_id; $self->_set_instance_id($current->instance_id); - $self->_find(current_id => $current_id, chronology => 1); + $self->_find(current_id => $current_id, chronology => 1, chronology_page => $options{page} // 1); } sub find_draftuser_id @@ -820,6 +823,24 @@ sub clear $self->clear_id_count; } +sub _get_record_for_chronology { + my ( $self, $record, $records, %find ) = @_; + return unless $find{chronology}; + + my @changed; + + $records->rewind($record->edited_time->values->[0], %find); + + $records->fetch_multivalues( + record_ids => [$record->record_id], + retrieved => [$record->{$record->record_id}], + records => [$record], + is_draft => $find{draftuser_id}, + already_seen => $records->already_seen, + chronology => 1, + ); +} + # XXX This whole section is getting messy and duplicating a lot of code from # GADS::Records. Ideally this needs to share the same code. sub _find @@ -832,6 +853,8 @@ sub _find error __"You do not have access to this deleted record" if $find{deleted} && !$self->layout->user_can("purge") && !$GADS::Schema::IGNORE_PERMISSIONS; + my $last_page; + my %params = ( user => $self->user, layout => $self->layout, @@ -870,10 +893,27 @@ sub _find ); # No linked here so that we get the ones needed in accordance with this loop (could be either) my @prefetches = $records->jpfetch(search => 1, %common); # Still need search in case of view limit - last if !@prefetches && !$first_run; + last unless @prefetches || $first_run; # Inverted - didn't like how the condition was (slightly) confusing. my %options = $find{current_id} || $find{draftuser_id} ? () : (root_table => 'record', no_current => 1); my $search = $records->search_query(linked => 1, chronology => $find{chronology}, rewind => $records->rewind_values, %common, %options); - # Still need search in case of view limit + if ($find{chronology}) { + my $record_search = $self->schema->resultset('Current')->find( + $find{current_id}, + )->records->search( + {}, + { + select => [ 'id' ], + rows => 10, + page => $find{chronology_page} // 1, + result_class => 'DBIx::Class::ResultClass::HashRefInflator', + order_by => { '-desc' => 'created' }, + } + ); + my @ids = map { $_->{id} } $record_search->all; + $last_page = $record_search->pager->last_page == $find{chronology_page} ? 1: 0; + push @$search, { 'record_single.id' => { -in => \@ids } }; + } + # Still need search in case of view limit @prefetches = $records->jpfetch(search => 1, linked => 0, %common, %options); my $root_table; @@ -953,7 +993,7 @@ sub _find join => [@prefetches], columns => \@columns_fetch, }; - $params->{order_by} = 'record_single.created' + $params->{order_by} = { -desc => 'record_single.created'} if $find{chronology}; my $result = $self->schema->resultset($root_table)->search( [ @@ -1031,7 +1071,7 @@ sub _find set_record_created_user => $self->set_record_created_user, ); # Set data for this original record to the current version (latest) - $self->record($record->{$record_ids[-1]}); + $self->record($record->{$record_ids[0]}); } } else { @@ -1060,23 +1100,58 @@ sub _find if ($find{chronology}) { my @chronology; my $last_record; - foreach my $record (@record_objects) + if ($find{chronology_page} == 1) { + $self->clear_get_last_chronology_record; + my $record = $record_objects[0]; + my @changed; + + $self->_get_record_for_chronology($record, $records, %find); + foreach my $column ( @{ $record->columns_render } ) { + next if $column->internal; + my $datum = $record->fields->{ $column->id }; + if($column->type eq 'curval') { + my %new_ids = map { $_->{id} => $_ } @{$datum->values}; + my @values; + error "Values not empty" if @values; + foreach my $id (keys %new_ids) + { + my $new_value = $new_ids{$id}; + $new_value->{status} = "current"; + $new_value->{version_id} = $new_value->{record}->record_id; + push @values, $new_value; + } + push @changed, $column->presentation(datum_presentation => $datum->presentation(values => \@values)); + } + else { + push @changed, $column->presentation(datum_presentation => $datum->presentation); + } + } + + push @chronology, + { + editor => $record->edited_user, + datetime => $record->edited_time, + changed => \@changed, + }; + } else { + unshift @record_objects, $self->get_last_chronology_record if $self->get_last_chronology_record; + } + foreach my $i (0 .. $#record_objects-1) { - $records->rewind($record->edited_time->values->[0]); - $records->fetch_multivalues( - record_ids => [$record->record_id], - retrieved => [$record->{$record->record_id}], - records => [$record], - is_draft => $find{draftuser_id}, - already_seen => $records->already_seen, - chronology => 1, - ); + $record = $record_objects[$i+1]; + + $self->_get_record_for_chronology($record, $records, %find); + $last_record = $record; + + $record = $record_objects[$i]; + $self->_get_record_for_chronology($record, $records, %find); + my @changed; foreach my $column (@{$record->columns_render}) { next if $column->internal; my $datum = $record->fields->{$column->id}; - my $last_datum = $last_record && $last_record->fields->{$column->id}; + my $last_datum = $last_record->fields->{$column->id}; if ( (!$last_record && !$datum->blank) @@ -1097,7 +1172,7 @@ sub _find # Removed? if (!$new_ids{$id}) { - $old_value->{status} = "Removed"; + $old_value->{status} = "removed"; $old_value->{version_id} = $old_value->{record}->record_id; push @values, $old_value; } @@ -1105,7 +1180,7 @@ sub _find # Changed? my $new_value = delete $new_ids{$id}; next if $old_value->{value} eq $new_value->{value}; - $new_value->{status} = "Changed"; + $new_value->{status} = "changed"; $new_value->{version_id} = $new_value->{record}->record_id; push @values, $new_value; } @@ -1114,20 +1189,16 @@ sub _find foreach my $id (keys %new_ids) { my $new_value = $new_ids{$id}; - $new_value->{status} = "Added"; + $new_value->{status} = "added"; $new_value->{version_id} = $new_value->{record}->record_id; push @values, $new_value; } - foreach my $v (@values) - { - } - push @changed, $column->presentation(datum_presentation => $datum->presentation(values => \@values)) + push @changed, $column->presentation(datum_presentation => $datum->presentation(values => \@values)); } else { - push @changed, $column->presentation(datum_presentation => $datum->presentation) + push @changed, $column->presentation(datum_presentation => $datum->presentation); } } - } push @chronology, { editor => $record->edited_user, @@ -1136,12 +1207,72 @@ sub _find } if @changed; # There may have been changes, but not ones the user has access to $last_record = $record; } + if($last_page) { + my $record = $record_objects[-1]; + $self->_get_record_for_chronology($record, $records, %find); + + my @changed; + foreach my $column ( @{ $record->columns_render } ) { + next if $column->internal; + my $datum = $record->fields->{ $column->id }; + if($column->type eq 'curval') { + my %new_ids = map { $_->{id} => $_ } @{$datum->values}; + my @values; + foreach my $id (keys %new_ids) + { + my $new_value = $new_ids{$id}; + $new_value->{status} = "created"; + $new_value->{version_id} = $new_value->{record}->record_id; + push @values, $new_value; + } + push @changed, $column->presentation(datum_presentation => $datum->presentation(values => \@values)); + } + else { + push @changed, $column->presentation(datum_presentation => $datum->presentation); + } + } + push @chronology, + { + editor => $record->edited_user, + datetime => $record->edited_time, + changed => \@changed, + }; + } + $self->_set_last_chronology_page($last_page // 0); + $self->_set_last_chronology_record($last_record) if $last_record; $self->_set_chronology(\@chronology); } $self; # Allow chaining } +has get_last_chronology_record => ( + is => 'lazy', + clearer => 1, + builder => sub { + my $self = shift; + my $chronology = GADS::Chronology->instance; + return $chronology->last_record; + } +); + +sub _clear_last_chronology_record { + my $self = shift; + my $chronology = GADS::Chronology->instance; + $chronology->clear; + $self->_clear_get_last_chronology_record; +} + +sub _set_last_chronology_record { + my ($self, $record) = @_; + my $chronology = GADS::Chronology->instance; + $chronology->last_record($record); +} + +has last_chronology_page => ( + is => 'rwp', +); + has chronology => ( is => 'rwp', ); @@ -2411,7 +2542,7 @@ sub _field_write # child records. This is used in chronology view to # ensure the correct versions at the time of record # edit are used - version_datetime => $created, + version_datetime => $created, ); push @{$self->_records_to_write_after}, $record if $record->is_edited; diff --git a/lib/GADS/Role/Presentation/Chronology.pm b/lib/GADS/Role/Presentation/Chronology.pm deleted file mode 100644 index 4bebdcc7d..000000000 --- a/lib/GADS/Role/Presentation/Chronology.pm +++ /dev/null @@ -1,14 +0,0 @@ -package GADS::Role::Presentation::Chronology; - -use Moo::Role; - -sub chronology { - my ($self) = @_; - - return $self->presentation if $self->can('presentation'); - - # placeholder for now - this will be replaced with a more sensible implementation - return { id => $self->id, }; -} - -1; diff --git a/lib/GADS/Role/Presentation/Chronology/Curval.pm b/lib/GADS/Role/Presentation/Chronology/Curval.pm deleted file mode 100644 index a064b721c..000000000 --- a/lib/GADS/Role/Presentation/Chronology/Curval.pm +++ /dev/null @@ -1,27 +0,0 @@ -package GADS::Role::Presentation::Chronology::Curval; - -use strict; -use warnings; - -use Moo::Role; - -use Data::Dump qw(pp); - -around chronology => sub { - my ( $orig, $self ) = @_; - - my $chronology = $self->$orig; - - for my $f (@{ $chronology->{html_form}}) { - delete $f->{topics} if exists $f->{topics}; - } - - my $result = {$chronology->{column_name} => []}; - for my $l (@{ $chronology->{links}}) { - $result->{$chronology->{column_name}} = [ map {+{$_->{data}->{column_name}=>$_->{data}->{value}}} @{$l->{presentation}->{columns}}]; - } - - $result; -}; - -1; diff --git a/lib/GADS/Role/Presentation/Chronology/Date.pm b/lib/GADS/Role/Presentation/Chronology/Date.pm deleted file mode 100644 index a1be1cf66..000000000 --- a/lib/GADS/Role/Presentation/Chronology/Date.pm +++ /dev/null @@ -1,13 +0,0 @@ -package GADS::Role::Presentation::Chronology::Date; - -use Moo::Role; - -around chronology => sub { - my ( $orig, $self ) = @_; - - my $chronology = $self->$orig; - - return { $chronology->{column_name} => $chronology->{value} }; -}; - -1; diff --git a/lib/GADS/Role/Presentation/Chronology/Person.pm b/lib/GADS/Role/Presentation/Chronology/Person.pm deleted file mode 100644 index 4ef2473fe..000000000 --- a/lib/GADS/Role/Presentation/Chronology/Person.pm +++ /dev/null @@ -1,17 +0,0 @@ -package GADS::Role::Presentation::Chronology::Person; - -use Moo::Role; - -around chronology => sub { - my ($orig, $self) = @_; - - my $chronology = $self->$orig; - - return { - name => $chronology->{column_name}, - text => $chronology->{text}, - details => $chronology->{details}, - }; -}; - -1; \ No newline at end of file diff --git a/lib/GADS/Role/Presentation/Chronology/String.pm b/lib/GADS/Role/Presentation/Chronology/String.pm deleted file mode 100644 index 1af618605..000000000 --- a/lib/GADS/Role/Presentation/Chronology/String.pm +++ /dev/null @@ -1,13 +0,0 @@ -package GADS::Role::Presentation::Chronology::String; - -use Moo::Role; - -around chronology => sub { - my ( $orig, $self ) = @_; - - my $chronology = $self->$orig; - - return { $chronology->{column_name} => $chronology->{value} }; -}; - -1; diff --git a/lib/GADS/Role/Presentation/Chronology/Tree.pm b/lib/GADS/Role/Presentation/Chronology/Tree.pm deleted file mode 100644 index 0758a144a..000000000 --- a/lib/GADS/Role/Presentation/Chronology/Tree.pm +++ /dev/null @@ -1,15 +0,0 @@ -package GADS::Role::Presentation::Chronology::Tree; - -use Moo::Role; - -around chronology => sub { - my ($orig, $self) = @_; - - my $chronology = $self->$orig; - - return { - $chronology->{column_name} => $chronology->{value}, - }; -}; - -1; diff --git a/lib/GADS/Role/Presentation/Datum/Person.pm b/lib/GADS/Role/Presentation/Datum/Person.pm index ca3da85e6..501bbfc54 100644 --- a/lib/GADS/Role/Presentation/Datum/Person.pm +++ b/lib/GADS/Role/Presentation/Datum/Person.pm @@ -16,17 +16,19 @@ sub _presentation_details { }; } - for ( - [$person->{freetext1}, $site->register_freetext1_name], - [$person->{freetext2}, $site->register_freetext2_name] - ) { - next unless $_->[0]; - - push @details, { - definition => $_->[1], - value => $_->[0], - type => 'text' - }; + if($site) { + for ( + [$person->{freetext1}, $site->register_freetext1_name], + [$person->{freetext2}, $site->register_freetext2_name] + ) { + next unless $_->[0]; + + push @details, { + definition => $_->[1], + value => $_->[0], + type => 'text' + }; + } } return { diff --git a/src/frontend/components/button/_button.scss b/src/frontend/components/button/_button.scss index 261b9849f..9e74a8cd7 100644 --- a/src/frontend/components/button/_button.scss +++ b/src/frontend/components/button/_button.scss @@ -630,4 +630,9 @@ $btn-border-radius: 23px; .rename::before { @extend %icon-font; content: "\E80b"; -} \ No newline at end of file +} + +// We want a margin between the chronology and the button +.btn-js-load-chronology { + @extend .mt-3; +} diff --git a/src/frontend/components/button/lib/component.ts b/src/frontend/components/button/lib/component.ts index 6ae1ced67..c50da9c09 100644 --- a/src/frontend/components/button/lib/component.ts +++ b/src/frontend/components/button/lib/component.ts @@ -111,6 +111,12 @@ class ButtonComponent extends Component { createCancelButton(el); }); }); + map.set('btn-js-load-chronology', (el) => { + import(/* webpackChunkName: "load-more-chronology-button" */ './load-more-chronology-button') + .then(({default: createLoadMoreChronologyButton}) => { + createLoadMoreChronologyButton(el); + }); + }); ButtonComponent.staticButtonsMap = map; } diff --git a/src/frontend/components/button/lib/load-more-chronology-button.test.ts b/src/frontend/components/button/lib/load-more-chronology-button.test.ts new file mode 100644 index 000000000..d65c5e726 --- /dev/null +++ b/src/frontend/components/button/lib/load-more-chronology-button.test.ts @@ -0,0 +1,25 @@ +/* eslint-disable */ +import "../../../testing/globals.definitions"; +import { describe, it, expect } from '@jest/globals'; +import {LoadMoreChronologyButton, default as createLoadMoreChronologyButton} from './load-more-chronology-button'; +import { upload } from "../../../js/lib/util/upload/UploadControl"; + +declare global { + interface JQuery { + on(event: "chronology:append", handler: (event: JQuery.Event, content: any[]) => void): this; + } +} + +describe('LoadMoreChronologyButton', () => { + it('should create a button', () => { + // Mock jQuery elements + const $button = $(''); + const $target = $('
'); + const $spinner = $(''); + $('body').append($button, $target, $spinner); + + const btn = createLoadMoreChronologyButton($button); + + expect(btn).toBeInstanceOf(LoadMoreChronologyButton); + }); +}); diff --git a/src/frontend/components/button/lib/load-more-chronology-button.ts b/src/frontend/components/button/lib/load-more-chronology-button.ts new file mode 100644 index 000000000..b8e142b49 --- /dev/null +++ b/src/frontend/components/button/lib/load-more-chronology-button.ts @@ -0,0 +1,75 @@ +import { initializeRegisteredComponents } from "component"; +import { logging } from "logging"; +import ErrorHandler from "util/errorHandler"; + +export class LoadMoreChronologyButton { + private page: number = 1; + private $target: JQuery; + private $spinner: JQuery; + private errorHandler: ErrorHandler; + + constructor(private $el: JQuery) { + logging.log("Initializing LoadMoreChronologyButton for element:", $el); + this.$target = $('.chronology'); + this.$spinner = $(".chronology_spinner"); + this.errorHandler = new ErrorHandler(this.$target[0]); + this.init(); + } + + init() { + // Set up the click event handler for the button + this.$el.on("click", async () => { + // When the button is clicked, fetch the next page of chronology data + await this.fetchChronology(); + }); + // Fetch the first page of chronology data on initialization + this.fetchChronology(); + } + + async fetchChronology() { + // Disable the button + this.$el.prop("disabled", true); + // Get the record ID from the button's data attribute + const recordId = this.$el.data("record-id"); + // Get the current page number + const page = this.page; + // Download the chronology data for the next page + const url = `/api/chronology/${recordId}?page=${page}`; + try { + // Show the spinner while loading + this.$spinner.show(); + // Fetch the data + const response = await fetch(url); + // Check if the response is successful + if (!response.ok) { + // display an error message using the error handler + if(response.status !== 400) { + this.errorHandler.addError(`Failed to load chronology data. Server responded with status: ${response.status}`); + } + return; + } + // If successful, get the response data as text + const html = await response.text(); + // Append the new data to the chronology list + this.$target.append(html); + // Initialize any new components in the newly loaded data (e.g., tooltips, popovers) + initializeRegisteredComponents(this.$target[0]); + // Increment the page number for the next fetch + this.page += 1; + } catch (error) { + // If there is an error, use the error handler to display an error message + this.errorHandler.addError(`An error occurred while loading chronology data. Please try again. Error: ${error}`); + } finally { + // Hide the spinner + this.$spinner.hide(); + // Re-enable the button + this.$el.prop("disabled", false); + } + } +} + +const createLoadMoreChronologyButton = (el: JQuery) => { + return new LoadMoreChronologyButton(el); +}; + +export default createLoadMoreChronologyButton; diff --git a/src/frontend/components/button/lib/rename-button.ts b/src/frontend/components/button/lib/rename-button.ts index 4e66e6f31..6834f0446 100644 --- a/src/frontend/components/button/lib/rename-button.ts +++ b/src/frontend/components/button/lib/rename-button.ts @@ -158,7 +158,7 @@ class RenameButton { * @param {JQuery} button The button that was clicked * @param {JQuery.BlurEvent} e The blur event */ - private triggerRename(id: number, button: JQuery, e: JQuery.Event) { + private triggerRename(id: number, button: JQuery, e: JQuery.Event) { // eslint-disable-line @typescript-eslint/no-unused-vars const previousValue = $(`#current-${id}`).text(); const extension = '.' + previousValue.split('.').pop(); const newName = this.value.endsWith(extension) ? this.value : this.value + extension; diff --git a/src/frontend/components/form-group/autosave/lib/component.js b/src/frontend/components/form-group/autosave/lib/component.js index ae8a114a0..e34812730 100644 --- a/src/frontend/components/form-group/autosave/lib/component.js +++ b/src/frontend/components/form-group/autosave/lib/component.js @@ -11,7 +11,7 @@ class AutosaveComponent extends AutosaveBase { */ async initAutosave() { const $field = $(this.element); - const self = this; + const self = this; // eslint-disable-line @typescript-eslint/no-this-alias if ($field.data('is-readonly')) return; // For each field, when it changes save the value to the local storage diff --git a/src/frontend/components/select-all/lib/component.ts b/src/frontend/components/select-all/lib/component.ts index 7d4b782f8..4e6c00229 100644 --- a/src/frontend/components/select-all/lib/component.ts +++ b/src/frontend/components/select-all/lib/component.ts @@ -14,7 +14,6 @@ export default class SelectAllComponent extends Component { const boxes = parent.find("input[type=checkbox]"); boxes.toArray().forEach(item => { if (item === this.el[0]) return; - console.log("item", item); const i = item; i.checked = (this.el[0]).checked; }); diff --git a/src/frontend/js/lib/util/common.ts b/src/frontend/js/lib/util/common.ts index 0e9af2a22..a04a67d25 100644 --- a/src/frontend/js/lib/util/common.ts +++ b/src/frontend/js/lib/util/common.ts @@ -27,3 +27,9 @@ export const fromJson = (json: String | object) => { return {}; } } + +export const fixOverflow = (element: T) => { + if(element.scrollWidth > element.clientWidth && element.textContent) { + element.title = element.textContent; + } +} diff --git a/src/frontend/js/lib/util/errorHandler/lib/errorHandler.ts b/src/frontend/js/lib/util/errorHandler/lib/errorHandler.ts index 580ad2838..199eaef9d 100644 --- a/src/frontend/js/lib/util/errorHandler/lib/errorHandler.ts +++ b/src/frontend/js/lib/util/errorHandler/lib/errorHandler.ts @@ -43,7 +43,6 @@ export class ErrorHandler { this.errorContainer.append(errorElement); }); } else { - console.log('No errors to display'); this.errorContainer.hide(); } } diff --git a/src/frontend/js/lib/util/filedrag/lib/filedrag.ts b/src/frontend/js/lib/util/filedrag/lib/filedrag.ts index fd169da98..f3dc0d04a 100644 --- a/src/frontend/js/lib/util/filedrag/lib/filedrag.ts +++ b/src/frontend/js/lib/util/filedrag/lib/filedrag.ts @@ -40,7 +40,7 @@ class FileDrag { showElement($('[data-draggable="true"]')); if (this.options.debug) console.log(e.originalEvent.dataTransfer.files); showElement(this.el); - console.log(e.originalEvent.dataTransfer.files); + if(this.options.debug) console.log(e.originalEvent.dataTransfer.files); if (this.options.allowMultiple) { // For some reason the function will not accept a FileList, so we convert it to an array const files = Array.from(e.originalEvent.dataTransfer.files); diff --git a/t/003_chronology.t b/t/003_chronology.t index 63ed654b0..dab008061 100644 --- a/t/003_chronology.t +++ b/t/003_chronology.t @@ -57,29 +57,30 @@ $record->clear; $record->find_chronology_id(1); my @changed = @{$record->chronology}; - is(@changed, 3, "Correct number of total versions"); + # This includes the initial write, which is shown as a change of all fields, and the two subsequent edits, and finally the current values + is(@changed, 4, "Correct number of total versions"); # Initial write my @changes = @{(shift @changed)->{changed}}; is(@changes, 2, "Correct number of changes"); my $changed_string = shift @changes; - is($changed_string->{name_short}, 'L1string1', "Showing initial string as change"); + ok(defined $changed_string->{string1}, "Showing initial string as change"); my $changed_integer = shift @changes; - is($changed_integer->{name_short}, 'L1integer1', "Showing initial integer as change"); + ok(defined $changed_integer->{integer1}, "Showing initial integer as change"); # First change @changes = @{(shift @changed)->{changed}}; is(@changes, 2, "Correct number of changes"); $changed_string = shift @changes; - is($changed_string->{name_short}, 'L1string1', "Showing change of string"); + ok(defined $changed_string->{string1}, "Showing change of string"); $changed_integer = shift @changes; - is($changed_integer->{name_short}, 'L1integer1', "Showing change of integer"); + ok(defined $changed_integer->{integer1}, "Showing change of integer"); # Second change @changes = @{(shift @changed)->{changed}}; is(@changes, 1, "Correct number of changes"); $changed_integer = shift @changes; - is($changed_integer->{name_short}, 'L1integer1', "Showing change of integer in second edit"); + ok(defined $changed_integer->{integer1}, "Showing change of integer in second edit"); } # Check changes as user without permission on integer field @@ -90,19 +91,20 @@ $record->clear; $record->find_chronology_id(1); my @changed = @{$record->chronology}; - is(@changed, 2, "Correct number of total versions"); + # This includes the initial write, which is shown as a change of all fields, and the two subsequent edits, and finally the current values + is(@changed, 3, "Correct number of total versions"); # Initial write my @changes = @{(shift @changed)->{changed}}; is(@changes, 1, "Correct number of changes"); my $changed_string = shift @changes; - is($changed_string->{name_short}, 'L1string1', "Showing initial string as change"); + ok(defined $changed_string->{string1}, "Showing initial string as change"); # First change @changes = @{(shift @changed)->{changed}}; is(@changes, 1, "Correct number of changes"); $changed_string = shift @changes; - is($changed_string->{name_short}, 'L1string1', "Showing change of string"); + ok(defined $changed_string->{string1}, "Showing change of string"); # Second change not shown as integer not visible } diff --git a/views/chronology.tt b/views/chronology.tt index 74d1abac5..365007943 100644 --- a/views/chronology.tt +++ b/views/chronology.tt @@ -1,44 +1,15 @@ [% - PROCESS snippets/datum.tt; - PROCESS snippets/record_readmore.tt; - # add standardized page header INCLUDE layouts/page_header.tt title = "Chronology of edits for record " _ record.current_id; %]
- [% - FOREACH version IN record.chronology; - initial = loop.first ? 1 : 0; - action_datetime = version.datetime.as_string; - action_type = initial ? 'created' : 'updated'; - action_by = version.editor.id ? " by " _ version.editor.as_string : ''; - - %] -
-

- [% action_datetime _ " - " _ " record " _ action_type _ action_by | html %] -

- -
-
-
-
    - [% - FOREACH field IN version.changed; - label_changed_to = field.type == "curval" OR initial ? '' : 'changed to '; - %] -
  • - [% field.name | html %] - [% label_changed_to; render_datum(field); %] -
    • - [% END %] -
-
-
-
+
+ - [% END %] +
diff --git a/views/chronology/chronology.tt b/views/chronology/chronology.tt new file mode 100644 index 000000000..e84dd7244 --- /dev/null +++ b/views/chronology/chronology.tt @@ -0,0 +1,41 @@ +[% + PROCESS snippets/datum.tt; + PROCESS snippets/record_readmore.tt; +%] + +[% + FOREACH version IN record.chronology; + initial = loop.first ? page == 1 : 0; + action_datetime = version.datetime.as_string; + final_page = loop.last ? last_page : 0; + action_type = initial ? 'Current Version' : final_page ? 'created' : 'updated'; + action_by = version.editor.id ? " by " _ version.editor.as_string : ''; +%] +
+

+ [% IF initial %] + Current Version + [% ELSE %] + [% action_datetime _ " - " _ " record " _ action_type _ action_by | html %] + [% END %] +

+ +
+
+
+
    + [% + FOREACH field IN version.changed; + label_changed_to = field.type == "curval" OR initial ? '' : final_page ? 'Set as ' : 'changed to '; + %] +
  • + [% field.name | html %] + [% label_changed_to; render_datum(field); %] +
    • + [% END %] +
+
+
+
+
+[% END %] From f8fb8056b5cd3701119a1abb37b4058c28a4b6f3 Mon Sep 17 00:00:00 2001 From: Dave Roberts Date: Mon, 18 May 2026 15:21:10 +0100 Subject: [PATCH 5/6] Fix errors in commits and add comments clarifying chronology changes --- lib/GADS/Chronology.pm | 2 -- lib/GADS/Datum/Rag.pm | 1 - lib/GADS/Record.pm | 5 +++-- .../button/lib/load-more-chronology-button.test.ts | 7 ------- .../components/button/lib/load-more-chronology-button.ts | 4 ++++ src/frontend/components/button/lib/rename-button.ts | 4 ++-- .../components/form-group/autosave/lib/component.js | 2 +- src/frontend/js/lib/util/common.ts | 8 +------- 8 files changed, 11 insertions(+), 22 deletions(-) diff --git a/lib/GADS/Chronology.pm b/lib/GADS/Chronology.pm index ab90984bc..fe9fe6755 100644 --- a/lib/GADS/Chronology.pm +++ b/lib/GADS/Chronology.pm @@ -1,6 +1,4 @@ =comment -I _really_ don't like this - I have to work out some way to share the last chronology record between requests, -and this is the only way I can think of to do it without some sort of odd persistence somewhere. It's a singleton object that just holds the last chronology record, and the API can set it when it finds a chronology, and the Record class can get it when it needs to find the next page of a chronology. =cut diff --git a/lib/GADS/Datum/Rag.pm b/lib/GADS/Datum/Rag.pm index 78b8b0dfc..2e708ad45 100644 --- a/lib/GADS/Datum/Rag.pm +++ b/lib/GADS/Datum/Rag.pm @@ -122,7 +122,6 @@ sub grade { shift->as_grade } sub as_grade { my $self = shift; - return "invalid" unless defined $self->_value_single; return $mapping{ $self->_value_single }; } diff --git a/lib/GADS/Record.pm b/lib/GADS/Record.pm index b9e896e1e..7686c1208 100644 --- a/lib/GADS/Record.pm +++ b/lib/GADS/Record.pm @@ -56,8 +56,6 @@ use MooX::Types::MooseLike::Base qw(Maybe Bool Int ArrayRef HashRef); use MooX::Types::MooseLike::DateTime qw/DateAndTime/; use namespace::clean; -use JSON qw/to_json/; - with 'GADS::DateTime'; with 'GADS::Role::Presentation::Record'; @@ -1100,6 +1098,7 @@ sub _find if ($find{chronology}) { my @chronology; my $last_record; + # First entry - this is the record as it currently stands if ($find{chronology_page} == 1) { $self->clear_get_last_chronology_record; my $record = $record_objects[0]; @@ -1136,6 +1135,7 @@ sub _find } else { unshift @record_objects, $self->get_last_chronology_record if $self->get_last_chronology_record; } + # Show all changes from the original - ignore the last one as this will become the $last_record for comparison foreach my $i (0 .. $#record_objects-1) { $record = $record_objects[$i+1]; @@ -1207,6 +1207,7 @@ sub _find } if @changed; # There may have been changes, but not ones the user has access to $last_record = $record; } + # If we're on the last page, return the record state as it was created if($last_page) { my $record = $record_objects[-1]; $self->_get_record_for_chronology($record, $records, %find); diff --git a/src/frontend/components/button/lib/load-more-chronology-button.test.ts b/src/frontend/components/button/lib/load-more-chronology-button.test.ts index d65c5e726..223d8fd6c 100644 --- a/src/frontend/components/button/lib/load-more-chronology-button.test.ts +++ b/src/frontend/components/button/lib/load-more-chronology-button.test.ts @@ -2,13 +2,6 @@ import "../../../testing/globals.definitions"; import { describe, it, expect } from '@jest/globals'; import {LoadMoreChronologyButton, default as createLoadMoreChronologyButton} from './load-more-chronology-button'; -import { upload } from "../../../js/lib/util/upload/UploadControl"; - -declare global { - interface JQuery { - on(event: "chronology:append", handler: (event: JQuery.Event, content: any[]) => void): this; - } -} describe('LoadMoreChronologyButton', () => { it('should create a button', () => { diff --git a/src/frontend/components/button/lib/load-more-chronology-button.ts b/src/frontend/components/button/lib/load-more-chronology-button.ts index b8e142b49..53c0634a6 100644 --- a/src/frontend/components/button/lib/load-more-chronology-button.ts +++ b/src/frontend/components/button/lib/load-more-chronology-button.ts @@ -10,9 +10,13 @@ export class LoadMoreChronologyButton { constructor(private $el: JQuery) { logging.log("Initializing LoadMoreChronologyButton for element:", $el); + // Where to put the chronology entries this.$target = $('.chronology'); + // Loading spinner this.$spinner = $(".chronology_spinner"); + // Error handler - attached to the chronology container this.errorHandler = new ErrorHandler(this.$target[0]); + // Initialize the button this.init(); } diff --git a/src/frontend/components/button/lib/rename-button.ts b/src/frontend/components/button/lib/rename-button.ts index 6834f0446..7496e5bb5 100644 --- a/src/frontend/components/button/lib/rename-button.ts +++ b/src/frontend/components/button/lib/rename-button.ts @@ -102,7 +102,7 @@ class RenameButton { /** * Perform click event * @param {number} id The id of the field - * @param {JQuery.ClickEvent} ev The event object + * @param {JQuery.ClickEvent} ev The event object */ private renameClick(id: number, ev: JQuery.ClickEvent) { ev.preventDefault(); @@ -158,7 +158,7 @@ class RenameButton { * @param {JQuery} button The button that was clicked * @param {JQuery.BlurEvent} e The blur event */ - private triggerRename(id: number, button: JQuery, e: JQuery.Event) { // eslint-disable-line @typescript-eslint/no-unused-vars + private triggerRename(id: number, button: JQuery, e: JQuery.Event) { const previousValue = $(`#current-${id}`).text(); const extension = '.' + previousValue.split('.').pop(); const newName = this.value.endsWith(extension) ? this.value : this.value + extension; diff --git a/src/frontend/components/form-group/autosave/lib/component.js b/src/frontend/components/form-group/autosave/lib/component.js index e34812730..ae8a114a0 100644 --- a/src/frontend/components/form-group/autosave/lib/component.js +++ b/src/frontend/components/form-group/autosave/lib/component.js @@ -11,7 +11,7 @@ class AutosaveComponent extends AutosaveBase { */ async initAutosave() { const $field = $(this.element); - const self = this; // eslint-disable-line @typescript-eslint/no-this-alias + const self = this; if ($field.data('is-readonly')) return; // For each field, when it changes save the value to the local storage diff --git a/src/frontend/js/lib/util/common.ts b/src/frontend/js/lib/util/common.ts index a04a67d25..8d4c4ced7 100644 --- a/src/frontend/js/lib/util/common.ts +++ b/src/frontend/js/lib/util/common.ts @@ -26,10 +26,4 @@ export const fromJson = (json: String | object) => { } catch (e) { return {}; } -} - -export const fixOverflow = (element: T) => { - if(element.scrollWidth > element.clientWidth && element.textContent) { - element.title = element.textContent; - } -} +}; From a491733044d75afd23431f9a9a7944fa1949eba8 Mon Sep 17 00:00:00 2001 From: Dave Roberts Date: Mon, 18 May 2026 16:15:05 +0100 Subject: [PATCH 6/6] Update from code review --- lib/GADS/API.pm | 4 ++-- lib/GADS/Record.pm | 1 - .../components/button/lib/load-more-chronology-button.ts | 2 -- 3 files changed, 2 insertions(+), 5 deletions(-) diff --git a/lib/GADS/API.pm b/lib/GADS/API.pm index a6e65f81e..ab9cc45b1 100644 --- a/lib/GADS/API.pm +++ b/lib/GADS/API.pm @@ -585,9 +585,9 @@ post '/api/settings/logo' => require_login sub { }; get '/api/chronology/:id' => require_login sub { - my $user = logged_in_user; + my $user = logged_in_user; my $current_id = route_parameters->get('id'); - my $page = query_parameters->get('page') // 1; + my $page = query_parameters->get('page') // 1; my $record = GADS::Record->new( user => $user, diff --git a/lib/GADS/Record.pm b/lib/GADS/Record.pm index 7686c1208..5f76febf8 100644 --- a/lib/GADS/Record.pm +++ b/lib/GADS/Record.pm @@ -1111,7 +1111,6 @@ sub _find if($column->type eq 'curval') { my %new_ids = map { $_->{id} => $_ } @{$datum->values}; my @values; - error "Values not empty" if @values; foreach my $id (keys %new_ids) { my $new_value = $new_ids{$id}; diff --git a/src/frontend/components/button/lib/load-more-chronology-button.ts b/src/frontend/components/button/lib/load-more-chronology-button.ts index 53c0634a6..46378fdcf 100644 --- a/src/frontend/components/button/lib/load-more-chronology-button.ts +++ b/src/frontend/components/button/lib/load-more-chronology-button.ts @@ -1,5 +1,4 @@ import { initializeRegisteredComponents } from "component"; -import { logging } from "logging"; import ErrorHandler from "util/errorHandler"; export class LoadMoreChronologyButton { @@ -9,7 +8,6 @@ export class LoadMoreChronologyButton { private errorHandler: ErrorHandler; constructor(private $el: JQuery) { - logging.log("Initializing LoadMoreChronologyButton for element:", $el); // Where to put the chronology entries this.$target = $('.chronology'); // Loading spinner