-
Notifications
You must be signed in to change notification settings - Fork 38
Conversation
Vivek-Lahole
commented
May 7, 2024
•
edited
Loading
edited
- To see the specific tasks where the Asana app for GitHub is being used, see below:
- https://app.asana.com/0/0/1207218657171609
- https://app.asana.com/0/0/1207143924905573
- https://app.asana.com/0/0/1207216946001611
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This method should be redisigned:
- Rule of thumb is we need to make all of our api's reusable for all apps in the back end. We need to avoid custom api's as much as possible
For example in this case our main goal is to get data of an app stored in the database whether its the webconnector or not!
So with that in mind, we have to rename this to "get-app" which takes in a the name of the app and returns the data of the app from the backend.
For example calling the endpoint with "webconnector" will return links, with another app will return custom data for that app.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same applies here! How can we make this api general across the board for all apps to add custom data payloads?
} | ||
); | ||
|
||
route.post("/savelink", middlewares.wrap(require("./webConnector").default)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as this one! Lets avoid custom api's for one app, its not scalable.
packages/apps/webconnector/README.md
Outdated
@@ -0,0 +1,25 @@ | |||
# 📝 Confluence Integration | |||
|
|||
Integrate Ocular with Confluence to enable fetching Confluence spaces and pages within Ocular. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change Confluence to WebConnector?
return { | ||
name: AppNameDefinitions.WEBCONNECTOR, | ||
logo: "/asana.svg", | ||
description: "Web Connector", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use Chat Gpt To Generate A Longer Description for WebConnector
description: "Web Connector", | |
description: "Web Connector", |
|
||
await this.oauthService_.update(oauth.id, { last_sync: new Date() }); | ||
|
||
// const current_org = await this.organisationService.listInstalledApps(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Create an issue in Asana to add back this logic?
batchJob.context?.link_id as string | ||
); | ||
stream.on("data", (documents) => { | ||
this.eventBusService_.emit(INDEX_DOCUMENT_EVENT, documents); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should send docs to Kafka here! Check other apps such as GDrive!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should avoid custom api's for custom apps. This logic should be handled by the /apps/retrieve-app endpoint!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Design Feedback: The Api files should hold minimal logic so this logic should be moved into the services. Basically we should be able to use the endpoint to update other apps in the future as well.
Lets redesign this and create a new method on the organization service called updateInstalledAppMetadata - this will allow the metadata of any app to be updated by this end point.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Delete this Migration, it seems to be outdated!