Merge branch 'fix/record-save-diagnostics' into 'main'
fix: запись не сохраняется — диагностика + bean-валидация 400 See merge request 2-6/2-6-4/terravault/ordinis!251
This commit is contained in:
@@ -219,4 +219,39 @@ describe('SchemaDrivenForm', () => {
|
|||||||
)
|
)
|
||||||
expect(screen.getByText('Дубликат business_key')).toBeInTheDocument()
|
expect(screen.getByText('Дубликат business_key')).toBeInTheDocument()
|
||||||
})
|
})
|
||||||
|
|
||||||
|
// Regression: после первой неудачной AJV-валидации manual-ошибки
|
||||||
|
// (setError на data.*) застревали в formState.errors. На повторном Save
|
||||||
|
// RHF handleSubmit видел непустой errors и молча НЕ вызывал submit —
|
||||||
|
// форма «не сохранялась» после исправления полей, ошибок не видно.
|
||||||
|
it('повторный submit после исправления валидации вызывает onSubmit', async () => {
|
||||||
|
const user = userEvent.setup()
|
||||||
|
const onSubmit = vi.fn()
|
||||||
|
const { container } = renderWithI18n(
|
||||||
|
<SchemaDrivenForm
|
||||||
|
schema={minimalSchema}
|
||||||
|
supportedLocales={['ru-RU']}
|
||||||
|
defaultLocale="ru-RU"
|
||||||
|
mode="create"
|
||||||
|
isPending={false}
|
||||||
|
onSubmit={onSubmit}
|
||||||
|
onCancel={() => {}}
|
||||||
|
/>,
|
||||||
|
)
|
||||||
|
const save = () => screen.getByRole('button', { name: /^Сохранить$/i })
|
||||||
|
const field = (name: string): HTMLElement => {
|
||||||
|
const el = container.querySelector(`input[name="${name}"]`)
|
||||||
|
if (!el) throw new Error(`field input not found: ${name}`)
|
||||||
|
return el as HTMLElement
|
||||||
|
}
|
||||||
|
// Попытка 1 — required code/name пустые → AJV fail, submit не проходит.
|
||||||
|
await user.click(save())
|
||||||
|
expect(onSubmit).not.toHaveBeenCalled()
|
||||||
|
// Исправляем — заполняем оба required-поля.
|
||||||
|
await user.type(field('data.code'), 'X1')
|
||||||
|
await user.type(field('data.name'), 'Тест')
|
||||||
|
// Попытка 2 — данные валидны, должно вызвать onSubmit.
|
||||||
|
await user.click(save())
|
||||||
|
expect(onSubmit).toHaveBeenCalledTimes(1)
|
||||||
|
})
|
||||||
})
|
})
|
||||||
|
|||||||
@@ -268,10 +268,18 @@ export const SchemaDrivenForm = ({
|
|||||||
}, [visibleFieldCount, totalFieldCount])
|
}, [visibleFieldCount, totalFieldCount])
|
||||||
|
|
||||||
const submit: SubmitHandler<FormValues> = (values) => {
|
const submit: SubmitHandler<FormValues> = (values) => {
|
||||||
|
// [ordinis-diag] DIAGNOSTIC BUILD — временная инструментация для поиска
|
||||||
|
// причины «Save ничего не делает». Удалить после диагностики.
|
||||||
|
console.warn('[ordinis-diag] submit(): RHF-валидация пройдена, handler запущен', {
|
||||||
|
mode,
|
||||||
|
businessKey: values.businessKey,
|
||||||
|
dataKeys: Object.keys(values.data ?? {}),
|
||||||
|
})
|
||||||
if (values.validFrom && values.validTo) {
|
if (values.validFrom && values.validTo) {
|
||||||
const from = new Date(values.validFrom)
|
const from = new Date(values.validFrom)
|
||||||
const to = new Date(values.validTo)
|
const to = new Date(values.validTo)
|
||||||
if (!isNaN(from.getTime()) && !isNaN(to.getTime()) && from >= to) {
|
if (!isNaN(from.getTime()) && !isNaN(to.getTime()) && from >= to) {
|
||||||
|
console.warn('[ordinis-diag] submit(): ЗАБЛОКИРОВАНО — validTo <= validFrom')
|
||||||
setError('validTo', { type: 'manual', message: t('form.error.validToBeforeFrom') })
|
setError('validTo', { type: 'manual', message: t('form.error.validToBeforeFrom') })
|
||||||
setActiveTab('identity')
|
setActiveTab('identity')
|
||||||
return
|
return
|
||||||
@@ -279,7 +287,13 @@ export const SchemaDrivenForm = ({
|
|||||||
}
|
}
|
||||||
if (validator) {
|
if (validator) {
|
||||||
const ok = validator(values.data)
|
const ok = validator(values.data)
|
||||||
|
console.warn('[ordinis-diag] submit(): AJV-валидация →', {
|
||||||
|
ok,
|
||||||
|
errorCount: validator.errors?.length ?? 0,
|
||||||
|
errors: validator.errors,
|
||||||
|
})
|
||||||
if (!ok && validator.errors && validator.errors.length > 0) {
|
if (!ok && validator.errors && validator.errors.length > 0) {
|
||||||
|
console.warn('[ordinis-diag] submit(): ЗАБЛОКИРОВАНО — AJV errors, onSubmit НЕ вызван')
|
||||||
applyAjvErrors(validator.errors, setError)
|
applyAjvErrors(validator.errors, setError)
|
||||||
const firstField = firstFieldFromErrors(validator.errors)
|
const firstField = firstFieldFromErrors(validator.errors)
|
||||||
if (firstField) {
|
if (firstField) {
|
||||||
@@ -288,10 +302,15 @@ export const SchemaDrivenForm = ({
|
|||||||
}
|
}
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
} else {
|
||||||
|
console.warn('[ordinis-diag] submit(): validator == null (схема не скомпилировалась)')
|
||||||
}
|
}
|
||||||
const derivedKey = idSource && mode === 'create'
|
const derivedKey = idSource && mode === 'create'
|
||||||
? String(values.data?.[idSource] ?? '').trim()
|
? String(values.data?.[idSource] ?? '').trim()
|
||||||
: values.businessKey.trim()
|
: values.businessKey.trim()
|
||||||
|
console.warn('[ordinis-diag] submit(): вызываю onSubmit() → мутация', {
|
||||||
|
businessKey: derivedKey,
|
||||||
|
})
|
||||||
onSubmit({
|
onSubmit({
|
||||||
businessKey: derivedKey,
|
businessKey: derivedKey,
|
||||||
data: values.data,
|
data: values.data,
|
||||||
@@ -300,6 +319,13 @@ export const SchemaDrivenForm = ({
|
|||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// [ordinis-diag] RHF onInvalid — вызывается когда handleSubmit РЕШИЛ что
|
||||||
|
// форма невалидна и НЕ вызвал submit(). Показывает что именно RHF считает
|
||||||
|
// ошибкой (включая застрявшие manual-ошибки). Удалить после диагностики.
|
||||||
|
const diagOnInvalid = (errs: unknown) => {
|
||||||
|
console.warn('[ordinis-diag] handleSubmit ЗАБЛОКИРОВАН (onInvalid) — formState.errors:', errs)
|
||||||
|
}
|
||||||
|
|
||||||
const tabErrorCount = countErrorsPerSection(formState.errors, buckets, sectionOrder)
|
const tabErrorCount = countErrorsPerSection(formState.errors, buckets, sectionOrder)
|
||||||
const visibleTabs: TabItem[] = sectionOrder
|
const visibleTabs: TabItem[] = sectionOrder
|
||||||
.filter((id) => id === 'identity' || (buckets[id]?.length ?? 0) > 0)
|
.filter((id) => id === 'identity' || (buckets[id]?.length ?? 0) > 0)
|
||||||
@@ -474,7 +500,12 @@ export const SchemaDrivenForm = ({
|
|||||||
return (
|
return (
|
||||||
<form
|
<form
|
||||||
id={formId}
|
id={formId}
|
||||||
onSubmit={handleSubmit(submit)}
|
onSubmit={(e) => {
|
||||||
|
// [ordinis-diag] DIAGNOSTIC — событие submit формы вообще дошло?
|
||||||
|
// Удалить после диагностики.
|
||||||
|
console.warn('[ordinis-diag] <form> submit event сработал (formId=' + String(formId) + ')')
|
||||||
|
void handleSubmit(submit, diagOnInvalid)(e)
|
||||||
|
}}
|
||||||
ref={formContainerRef}
|
ref={formContainerRef}
|
||||||
>
|
>
|
||||||
<div className="space-y-4 min-w-0">
|
<div className="space-y-4 min-w-0">
|
||||||
|
|||||||
@@ -578,6 +578,12 @@ function DictionaryDetail() {
|
|||||||
const rawRecordQuery = useRecordRaw(name, editingKey)
|
const rawRecordQuery = useRecordRaw(name, editingKey)
|
||||||
|
|
||||||
const handleSubmit = (req: CreateRecordRequest) => {
|
const handleSubmit = (req: CreateRecordRequest) => {
|
||||||
|
// [ordinis-diag] DIAGNOSTIC BUILD — удалить после диагностики.
|
||||||
|
console.warn('[ordinis-diag] route.handleSubmit(): получен req от формы', {
|
||||||
|
editKind: edit.kind,
|
||||||
|
approvalRequired,
|
||||||
|
businessKey: req.businessKey,
|
||||||
|
})
|
||||||
// Fill validFrom at submit (review #5): defaultValues в RHF берётся
|
// Fill validFrom at submit (review #5): defaultValues в RHF берётся
|
||||||
// ТОЛЬКО при mount. Если user открыл modal в 12:00 и submit в 12:05 —
|
// ТОЛЬКО при mount. Если user открыл modal в 12:00 и submit в 12:05 —
|
||||||
// `validFrom` в req это значение из 12:00. Backend check `validFrom >
|
// `validFrom` в req это значение из 12:00. Backend check `validFrom >
|
||||||
@@ -631,14 +637,20 @@ function DictionaryDetail() {
|
|||||||
)
|
)
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
console.warn('[ordinis-diag] route: вызываю createMut.mutate() — POST /records')
|
||||||
createMut.mutate(
|
createMut.mutate(
|
||||||
{ payload: submitReq, idempotencyKey: recordIdempotencyKey },
|
{ payload: submitReq, idempotencyKey: recordIdempotencyKey },
|
||||||
{
|
{
|
||||||
onSuccess: () => {
|
onSuccess: () => {
|
||||||
|
console.warn('[ordinis-diag] createMut onSuccess — запись создана')
|
||||||
setEdit({ kind: 'closed' })
|
setEdit({ kind: 'closed' })
|
||||||
resetRecordIdempotencyKey()
|
resetRecordIdempotencyKey()
|
||||||
},
|
},
|
||||||
onError: (err) => {
|
onError: (err) => {
|
||||||
|
console.warn('[ordinis-diag] createMut onError', {
|
||||||
|
status: (err as { response?: { status?: number } })?.response?.status,
|
||||||
|
data: (err as { response?: { data?: unknown } })?.response?.data,
|
||||||
|
})
|
||||||
// Create rarely conflicts (no prior record to lock), но если backend
|
// Create rarely conflicts (no prior record to lock), но если backend
|
||||||
// вернёт 409 по уникальному ключу — businessKey возьмём из payload.
|
// вернёт 409 по уникальному ключу — businessKey возьмём из payload.
|
||||||
handleSaveError(err, submitReq.businessKey ?? '')
|
handleSaveError(err, submitReq.businessKey ?? '')
|
||||||
@@ -669,6 +681,7 @@ function DictionaryDetail() {
|
|||||||
)
|
)
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
console.warn('[ordinis-diag] route: вызываю updateMut.mutate() — PUT /records/' + edit.record.businessKey)
|
||||||
updateMut.mutate(
|
updateMut.mutate(
|
||||||
{
|
{
|
||||||
businessKey: edit.record.businessKey,
|
businessKey: edit.record.businessKey,
|
||||||
@@ -677,10 +690,17 @@ function DictionaryDetail() {
|
|||||||
},
|
},
|
||||||
{
|
{
|
||||||
onSuccess: () => {
|
onSuccess: () => {
|
||||||
|
console.warn('[ordinis-diag] updateMut onSuccess — запись обновлена')
|
||||||
setEdit({ kind: 'closed' })
|
setEdit({ kind: 'closed' })
|
||||||
resetRecordIdempotencyKey()
|
resetRecordIdempotencyKey()
|
||||||
},
|
},
|
||||||
onError: (err) => handleSaveError(err, edit.record.businessKey),
|
onError: (err) => {
|
||||||
|
console.warn('[ordinis-diag] updateMut onError', {
|
||||||
|
status: (err as { response?: { status?: number } })?.response?.status,
|
||||||
|
data: (err as { response?: { data?: unknown } })?.response?.data,
|
||||||
|
})
|
||||||
|
handleSaveError(err, edit.record.businessKey)
|
||||||
|
},
|
||||||
},
|
},
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|||||||
+27
@@ -9,6 +9,7 @@ import org.springframework.beans.factory.ObjectProvider;
|
|||||||
import org.springframework.http.HttpStatus;
|
import org.springframework.http.HttpStatus;
|
||||||
import org.springframework.http.ResponseEntity;
|
import org.springframework.http.ResponseEntity;
|
||||||
import org.springframework.http.converter.HttpMessageNotReadableException;
|
import org.springframework.http.converter.HttpMessageNotReadableException;
|
||||||
|
import org.springframework.web.bind.MethodArgumentNotValidException;
|
||||||
import org.springframework.web.bind.annotation.ExceptionHandler;
|
import org.springframework.web.bind.annotation.ExceptionHandler;
|
||||||
import org.springframework.web.bind.annotation.RestControllerAdvice;
|
import org.springframework.web.bind.annotation.RestControllerAdvice;
|
||||||
import org.springframework.web.server.ResponseStatusException;
|
import org.springframework.web.server.ResponseStatusException;
|
||||||
@@ -140,6 +141,32 @@ public class GlobalExceptionHandler {
|
|||||||
return ResponseEntity.status(e.getStatusCode()).body(body);
|
return ResponseEntity.status(e.getStatusCode()).body(body);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Bean-валидация {@code @Valid @RequestBody} провалилась — клиент прислал
|
||||||
|
* тело, не прошедшее {@code @NotBlank}/{@code @NotNull}/{@code @Size} и т.п.
|
||||||
|
*
|
||||||
|
* <p>До этого handler'а {@link MethodArgumentNotValidException} падал в
|
||||||
|
* generic {@link Exception} catch ниже → 500 {@code internal_error} с пустым
|
||||||
|
* traceId вместо 400. Эффект: admin-ui на создании/редактировании записи
|
||||||
|
* получал бесполезный «Internal server error» без указания полей, форма
|
||||||
|
* молча не сохранялась. Теперь — 400 {@code validation_failed} с
|
||||||
|
* пофайловой разбивкой ошибок (как у доменного {@link ValidationException},
|
||||||
|
* только статус 400 — тело запроса malformed, а не бизнес-правило).
|
||||||
|
*/
|
||||||
|
@ExceptionHandler(MethodArgumentNotValidException.class)
|
||||||
|
public ResponseEntity<ApiErrorResponse> onMethodArgumentNotValid(MethodArgumentNotValidException e) {
|
||||||
|
List<ApiErrorResponse.FieldError> fields = e.getBindingResult().getFieldErrors().stream()
|
||||||
|
.map(fe -> new ApiErrorResponse.FieldError(
|
||||||
|
fe.getField(),
|
||||||
|
fe.getCode() != null ? fe.getCode() : "invalid",
|
||||||
|
fe.getDefaultMessage() != null ? fe.getDefaultMessage() : "invalid"))
|
||||||
|
.toList();
|
||||||
|
log.debug("Bean validation failed: {} field error(s)", fields.size());
|
||||||
|
var body = ApiErrorResponse.withFieldErrors(
|
||||||
|
400, "validation_failed", "Запрос не прошёл валидацию", fields, currentTraceId());
|
||||||
|
return ResponseEntity.badRequest().body(body);
|
||||||
|
}
|
||||||
|
|
||||||
@ExceptionHandler(Exception.class)
|
@ExceptionHandler(Exception.class)
|
||||||
public ResponseEntity<ApiErrorResponse> onGeneric(Exception e) {
|
public ResponseEntity<ApiErrorResponse> onGeneric(Exception e) {
|
||||||
log.error("Unhandled exception in REST API", e);
|
log.error("Unhandled exception in REST API", e);
|
||||||
|
|||||||
+63
@@ -0,0 +1,63 @@
|
|||||||
|
package cloud.nstart.terravault.ordinis.restapi.error;
|
||||||
|
|
||||||
|
import io.micrometer.tracing.Tracer;
|
||||||
|
import org.junit.jupiter.api.Test;
|
||||||
|
import org.springframework.beans.factory.ObjectProvider;
|
||||||
|
import org.springframework.core.MethodParameter;
|
||||||
|
import org.springframework.http.ResponseEntity;
|
||||||
|
import org.springframework.validation.BeanPropertyBindingResult;
|
||||||
|
import org.springframework.validation.BindingResult;
|
||||||
|
import org.springframework.validation.FieldError;
|
||||||
|
import org.springframework.web.bind.MethodArgumentNotValidException;
|
||||||
|
|
||||||
|
import java.util.Collections;
|
||||||
|
import java.util.Iterator;
|
||||||
|
|
||||||
|
import static org.assertj.core.api.Assertions.assertThat;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Unit tests для {@link GlobalExceptionHandler}.
|
||||||
|
*
|
||||||
|
* <p>Regression: {@link MethodArgumentNotValidException} (провал bean-валидации
|
||||||
|
* {@code @Valid @RequestBody}) раньше не имел собственного handler'а → падал в
|
||||||
|
* generic {@code Exception} catch → HTTP 500 {@code internal_error}. admin-ui
|
||||||
|
* на создании/редактировании записи получал бесполезный «Internal server error»,
|
||||||
|
* форма молча не сохранялась. Должен возвращать 400 {@code validation_failed}.
|
||||||
|
*/
|
||||||
|
class GlobalExceptionHandlerTest {
|
||||||
|
|
||||||
|
/** Пустой ObjectProvider — currentTraceId() получит null, без трассировки. */
|
||||||
|
private static final ObjectProvider<Tracer> NO_TRACER = new ObjectProvider<>() {
|
||||||
|
@Override public Tracer getObject() { throw new UnsupportedOperationException(); }
|
||||||
|
@Override public Tracer getObject(Object... args) { throw new UnsupportedOperationException(); }
|
||||||
|
@Override public Tracer getIfAvailable() { return null; }
|
||||||
|
@Override public Tracer getIfUnique() { return null; }
|
||||||
|
@Override public Iterator<Tracer> iterator() { return Collections.emptyIterator(); }
|
||||||
|
};
|
||||||
|
|
||||||
|
private final GlobalExceptionHandler handler = new GlobalExceptionHandler(NO_TRACER);
|
||||||
|
|
||||||
|
/** Цель только для конструктора {@link MethodParameter} — нужен реальный параметр метода. */
|
||||||
|
@SuppressWarnings("unused")
|
||||||
|
private void sampleEndpoint(String body) {}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
void beanValidationFailureReturns400WithFieldErrors() throws Exception {
|
||||||
|
BindingResult br = new BeanPropertyBindingResult(new Object(), "createRecordRequest");
|
||||||
|
br.addError(new FieldError("createRecordRequest", "businessKey", "must not be blank"));
|
||||||
|
br.addError(new FieldError("createRecordRequest", "data", "must not be null"));
|
||||||
|
MethodParameter mp = new MethodParameter(
|
||||||
|
GlobalExceptionHandlerTest.class.getDeclaredMethod("sampleEndpoint", String.class), 0);
|
||||||
|
MethodArgumentNotValidException ex = new MethodArgumentNotValidException(mp, br);
|
||||||
|
|
||||||
|
ResponseEntity<ApiErrorResponse> resp = handler.onMethodArgumentNotValid(ex);
|
||||||
|
|
||||||
|
// Главное: 400, НЕ 500.
|
||||||
|
assertThat(resp.getStatusCode().value()).isEqualTo(400);
|
||||||
|
assertThat(resp.getBody()).isNotNull();
|
||||||
|
assertThat(resp.getBody().code()).isEqualTo("validation_failed");
|
||||||
|
assertThat(resp.getBody().fieldErrors())
|
||||||
|
.extracting(ApiErrorResponse.FieldError::path)
|
||||||
|
.containsExactlyInAnyOrder("businessKey", "data");
|
||||||
|
}
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user