1.0.0 #1
1.0.0 #1
Conversation
bilgeyucel
requested review from
mucahit,
burakguneli,
bedirhankaradogan,
edizcelik and
brkn
lib/ExceptionTransformer.js
Outdated
| @@ -1,6 +1,7 @@ | |||
| "use strict"; | |||
| Object.defineProperty(exports, "__esModule", { value: true }); | |||
| const mapUtils_1 = require("./utils/mapUtils"); | |||
| const errorConstants_1 = require("./utils/errorConstants"); | |||
There was a problem hiding this comment.
Why do we need to add a number suffix?
There was a problem hiding this comment.
This is the output file. You can just review the ts files
src/ExceptionTransformer.ts
Outdated
| ) { | ||
| if (typeof errorDetail.non_field_errors[0] === "string") { | ||
| message = errorDetail.non_field_errors[0]; | ||
| } else { |
There was a problem hiding this comment.
I will open an issue about this to extract a logical error message from errorDetail.non_field_errors[0] if it exists. So in your first if block, the condition should just check if errorDetail.non_field_errors[0] is null or undefined and then we will recursively look for a value. We can publish without making this enhancement but it will be a good addition.
src/ExceptionTransformer.ts
Outdated
|
|
||
| if (unknownErrorKeys.length) { | ||
| message = `${unknownErrorKeys[0]}: ${ | ||
| errorDetail[unknownErrorKeys[0]] |
There was a problem hiding this comment.
That same recursive function should be used here that extract a message from unknownErrorKeys[0] because it may not be a simple string. The function would take unknownErrorKeys[0] as argument and check and extract the first logical error message.
|
Please upgrade the dependency versions to the latest here as well |
* Pass `genericErrorMessage` to class object as a parameter * Add `getErrorDetail` util function * Update `compilerOptions` * Update version to `1.0.0`
|
Thanks for the review. Changes are made @edizcelik @bedirhankaradogan |
* A long string created with nested key names can be provided as `fieldName` to `getFieldError` function. * errorDetail can be `string[]`, `ExceptionDetail` or `ExceptionDetail[]`. Consider this in `generateErrorMessage` function * `genericErrorMessage` is required to initialize `ExceptionTransformer`. `onExceptonExceptioio` is optional. * Add new util functions to `errorUtils` * Make more strict type checkings * Add explanatory informations as comment Related issue: #2
…r` returns string[] or undefined * Improve `deleteProperty` and `getValueFromPath` functions by giving them type * Change `ExceptionDetail` type * Make sure `generateSpecificFieldError` returns string[] or undefined * Change some namings
|
Thanks for the review @edizcelik , changes are made. |
| } | ||
| this.genericErrorMessage = genericErrorMessage; |
There was a problem hiding this comment.
I think it would be useful for some certain cases, such as changing language of an app, that I am able to modify genericErrorMessage after initialization
There was a problem hiding this comment.
Added a new function: changeGenericErrorMessage to change genericErrorMessage
src/ExceptionTransformer.ts
Outdated
| shouldDisplayFallbackMessage = true; | ||
| } | ||
|
|
||
| if (shouldDisplayFallbackMessage) { |
There was a problem hiding this comment.
Instead of having a variable for this, would it work to just check against message ?
| if (shouldDisplayFallbackMessage) { | |
| if (!message) { |
It seems this would work because it means that we couldn't generate a message from the provided errorDetail
There was a problem hiding this comment.
It is not possible to change the check like this since when all the errors are known, message will be "" again. This would create confusion.
But we can change if (shouldDisplayFallbackMessage) to else since now there is no other possibility.
* Add `removeKnownKeysFromErrorDetail` and `generateFieldErrorFromErrorDetail` * Add `changeGenericErrorMessage` to change generic error message after initializing `exceptionTransformer` * Make necessary changes for npm upload
generateSpesificFieldErrorandgenerateErrorMessagefunctions