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

First Draft of Audit Log FE #137

Merged
merged 6 commits into from
Jan 16, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
9 changes: 6 additions & 3 deletions frontend/assets/css/overrides.css
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,12 @@ div.p-toast {
width: auto;
}

.code {
.code, .code-block {
font-family: monospace;
background-color: #eee;
padding: .25rem .5rem;
border-radius: .25rem;
padding: .5rem;
border-radius: .5rem;
display: inline-block;
font-size: 0.75rem;
white-space: pre-wrap;
}
10 changes: 1 addition & 9 deletions frontend/components/standard/Debug.vue
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ function createCircularReplacer (): (this: any, key: string, value: any) => any
header-class="surface-800"
>
<div
class="code surface-50"
class="code-block surface-50"
>
{{ valueAsStr }}
</div>
Expand All @@ -53,14 +53,6 @@ function createCircularReplacer (): (this: any, key: string, value: any) => any
width: fit-content;
display: inline-block;

.code {
display: inline-block;
font-size: 0.75rem;
line-height: 0.75rem;
white-space: pre-wrap;
font-family: monospace;
}

.p-accordion-header .p-accordion-header-link {
gap: 1rem;
padding: 0.5rem 0.75rem;
Expand Down
17 changes: 17 additions & 0 deletions frontend/lang/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -307,5 +307,22 @@
"Uploading": "Uploading",
"Validating": "Validating",
"Waiting": "Waiting"
},
"pages/audit-logs": {
"Action": "Action",
"Actor": "Actor",
"Actor ID": "Actor ID",
"Actor Owner ID": "Actor Owner ID",
"Actor Type": "Actor Type",
"Time": "Time",
"ID": "ID",
"Primary Target": "Primary Target",
"Primary Target ID": "Primary Target ID",
"Primary Target Owner": "Primary Target Owner",
"Primary Target Type": "Primary Target Type",
"Secondary Target": "Secondary Target",
"Secondary Target ID": "Secondary Target ID",
"Secondary Target Owner": "Secondary Target Owner",
"Secondary Target Type": "Secondary Target Type"
}
}
200 changes: 200 additions & 0 deletions frontend/lib/auditlogquery/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,200 @@
import { type AuditLogQueryReq, type AuditLogQuerySort, type AuditLogQueryWhere, type AuditLogQuerySortBy } from '@/openapi/generated/pacta'
import { type WritableComputedRef } from 'vue'
import { type useLocalePath } from '@nuxtjs/i18n/dist/runtime/composables'
import { computed } from 'vue'

const encodeAuditLogQuerySorts = (sorts: AuditLogQuerySort[]): string => {
const components: string[] = []
for (const sort of sorts) {
components.push(`${sort.ascending ? 'A' : 'D'}:${sort.by.replace('AuditLogQuerySortBy', '')}`)
}
return components.join(',')
}

const decodeAuditLogQuerySorts = (sorts: string): AuditLogQuerySort[] => {
const components = sorts.split(',')
const result: AuditLogQuerySort[] = []
for (const component of components) {
if (component === '') {
continue
}
const [dir, byStr] = component.split(':')
result.push({
ascending: dir === 'A',
by: 'AuditLogQuerySortBy' + byStr as AuditLogQuerySortBy,
})
}
return result
}

const encodeAuditLogQueryWheres = (wheres: AuditLogQueryWhere[]): string => {
const components: string[] = []
for (const where of wheres) {
if (where.inAction) {
components.push(`Action:${where.inAction.join('|')}`)
} else if (where.inActorId) {
components.push(`ActorId:${where.inActorId.join('|')}`)
} else if (where.inActorType) {
components.push(`ActorType:${where.inActorType.join('|')}`)
} else if (where.inActorOwnerId) {
components.push(`ActorOwnerId:${where.inActorOwnerId.join('|')}`)
} else if (where.inId) {
components.push(`Id:${where.inId.join('|')}`)
} else if (where.inTargetType) {
components.push(`TargetType:${where.inTargetType.join('|')}`)
} else if (where.inTargetId) {
components.push(`TargetId:${where.inTargetId.join('|')}`)
} else if (where.inTargetOwnerId) {
components.push(`TargetOwnerId:${where.inTargetOwnerId.join('|')}`)
} else if (where.minCreatedAt) {
components.push(`MinCreatedAt:${where.minCreatedAt}`)
} else if (where.maxCreatedAt) {
components.push(`MaxCreatedAt:${where.maxCreatedAt}`)
} else {
console.warn(new Error(`Unknown where: ${JSON.stringify(where)}`))
}
}
return components.join(',')
}

const decodeAudtLogQueryWheres = (wheres: string): AuditLogQueryWhere[] => {
const components = wheres.split(',')
const result: AuditLogQueryWhere[] = []
for (const component of components) {
if (component === '') {
continue
}
const [key, value] = component.split(':')
switch (key) {
case 'Action':
result.push({
inAction: value.split('|') as AuditLogQueryWhere['inAction'],
})
break
case 'ActorId':
result.push({
inActorId: value.split('|') as AuditLogQueryWhere['inActorId'],
})
break
case 'ActorType':
result.push({
inActorType: value.split('|') as AuditLogQueryWhere['inActorType'],
})
break
case 'ActorOwnerId':
result.push({
inActorOwnerId: value.split('|') as AuditLogQueryWhere['inActorOwnerId'],
})
break
case 'Id':
result.push({
inId: value.split('|') as AuditLogQueryWhere['inId'],
})
break
case 'TargetType':
result.push({
inTargetType: value.split('|') as AuditLogQueryWhere['inTargetType'],
})
break
case 'TargetId':
result.push({
inTargetId: value.split('|') as AuditLogQueryWhere['inTargetId'],
})
break
case 'TargetOwnerId':
result.push({
inTargetOwnerId: value.split('|') as AuditLogQueryWhere['inTargetOwnerId'],
})
break
case 'MinCreatedAt':
result.push({
minCreatedAt: value,
})
break
case 'MaxCreatedAt':
result.push({
maxCreatedAt: value,
})
break
default:
console.warn(new Error(`Unknown where: ${JSON.stringify(key)}`))
}
}
return result
}

const encodeAuditLogQueryLimit = (limit: number): string => {
if (limit === limitDefault) {
return ''
}
return `${limit}`
}

const decodeAuditLogQueryLimit = (limit: string): number => {
if (limit === '') {
return limitDefault
}
return parseInt(limit)
}

const encodeAuditLogQueryCursor = (cursor: string): string => {
return encodeURIComponent(cursor)
}

const decodeAuditLogQueryCursor = (cursor: string): string => {
return decodeURIComponent(cursor)
}

const sortsQP = 's'
const wheresQP = 'w'
const limitQP = 'l'
const limitDefault = 100
const cursorQP = 'c'
const pageURLBase = '/auditlog'

export const urlReactiveAuditLogQuery = (fromQueryReactiveWithDefault: (key: string, defaultValue: string) => WritableComputedRef<string>): WritableComputedRef<AuditLogQueryReq> => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

You're playing a dangerous game, making library functions that depend on access to things that use setup-only constructs like computed. It's already two levels of indirection (urlReactiveAuditLogQuery -> fromQueryReactiveWithDefault -> computed), it just makes it too easy to forget you should only be calling it in a setup function.

It looks like this will only ever be used once, you could just inline it into /audit-logs or into a component used by /audit-logs where it's unambiguously only ever called from setup

And more generally, I think this pattern of taking in functions that should only ever be called in setup is dicey, for the above mentioned reason that it obfuscates where things are being called.

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 think this isn't quite correct: I'm pretty sure (from experience around how we do editors, ex) that computed can be created and used outside of setup. One of the vue authors seems to hold this position too. Could you provide documentation for exactly what can and can't be called outside of setup? A 5 minute google left me without a clear north star.

By taking fromQueryReactiveWithDefault as an argument rather than invoking the composable to pull it out, we avoid invoking a composable (which I agree is a riskier game).

My main goal was just keeping the logic for constructing + decoding URLs that point to audit log queries in one place, to avoid exporting the ~10 helper methods and constants that would be needed to replicate this interface outside of this package. I agree that we shouldn't overly obfuscate where/when core functionality is being used, but I think this approach actually does a good job of balancing between the competing goals as I understand them.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh that's interesting, I thought computed fell into that bucket too. Looking at the Nuxt docs, it's basically all of the use* helpers (some of which are just compiler macros that get expanded at build time) that need to be invoked in setup, but computed isn't documented as such

const qSorts = fromQueryReactiveWithDefault(sortsQP, '')
const qWheres = fromQueryReactiveWithDefault(wheresQP, '')
const qLimit = fromQueryReactiveWithDefault(limitQP, '')
const qCursor = fromQueryReactiveWithDefault(cursorQP, '')

return computed({
get: (): AuditLogQueryReq => {
return {
sorts: decodeAuditLogQuerySorts(qSorts.value),
wheres: decodeAudtLogQueryWheres(qWheres.value),
limit: decodeAuditLogQueryLimit(qLimit.value),
cursor: decodeAuditLogQueryCursor(qCursor.value),
}
},
set: (value: AuditLogQueryReq) => {
qSorts.value = encodeAuditLogQuerySorts(value.sorts ?? [])
qWheres.value = encodeAuditLogQueryWheres(value.wheres)
qLimit.value = encodeAuditLogQueryLimit(value.limit ?? limitDefault)
qCursor.value = encodeAuditLogQueryCursor(value.cursor ?? '')
},
})
}

export const createURLAuditLogQuery = (localePath: ReturnType<typeof useLocalePath>, req: AuditLogQueryReq): string => {
gbdubs marked this conversation as resolved.
Show resolved Hide resolved
const qSorts = encodeAuditLogQuerySorts(req.sorts ?? [])
const qWheres = encodeAuditLogQueryWheres(req.wheres)
const qLimit = encodeAuditLogQueryLimit(req.limit ?? limitDefault)
const qCursor = encodeAuditLogQueryCursor(req.cursor ?? '')
const q = new URLSearchParams()
if (qSorts) {
q.set(sortsQP, qSorts)
}
if (qWheres) {
q.set(wheresQP, qWheres)
}
if (qLimit) {
q.set(limitQP, qLimit)
}
if (qCursor) {
q.set(cursorQP, qCursor)
}
const url = new URL(pageURLBase)
url.search = q.toString()
return localePath(url.toString())
}
Loading