Add override view option to see restricted records (#814) - #650
Conversation
pwlodarski-ctrlo
left a comment
There was a problem hiding this comment.
PR reviewed - minor comments added.
| my @search; | ||
| return [] if $self->no_view_limits; | ||
| foreach my $view (@{$self->_view_limits}) | ||
| if (!$self->_view_limit_override) |
There was a problem hiding this comment.
Could this be an unless?
unless ($self->_view_limit_override)
There was a problem hiding this comment.
Thanks Pawel, see previous comment on other PR :)
| __PACKAGE__->load_namespaces; | ||
|
|
||
| our $VERSION = 111; | ||
| our $VERSION = 112; |
There was a problem hiding this comment.
Multiple PRs are open which have schema updates - the versioning or order of merging would need to be considered when it comes to it.
| } | ||
|
|
||
| # Setting a new view limit override | ||
| if (defined param('view_limit_override')) |
There was a problem hiding this comment.
Could this be: if (my $override = param('view_limit_override')) ?
There was a problem hiding this comment.
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)
There was a problem hiding this comment.
Makes sense, thanks for clarifying!
| schema => $schema, | ||
| ); | ||
| try { $record->find_current_id($sensitive_record->current_id) }; | ||
| like($@, qr/record not found/, "Unable to open limited record"); |
There was a problem hiding this comment.
In this case it is the result of the try block from the previous line, and should contain the error message being matched.
No description provided.