-
Notifications
You must be signed in to change notification settings - Fork 0
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
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
5100cf6
WIP
gbdubs ca9997d
Merge branch 'main' into grady/auditlogfe2
gbdubs ab8b1b4
Finishes up First Draft
gbdubs cb2a895
Self Review
gbdubs 6d7f2b3
Merge branch 'main' into grady/auditlogfe2
gbdubs e2d0442
Addresses Review Comments
gbdubs File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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> => { | ||
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()) | ||
} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
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 likecomputed
. 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 asetup
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 fromsetup
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.There was a problem hiding this comment.
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 ofsetup
. 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.
There was a problem hiding this comment.
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 theuse*
helpers (some of which are just compiler macros that get expanded at build time) that need to be invoked insetup
, butcomputed
isn't documented as such