Feature/add entry patch endpoint #475
Conversation
| assetsManifestContent = response.data; | ||
|
|
||
| } catch (error) { | ||
| throw new AssetsManifestError(`"assetsDiscoveryUrl" ${assetsManifestUrl} is not available. Check the url via browser manually.`); |
There was a problem hiding this comment.
looks like it hides original error. Can we propagate it?
There was a problem hiding this comment.
I think in that case we should not distinguish an error. Any network error shows us we have to debug accessibility of asset
There was a problem hiding this comment.
I understand that, but if we do not have original error in error that is thrown and do not have it in logs, it would be hard to debug
| try { | ||
| return await assetsManifestContentValidator.validateAsync(assetsManifestContent); | ||
| } catch(error) { | ||
| throw new AssetsManifestError('"spaBundle" must be specified in the manifest file from provided "assetsDiscoveryUrl" if it was not specified manually'); |
There was a problem hiding this comment.
I'd propagate original Joi error
There was a problem hiding this comment.
Thank you. Joi error will be more reliable in this case because validation object can be extended.
I have added prettier for registry. It is running on pre-commit hook. |
| } else if (identifier.startsWith(this.resourceIdentifiers.APP)) { | ||
| const entityIdentifier = this.getEntityIdentifier(identifier, this.resourceIdentifiers.APP); | ||
| return this.getAppInstance(entityIdentifier); | ||
| } else { |
There was a problem hiding this comment.
no need to have else condition here
There was a problem hiding this comment.
Thanks. Changed.
| @@ -0,0 +1,3 @@ | |||
| export class FqrnError extends Error { | |||
There was a problem hiding this comment.
Maybe it is better to rename to FullyQualifiedResourceNameError ?
| try { | ||
| results = await entryService.patch(params, { user: request.user }); | ||
| } catch (error) { | ||
| if (error instanceof NotFoundFqrnError) { |
There was a problem hiding this comment.
if (error instanceof NotFoundFqrnError || error instanceof ValidationFqrnError) {
return response.status(error.code).send(error.message);
}
if (error instanceof Joi.ValidationError) {
return response.status(422).send(joiErrorToResponse(error));
}
if (error instanceof AssetsManifestError) {
return response.status(error.code).send(error.message);
}
There was a problem hiding this comment.
I think it make sence to optimize catching approach a bit later since all routes are supported single approach.
In ideal case all errors should be presented in single format that could be parsed in the same manner.
| try { | ||
| results = await sharedLibEntry.patch(sharedLib, { user: req.user }); | ||
| } catch (error) { | ||
| if (error instanceof NotFoundFqrnError) { |
There was a problem hiding this comment.
no need in else if conditions here
add
/api/v1/entries/:fqrnendpoint.It allows to patch fully qualified resource names (SharedLibs and Applications).
Motivation for it is providing single resource to update common properties for general entities.
Use case is patching localisation manifest in single way for SharedLibs and Applications