Refactor getEntry endpoint #310
Conversation
stepanenkoxx
force-pushed
the
DLAPI-248-refactor-get-entry-endpoint
branch
from
a3d0c33 to
09cb85c
Compare
stepanenkoxx
force-pushed
the
DLAPI-248-refactor-get-entry-endpoint
branch
2 times, most recently
from
97c7157 to
ac582a5
Compare
| throw new AppError(US_ERRORS.NOT_EXIST_ENTRY, { | ||
| code: US_ERRORS.NOT_EXIST_ENTRY, | ||
| }); |
There was a problem hiding this comment.
In this file I just raised the error above to reduce code nesting
src/routes.ts
Outdated
| getEntryV2: makeRoute({ | ||
| route: 'GET /v2/entries/:entryId', | ||
| handler: entries.getEntryV2Controller, | ||
| }), | ||
| privateGetEntry: makeRoute({ | ||
| route: 'GET /private/entries/:entryId', | ||
| handler: entries.getEntry, | ||
| authPolicy: AuthPolicy.disabled, | ||
| private: true, | ||
| }), | ||
| privateGetEntryV2: makeRoute({ | ||
| route: 'GET /v2/private/entries/:entryId', | ||
| handler: entries.getEntryV2Controller, | ||
| authPolicy: AuthPolicy.disabled, | ||
| private: true, | ||
| }), |
There was a problem hiding this comment.
This endpoint has the same contract as the old one, except for includeFavorite param
It seems we can just start sending includeFavorite where necessary in the UI and simply replace the controllers in the old endpoints, i guess we can even switch controllers with flag in US
There was a problem hiding this comment.
"i guess we can even switch controllers with flag in US" – good idea
|
|
||
| let userLoginPromise; | ||
| if (includeFavorite) { | ||
| userLoginPromise = user.login ? user.login : getEntryResolveUserLogin({ctx}); |
There was a problem hiding this comment.
It works correctly, but is it possible to store only promises in userLoginPromise?
user.login ? Promise.resolve(user.login) : getEntryResolveUserLogin({ctx})
| }; | ||
| } | ||
|
|
||
| case US_ERRORS.OPERATION_TIMEOUT: { |
There was a problem hiding this comment.
Operation is used for existing entity, can we use Action or Request here?
src/routes.ts
Outdated
| getEntryV2: makeRoute({ | ||
| route: 'GET /v2/entries/:entryId', | ||
| handler: entries.getEntryV2Controller, | ||
| }), | ||
| privateGetEntry: makeRoute({ | ||
| route: 'GET /private/entries/:entryId', | ||
| handler: entries.getEntry, | ||
| authPolicy: AuthPolicy.disabled, | ||
| private: true, | ||
| }), | ||
| privateGetEntryV2: makeRoute({ | ||
| route: 'GET /v2/private/entries/:entryId', | ||
| handler: entries.getEntryV2Controller, | ||
| authPolicy: AuthPolicy.disabled, | ||
| private: true, | ||
| }), |
There was a problem hiding this comment.
"i guess we can even switch controllers with flag in US" – good idea
| .where((builder) => { | ||
| builder.where({ | ||
| [`${Entry.tableName}.entryId`]: entryId, | ||
| [`${Entry.tableName}.isDeleted`]: false, |
There was a problem hiding this comment.
It was the same in the old method, but I think we need to add tenantId here, it's checked below but I don't see the point in not checking it right in the select. Maybe @jhoncool knows the reason why we shouldn't do this.
There was a problem hiding this comment.
I don't know but I think it's okay to use it here
| builder.andWhere({mirrored: true}); | ||
| } | ||
| }) | ||
| .withGraphJoined(`[${graphRelations.join(', ')}]`) |
There was a problem hiding this comment.
As we discussed, we no longer use withGraphJoined, need to create a new presentation (example #318).
| } else if (branch === 'saved') { | ||
| graphRelations.push('savedRevision(revisionModifier)'); | ||
| } else { | ||
| graphRelations.push('publishedRevision(revisionModifier)'); |
There was a problem hiding this comment.
Here should be a fallback to saved revision if the published one does not exist.
There was a problem hiding this comment.
I was wrong, everything works correctly, because we have branch = 'saved' by default
| !isPrivateRoute && !onlyPublic && !onlyMirrored && !isEmbedding; | ||
|
|
||
| if (checkWorkbookEnabled) { | ||
| iamPermissions = await checkWorkbookEntry({ |
There was a problem hiding this comment.
Should we wrap the checkWorkbookEntry function with withTimeout?
stepanenkoxx
force-pushed
the
DLAPI-248-refactor-get-entry-endpoint
branch
from
3d06410 to
3f117fe
Compare
3 participants
No description provided.