Skip to content
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

Audit Log API Layer #99

Merged
merged 3 commits into from
Jan 2, 2024
Merged

Audit Log API Layer #99

merged 3 commits into from
Jan 2, 2024

Conversation

gbdubs
Copy link
Contributor

@gbdubs gbdubs commented Dec 28, 2023

  • Creates API layer for Audit Log Queries (largely mirroring the database layer - I'm thinking of how lovely and useful that approach was for ABOUND, and seeking to replicate it here, albeit for likely a less user-visible use case).
  • Tweaks a few things I got wrong in the initial audit log design:
    • we want the ActorType to refer to the pathway through which the user is accessing, rather than the property of the user. This unfortunately required a DB migration to add the new enum values.
    • Adds a ParseSortBy method to the DB Layer.
    • Tweaks naming to be more consistent with the core types.

@gbdubs gbdubs requested a review from bcspragu December 28, 2023 21:06
Comment on lines +111 to +153
result := &db.AuditLogQueryWhere{}
if i.InId != nil {
result.InID = fromStrs[pacta.AuditLogID](*i.InId)
}
if i.MinCreatedAt != nil {
result.MinCreatedAt = *i.MinCreatedAt
}
if i.MaxCreatedAt != nil {
result.MaxCreatedAt = *i.MaxCreatedAt
}
if i.InAction != nil {
as, err := convAll(*i.InAction, auditLogActionFromOAPI)
if err != nil {
return nil, fmt.Errorf("converting audit log query where in action: %w", err)
}
result.InAction = as
}
if i.InActorType != nil {
at, err := convAll(*i.InActorType, auditLogActorTypeFromOAPI)
if err != nil {
return nil, fmt.Errorf("converting audit log query where in actor type: %w", err)
}
result.InActorType = at
}
if i.InActorId != nil {
result.InActorID = *i.InActorId
}
if i.InActorOwnerId != nil {
result.InActorOwnerID = fromStrs[pacta.OwnerID](*i.InActorOwnerId)
}
if i.InTargetType != nil {
tt, err := convAll(*i.InTargetType, auditLogTargetTypeFromOAPI)
if err != nil {
return nil, fmt.Errorf("converting audit log query where in target type: %w", err)
}
result.InTargetType = tt
}
if i.InTargetId != nil {
result.InTargetID = *i.InTargetId
}
if i.InTargetOwnerId != nil {
result.InTargetOwnerID = fromStrs[pacta.OwnerID](*i.InTargetOwnerId)
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Did RMI ask for any/all of this functionality? Seems like a lot of ways to slice and dice stuff out of the gate, versus just the MVP.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had a long convo with Hodie about this early in the project. My recollection is that the goal was to make audit logs useful for debuging + analytics, in addition to serving their purpose of giving folks confidence in the data handling practices.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Another benefit is that the API exposed maps cleanly onto the PV Pagination Table interface (sorting, filtering etc) so building a maintainable frontend for this is going to be delightfully easy.

cmd/server/pactasrv/conv/pacta_to_oapi.go Outdated Show resolved Hide resolved
@gbdubs gbdubs enabled auto-merge (squash) January 2, 2024 15:13
@gbdubs gbdubs merged commit 85a0982 into main Jan 2, 2024
2 checks passed
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.

2 participants