Chore: Attachment Definitions and UiKitDefinitions #22354
Conversation
sampaiodiego
left a comment
sampaiodiego
left a comment
There was a problem hiding this comment.
additionally to the comments I added on the code, I think very weird having duplicated items on some file paths, like for example definition/IMessage/IMessage.ts and definition/IMessage/MessageAttachment/MessageAttachment.ts
what if those duplicates are moved to the index.ts files? like definition/IMessage/index.ts and definition/IMessage/MessageAttachment/index.ts respectively. or maybe moving them one folder up.. wdyt?
app/apps/server/bridges/uploads.ts
Outdated
|
|
||
| const getUploadDetails = (details: IUploadDetails): Partial<IUploadDetails> => { | ||
| if (details.visitorToken) { | ||
| // eslint-disable-next-line @typescript-eslint/no-unused-vars |
There was a problem hiding this comment.
should we use ignoreRestSiblings:true instead of having to disable the ESLint rule? https://eslint.org/docs/rules/no-unused-vars#ignorerestsiblings
| const businessHourToReturn = { ...businessHourData }; | ||
| delete businessHourData.timezoneName; | ||
| delete businessHourData.departmentsToApplyBusinessHour; | ||
| delete (businessHourData as any).timezoneName; |
There was a problem hiding this comment.
I think we should fix this with a better solution. Typings are not well used here 🤔
app/integrations/lib/rocketchat.js
Outdated
| @@ -1,4 +1,8 @@ | |||
| import { RocketChatIntegrationHandler } from '../server/lib/triggerHandler'; | |||
There was a problem hiding this comment.
this file is imported by the client bundle, so doing this doesn't work..
even if this worked, it would fail because ../server/lib/triggerHandler has a dependency on Node's vm , which would fail when trying to use on client.
this is a pattern we found during many attempts, the index.js usage is recommended for import, and the file with the full name is for help developers to open the right file quickly, so they dont need to guess if the right file is a index or a named file. |
… into chore/attachments
… into chore/attachments
Proposed changes (including videos or screenshots)
Issue(s)
Steps to test or reproduce
Further comments