Skip to content

Add override view option to see restricted records (#814) #650

New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Open
abeverley wants to merge 1 commit into
base: dev
Choose a base branch
from
Open

Add override view option to see restricted records (#814) #650

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
145 changes: 89 additions & 56 deletions lib/GADS.pm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2224,15 +2224,16 @@ prefix '/:layout_name' => sub {
my $view = current_view($user, $layout);

my $records = GADS::Records->new(
user => $user,
layout => $layout,
schema => schema,
from => $fromdt,
to => $todt,
max_results => 1000,
view => $view,
search => session('search'),
view_limit_extra_id => current_view_limit_extra_id($user, $layout),
user => $user,
layout => $layout,
schema => schema,
from => $fromdt,
to => $todt,
max_results => 1000,
view => $view,
search => session('search'),
view_limit_extra_id => current_view_limit_extra_id($user, $layout),
view_limit_override_id => current_view_limit_override_id($user, $layout),
);

response_header "Cache-Control" => "max-age=0, must-revalidate, private";
Expand Down Expand Up @@ -2262,16 +2263,17 @@ prefix '/:layout_name' => sub {
my $view = current_view($user, $layout, $view_id);

my $records = GADS::Records->new(
from => $fromdt,
to => $todt,
exclusive => param('exclusive'),
user => $user,
layout => $layout,
schema => schema,
view => $view,
search => $is_dashboard ? undef : session('search'),
rewind => $is_dashboard ? undef : session('rewind'),
view_limit_extra_id => current_view_limit_extra_id($user, $layout),
from => $fromdt,
to => $todt,
exclusive => param('exclusive'),
user => $user,
layout => $layout,
schema => schema,
view => $view,
search => $is_dashboard ? undef : session('search'),
rewind => $is_dashboard ? undef : session('rewind'),
view_limit_extra_id => current_view_limit_extra_id($user, $layout),
view_limit_override_id => current_view_limit_override_id($user, $layout),
);

response_header "Cache-Control" => "max-age=0, must-revalidate, private";
Expand Down Expand Up @@ -2342,14 +2344,15 @@ prefix '/:layout_name' => sub {
forwardHome({ danger => "You do not have permission to bulk delete records"}, $layout->identifier.'/data')
unless $layout->user_can("bulk_delete");
my %params = (
user => $user,
search => session('search'),
layout => $layout,
schema => schema,
rewind => session('rewind'),
view => current_view($user, $layout),
view_limit_extra_id => current_view_limit_extra_id($user, $layout),
additional_filters => \@additional_filters,
user => $user,
search => session('search'),
layout => $layout,
schema => schema,
rewind => session('rewind'),
view => current_view($user, $layout),
view_limit_extra_id => current_view_limit_extra_id($user, $layout),
view_limit_override_id => current_view_limit_override_id($user, $layout),
additional_filters => \@additional_filters,
);
$params{limit_current_ids} = [body_parameters->get_all('delete_id')]
if body_parameters->get_all('delete_id');
Expand Down Expand Up @@ -2391,6 +2394,13 @@ prefix '/:layout_name' => sub {
session('persistent')->{view_limit_extra}->{$layout->instance_id} = $extra;
}

# Setting a new view limit override
if (defined param('view_limit_override'))

@pwlodarski-ctrlo pwlodarski-ctrlo Sep 1, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Could this be: if (my $override = param('view_limit_override')) ?

@abeverley abeverley Sep 1, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good question - normally, yes. However, in this case a defined but false value means to unset it (https://github.com/ctrlo/GADS/pull/650/changes#diff-88d1585c56a6b278e6f81ddada4991a8d7278f46f85a89e2af503130aaa2a3c9R4)

@pwlodarski-ctrlo pwlodarski-ctrlo Sep 1, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Makes sense, thanks for clarifying!

{
my $override = param('view_limit_override');
session('persistent')->{view_limit_override}->{$layout->instance_id} = $override;
}

my $new_view_id = param('view');
if (param 'views_other_user_clear')
{
Expand Down Expand Up @@ -2556,15 +2566,16 @@ prefix '/:layout_name' => sub {
elsif ($viewtype eq 'timeline')
{
my $records = GADS::Records->new(
user => $user,
view => $view,
search => session('search'),
layout => $layout,
user => $user,
view => $view,
search => session('search'),
layout => $layout,
# No "to" - will take appropriate number from today
from => DateTime->now, # Default
schema => schema,
rewind => session('rewind'),
view_limit_extra_id => current_view_limit_extra_id($user, $layout),
from => DateTime->now, # Default
schema => schema,
rewind => session('rewind'),
view_limit_extra_id => current_view_limit_extra_id($user, $layout),
view_limit_override_id => current_view_limit_override_id($user, $layout),
);
my $tl_options = session('persistent')->{tl_options}->{$layout->instance_id} ||= {};
if (param 'modal_timeline')
Expand Down Expand Up @@ -2660,13 +2671,14 @@ prefix '/:layout_name' => sub {
my $page = defined param('download') ? undef : session('page');

my %params = (
user => $user,
search => session('search'),
layout => $layout,
schema => schema,
rewind => session('rewind'),
additional_filters => \@additional_filters,
view_limit_extra_id => current_view_limit_extra_id($user, $layout),
user => $user,
search => session('search'),
layout => $layout,
schema => schema,
rewind => session('rewind'),
additional_filters => \@additional_filters,
view_limit_extra_id => current_view_limit_extra_id($user, $layout),
view_limit_override_id => current_view_limit_override_id($user, $layout),
);

# If this is a filter from a group view, then disable the group for
Expand Down Expand Up @@ -2826,6 +2838,7 @@ prefix '/:layout_name' => sub {

$params->{user_views} = $views->user_views;
$params->{views_limit_extra} = $views->views_limit_extra;
$params->{views_limit_override} = $views->views_limit_override;
$params->{current_view_limit_extra} = current_view_limit_extra($user, $layout) || $layout->default_view_limit_extra;
$params->{alerts} = $alert->all;
$params->{views_other_user} = session('views_other_user_id') && rset('User')->find(session('views_other_user_id')),
Expand Down Expand Up @@ -4119,14 +4132,15 @@ prefix '/:layout_name' => sub {

# The records to update
my %params = (
view => $view,
is_group => 0,
search => session('search'),
columns => [map { $_->id } $layout->all], # Need all columns to be able to write updated records
schema => schema,
user => $user,
layout => $layout,
view_limit_extra_id => current_view_limit_extra_id($user, $layout),
view => $view,
is_group => 0,
search => session('search'),
columns => [map { $_->id } $layout->all], # Need all columns to be able to write updated records
schema => schema,
user => $user,
layout => $layout,
view_limit_extra_id => current_view_limit_extra_id($user, $layout),
view_limit_override_id => current_view_limit_override_id($user, $layout),
);
$params{limit_current_ids} = [query_parameters->get_all('id')]
if query_parameters->get_all('id');
Expand Down Expand Up @@ -4575,6 +4589,24 @@ sub current_view_limit_extra_id
$view ? $view->id : undef;
}

sub current_view_limit_override
{ my ($user, $layout) = @_;
if (my $override_id = session('persistent')->{view_limit_override}->{$layout->instance_id})
{
# Check it's valid
my $override = schema->resultset('View')->find($override_id);
return $override
if $override && $override->instance_id == $override->instance_id && $override->is_limit_override;
}
return undef;
}

sub current_view_limit_override_id
{ my ($user, $layout) = @_;
my $view = current_view_limit_override($user, $layout);
$view ? $view->id : undef;
}

sub forwardHome {
my ($message, $page, %options) = @_;

Expand Down Expand Up @@ -4673,12 +4705,13 @@ sub _data_graph
my $layout = var 'layout';
my $view = current_view($user, $layout);
my $records = GADS::RecordsGraph->new(
user => $user,
search => session('search'),
view_limit_extra_id => current_view_limit_extra_id($user, $layout),
rewind => session('rewind'),
layout => $layout,
schema => schema,
user => $user,
search => session('search'),
view_limit_extra_id => current_view_limit_extra_id($user, $layout),
view_limit_override_id => current_view_limit_override_id($user, $layout),
rewind => session('rewind'),
layout => $layout,
schema => schema,
);
GADS::Graph::Data->new(
id => $id,
Expand Down
17 changes: 9 additions & 8 deletions lib/GADS/API.pm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1104,14 +1104,15 @@ sub _get_records {
my $length = $params->get('length') || 25;

my %params = (
user => $user,
schema => schema,
view => $view,
rows => $length,
page => 1 + ceil($start / $length),
layout => $layout,
rewind => session('rewind'),
view_limit_extra_id => current_view_limit_extra_id($user, $layout),
user => $user,
schema => schema,
view => $view,
rows => $length,
page => 1 + ceil($start / $length),
layout => $layout,
rewind => session('rewind'),
view_limit_extra_id => current_view_limit_extra_id($user, $layout),
view_limit_override_id => GADS::current_view_limit_override_id($user, $layout),
);
$params{is_group} = 0
if query_parameters->get('group_filter');
Expand Down
61 changes: 52 additions & 9 deletions lib/GADS/Records.pm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,24 @@ sub _build__view_limit_extra
return;
}

has _view_limit_override => (
is => 'lazy',
);

sub _build__view_limit_override
{ my $self = shift;
my $override_id = $self->view_limit_override_id
or return;
my $view_override = $self->schema->resultset('View')->find($override_id);
return GADS::View->new(
id => $view_override->id,
schema => $self->schema,
layout => $self->layout,
instance_id => $self->layout->instance_id,
) if $view_override->instance_id == $self->layout->instance_id
&& $view_override->is_limit_override;
}

# Any extra view limits in addition to those applied per-user
has view_limit_extra_id => (
is => 'lazy',
Expand All @@ -212,6 +230,13 @@ sub _build_view_limit_extra_id
$self->layout->default_view_limit_extra_id;
}

has view_limit_override_id => (
is => 'ro',
isa => Maybe[Int],
# Allow empty string from query params
coerce => sub { $_[0] || undef },
);

has no_view_limits => (
is => 'ro',
isa => Bool,
Expand All @@ -221,18 +246,21 @@ sub _view_limits_search
{ my ($self, %options) = @_;
my @search;
return [] if $self->no_view_limits;
foreach my $view (@{$self->_view_limits})
if (!$self->_view_limit_override)

@pwlodarski-ctrlo pwlodarski-ctrlo Sep 1, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Could this be an unless?

unless ($self->_view_limit_override)

@abeverley abeverley Sep 1, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks Pawel, see previous comment on other PR :)

@pwlodarski-ctrlo pwlodarski-ctrlo Sep 1, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Have done, thank you. :)

{
if (my $filter = $view->filter)
foreach my $view (@{$self->_view_limits})
{
my $decoded = $filter->as_hash;
if (keys %$decoded)
if (my $filter = $view->filter)
{
# Get the user search criteria.
# Ignore any permissions on this view, as otherwise an
# administrator-defined limited view of records may not take
# effect
push @search, $self->_search_construct($decoded, %options, ignore_perms => 1);
my $decoded = $filter->as_hash;
if (keys %$decoded)
{
# Get the user search criteria.
# Ignore any permissions on this view, as otherwise an
# administrator-defined limited view of records may not take
# effect
push @search, $self->_search_construct($decoded, %options, ignore_perms => 1);
}
}
}
}
Expand Down Expand Up @@ -800,8 +828,12 @@ sub _build__search_all_fields
);

my @columns_can_view;
my $override = $self->_view_limit_override;
foreach my $col ($self->layout->all(user_can_read => 1))
{
# If in override mode (showing all records) then do not allow a search
# on fields not contained in the override view
next if $override && !$override->has_column_id($col->id);
push @columns_can_view, $col->id;
push @columns_can_view, @{$col->curval_field_ids}
if ($col->type eq 'curval'); # Curval type needs all its columns from other layout
Expand Down Expand Up @@ -1933,6 +1965,14 @@ sub _build_columns_render
{ my $self = shift;

my @cols = grep $_->user_can('read'), @{$self->columns_selected};

# If an override view is selected, then need to only allow those columns to
# be viewed
if (my $override = $self->_view_limit_override)
{
@cols = grep $override->has_column_id($_->id), @cols;
}

if ($self->view && !@{$self->additional_filters})
{
# If in the normal grouped view, then move the grouped columns first in
Expand Down Expand Up @@ -2457,6 +2497,9 @@ sub rule_to_condition
$column
or return;

my $override = $self->_view_limit_override;
return if $override && !$override->has_column_id($column->id);

if ($filter->{operator} eq 'changed_after')
{
my $value;
Expand Down
2 changes: 1 addition & 1 deletion lib/GADS/Schema.pm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use base 'DBIx::Class::Schema';

__PACKAGE__->load_namespaces;

our $VERSION = 111;
our $VERSION = 112;

@pwlodarski-ctrlo pwlodarski-ctrlo Sep 1, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Multiple PRs are open which have schema updates - the versioning or order of merging would need to be considered when it comes to it.


our $IGNORE_PERMISSIONS;
our $IGNORE_PERMISSIONS_SEARCH;
Expand Down
2 changes: 2 additions & 0 deletions lib/GADS/Schema/Result/View.pm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ __PACKAGE__->add_columns(
{ data_type => "smallint", default_value => 0, is_nullable => 0 },
"is_limit_extra",
{ data_type => "smallint", default_value => 0, is_nullable => 0 },
"is_limit_override",
{ data_type => "smallint", default_value => 0, is_nullable => 0 },
"filter",
{ data_type => "text", is_nullable => 1 },
"instance_id",
Expand Down
15 changes: 15 additions & 0 deletions lib/GADS/View.pm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,21 @@ has columns => (
},
);

has _columns_hash => (
is => 'lazy',
);

sub _build__columns_hash
{ my $self = shift;
my %cols = map { $_ => 1 } @{$self->columns};
\%cols;
}

sub has_column_id
{ my ($self, $column_id) = @_;
$self->_columns_hash->{$column_id};
}

# Whether the view has a variable "CURUSER" condition
has has_curuser => (
is => 'lazy',
Expand Down
Loading
Loading