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

Enhancement: Variables correctifs #1303

Merged
merged 10 commits into from
Apr 3, 2023
24 changes: 16 additions & 8 deletions src/components/VariableCreateModal.vue
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,6 @@
{{ localization.info.create }}
</p-button>
</template>
<template #cancel>
<p-button inset @click="internalValue = false">
{{ localization.info.close }}
</p-button>
</template>
</p-modal>
</template>

Expand All @@ -39,6 +34,8 @@
import { Variable, VariableCreate } from '@/models'
import { isHandle, isRequired, isString } from '@/utilities'

const VALUE_MAX_CHARS = 255

const props = defineProps<{
showModal: boolean,
}>()
Expand All @@ -59,7 +56,15 @@

const api = useWorkspaceApi()

const isUnique: ValidationRule<string | undefined> = async (value, label, { signal, source, previousValue }) => {
const valueIsLessThanOrEqual: ValidationRule<string | undefined> = (value) => {
if (isNull(value) || !isString(value)) {
return false
}

return value.length <= VALUE_MAX_CHARS ? true : localization.error.stringValueTooLong(localization.info.value, VALUE_MAX_CHARS)
znicholasbrown marked this conversation as resolved.
Show resolved Hide resolved
}

const nameIsUnique: ValidationRule<string | undefined> = async (value, label, { signal, source, previousValue }) => {
if (value === previousValue) {
return
}
Expand Down Expand Up @@ -91,8 +96,8 @@
const tags = ref<string[]>([])

const rules: Record<string, ValidationRule<string | undefined>[]> = {
name: [isRequired(localization.info.name), isHandle(localization.info.name), isUnique],
value: [isRequired(localization.info.value)],
name: [isRequired(localization.info.name), isHandle(localization.info.name), nameIsUnique],
value: [isRequired(localization.info.value), valueIsLessThanOrEqual],
}

const { error: nameErrorMessage, state: nameState } = useValidation(name, localization.info.name, rules.name)
Expand All @@ -114,6 +119,9 @@
showToast(localization.success.createVariable, 'success')
emit('create', variable)
internalValue.value = false
name.value = undefined
value.value = undefined
tags.value = []
znicholasbrown marked this conversation as resolved.
Show resolved Hide resolved
} catch (error) {
console.error(error)
showToast(localization.error.createVariable, 'error')
Expand Down
23 changes: 14 additions & 9 deletions src/components/VariableEditModal.vue
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<template>
<p-modal v-model:showModal="internalValue" :title="localization.info.newVariable">
<p-modal v-model:showModal="internalValue" :title="localization.info.editVariable(name)">
<p-form @submit="submit">
<p-content>
<p-label :label="localization.info.name" :state="nameState" :message="nameErrorMessage">
Expand All @@ -21,11 +21,6 @@
{{ localization.info.save }}
</p-button>
</template>
<template #cancel>
<p-button inset @click="internalValue = false">
{{ localization.info.close }}
</p-button>
</template>
</p-modal>
</template>

Expand All @@ -39,6 +34,8 @@
import { Variable, VariableEdit } from '@/models'
import { isHandle, isRequired, isString } from '@/utilities'

const VALUE_MAX_CHARS = 255

const props = defineProps<{
variable: Variable,
showModal: boolean,
Expand All @@ -60,7 +57,15 @@

const api = useWorkspaceApi()

const isUnique: ValidationRule<string | undefined> = async (value, label, { signal, source, previousValue }) => {
const valueIsLessThanOrEqual: ValidationRule<string | undefined> = (value) => {
if (isNull(value) || !isString(value)) {
return false
}
znicholasbrown marked this conversation as resolved.
Show resolved Hide resolved

return value.length <= VALUE_MAX_CHARS ? true : localization.error.stringValueTooLong(localization.info.value, VALUE_MAX_CHARS)
znicholasbrown marked this conversation as resolved.
Show resolved Hide resolved
}

const nameIsUnique: ValidationRule<string | undefined> = async (value, label, { signal, source, previousValue }) => {
if (value === previousValue) {
return
}
Expand Down Expand Up @@ -97,8 +102,8 @@
const tags = ref<string[]>(props.variable.tags)

const rules: Record<string, ValidationRule<string | undefined>[]> = {
name: [isRequired(localization.info.name), isHandle(localization.info.name), isUnique],
value: [isRequired(localization.info.value)],
name: [isRequired(localization.info.name), isHandle(localization.info.name), nameIsUnique],
value: [isRequired(localization.info.value), valueIsLessThanOrEqual],
}

const { error: nameErrorMessage, state: nameState } = useValidation(name, localization.info.name, rules.name)
Expand Down
51 changes: 40 additions & 11 deletions src/components/VariablesTable.vue
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,21 @@
</div>
</template>

<p-table :data="variables" :columns="columns">
<p-table :data="variables" :columns="columns" :column-classes="columnClass">
<template #selection-heading>
<p-checkbox v-if="variables.length" v-model="model" @update:model-value="selectAllVariables" />
<div v-else />
<div class="variables-table__selection">
<p-checkbox v-if="variables.length" v-model="model" @update:model-value="selectAllVariables" />
</div>
</template>

<template #selection="{ row }">
<p-checkbox v-model="selectedVariables" :value="row.id" />
</template>

<template #name="{ row }">
<span>{{ row.name }}</span>
<div class="variables-table__name">
{{ row.name }}
</div>
</template>

<template #updated="{ row }">
Expand All @@ -44,8 +47,8 @@
</template>

<template #action="{ row }">
<div class="variables-table__action">
<VariableMenu :variable="row" size="xs" @delete="refreshSubscriptions" />
<div :key="row.id" class="variables-table__action">
<VariableMenu :variable="row" size="xs" @delete="refreshSubscriptions" @update="handleUpdate" />
</div>
</template>

Expand All @@ -71,13 +74,13 @@
</template>

<script lang="ts" setup>
import { PTable, PEmptyResults, CheckboxModel } from '@prefecthq/prefect-design'
import { PTable, PEmptyResults, CheckboxModel, TableColumn, ClassValue } from '@prefecthq/prefect-design'
import { useDebouncedRef, useSubscription } from '@prefecthq/vue-compositions'
import { computed, ref } from 'vue'
import { VariablesDeleteButton, VariableMenu, ResultsCount, SearchInput, SelectedCount } from '@/components'
import { useCan, useVariablesFilter, useWorkspaceApi } from '@/compositions'
import { localization } from '@/localization'
import { VariablesFilter } from '@/models/Filters'
import { VariablesFilter, Variable } from '@/models'
import { variableSortOptions } from '@/types'
import { formatDateTimeNumeric } from '@/utilities/dates'

Expand Down Expand Up @@ -118,12 +121,11 @@
{
property: 'name',
label: 'Name',
width: '64px',
width: '124px',
},
{
property: 'value',
label: 'Value',
width: '124px',
},
{
property: 'updated',
Expand All @@ -133,14 +135,20 @@
{
property: 'tags',
label: 'Tags',
width: '124px',
width: '248px',
},
{
label: 'Action',
width: '42px',
},
]

function columnClass(column: TableColumn): ClassValue {
return {
'variables-table__value-td': column.label === 'Value',
}
}

const selectedVariables = ref<string[]>([])
const selectAllVariables = (allVariablesSelected: CheckboxModel): string[] => {
if (allVariablesSelected) {
Expand Down Expand Up @@ -175,13 +183,18 @@

const emit = defineEmits<{
(event: 'delete'): void,
(event: 'update', value: Variable): void,
}>()

const deleteVariables = (): void => {
selectedVariables.value = []
refreshSubscriptions()
emit('delete')
}

const handleUpdate = (variable: Variable): void => {
emit('update', variable)
}
</script>

<style>
Expand Down Expand Up @@ -213,5 +226,21 @@

.variables-table__action { @apply
text-right
max-w-[42px]
}

.variables-table__selection { @apply
max-w-[20px]
}

.variables-table__value-td,
.variables-table__name { @apply
min-w-0
max-w-0
truncate
}

.variables-table__name { @apply
max-w-[124px]
}
</style>
1 change: 1 addition & 0 deletions src/localization/locale/en.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ export const en = {
resumeFlowRun: 'Failed to resume flow run',
retryRun: 'Failed to retry flow run',
scheduleFlowRun: 'Failed to schedule flow run',
stringValueTooLong: (subject: string, chars: number) => `${subject} must be less than or equal to ${chars} characters`,
submitNotification: 'Failed to submit notification',
updateBlock: 'Failed to update block',
updateNotification: 'Failed to update notification',
Expand Down