Skip to content

Update usage queries to reference auditlog.UserAuditEvent #255

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.

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

Merged
merged 1 commit into from
Aug 5, 2025

Conversation

labkey-adam
Copy link

@labkey-adam labkey-adam commented Aug 4, 2025

Rationale

EHR tests have been failing after the deprecation of the legacy "audit union" table because these usage queries still use it. Switching them to query the provisioned UserAuditEvent table directly is more straightforward and likely more efficient.

Related Pull Requests

Copy link
Member

@labkey-tchad labkey-tchad left a comment

Choose a reason for hiding this comment

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

There's another one in WNPRC_EHR

@bbimber
Copy link
Collaborator

bbimber commented Aug 4, 2025

@labkey-adam and/or @labkey-jeckels: do you know if this new query can be read by non-admin users? I do not recall if this specific use-case can be exercised by non-admins. However, this was historically a problem for user-facing things that tried to use the audit log.

@bbimber
Copy link
Collaborator

bbimber commented Aug 4, 2025

@labkey-adam and/or @labkey-jeckels: do you know if this new query can be read by non-admin users? I do not recall if this specific use-case can be exercised by non-admins. However, this was historically a problem for user-facing things that tried to use the audit log.

nevermind, this change is simpler than I initially thought. I think my question is still valid, but if there was a pre-existing problem, this wouldnt change it one way or another.

@labkey-adam
Copy link
Author

@labkey-adam and/or @labkey-jeckels: do you know if this new query can be read by non-admin users? I do not recall if this specific use-case can be exercised by non-admins. However, this was historically a problem for user-facing things that tried to use the audit log.

nevermind, this change is simpler than I initially thought. I think my question is still valid, but if there was a pre-existing problem, this wouldnt change it one way or another.

Correct, the old query was simply a union of all the event queries, meant to mimic the really old single table that held all events, so perms have not changed. Also note that the "new" queries are 13 years old. Only admins and those explicitly assigned the "See Audit Log Events" role can read the audit log tables.

@labkey-adam labkey-adam merged commit cffb250 into develop Aug 5, 2025
12 of 13 checks passed
@labkey-adam labkey-adam deleted the fb_usage_queries branch August 5, 2025 18:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants