-
Notifications
You must be signed in to change notification settings - Fork 217
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
added feature of terminating when the link is deleted issue number #352 issue has been solved #375
added feature of terminating when the link is deleted issue number #352 issue has been solved #375
Conversation
@muke1908 i have installed many libraries i faced difficulty in running the application at starting so it showing there is conflicts in package.json you can omit or remove those unwanted later please merge it as soon as possible . |
@@ -1,7 +1,7 @@ | |||
import { v4 as uuidv4 } from 'uuid'; | |||
import { generatePIN } from './pin'; | |||
|
|||
const { CHAT_LINK_DOMAIN } = process.env; | |||
const CHAT_LINK_DOMAIN = process.env.CHAT_LINK_DOMAIN || 'http://localhost:3000'; |
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.
Please remove this fallback.
const dbName = process.env.MONGO_DB_NAME; | ||
// added local db address if you didnt create a .env file | ||
const uri = process.env.MONGO_URI || 'mongodb://localhost:27017'; | ||
const dbName = process.env.MONGO_DB_NAME || 'realtimechatting'; |
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.
Please remove the arbitrary defaults. Let it be set explicitly in .env
const uri = process.env.MONGO_URI || 'mongodb://localhost:27017'; | ||
const dbName = process.env.MONGO_DB_NAME || 'realtimechatting'; | ||
|
||
console.log('uri', uri); |
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.
unnecessary console.log
|
||
|
||
// Helper function to throttle a function call | ||
function throttle<T extends (...args: any[]) => void>(func: T, limit: number): (...args: Parameters<T>) => void { |
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.
can be moved to ./utils.
try { | ||
// FOR PRODUCTION MODE URL SHOULD BE https://chat-e2ee-2.azurewebsites.net/api/chat-link/status/${channelID} | ||
// FOR RUNING LOCALLY URL SHOULD BE http://localhost:3001/api/chat-link/status/${channelID} | ||
const URL = `http://localhost:3001/api/chat-link/status/${channelID}` |
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.
urls should not be hard-coded. Also UI layer does not need to know anything about the server. That's why we have the service.
You can expose another method in service
, and use that here.
@muke1908 can please guide me what and all need to make change in order to create new method ? |
I think you already did it earlier here #352 (comment) |
yes i did but the error not found so first i created method in sdk.ts and added that in types.ts after that run build command then also it didnt reflect on my frontend |
Did you try this #352 (comment) |
@muke1908 after trying only i purposed this method . i tried for more then 2 days but it showing same error method is not found . please mention a detailed description how to create new method in service folder and how backend folder interact with service folder please |
Once it's done, if you make change in |
closing as redundant |
Hey, thanks for assigning this issue to me!
I successfully implemented this feature. Most of the files show changes due to the addition of a cursor at the end. In the frontend, we used a useEffect hook to validate whether the link is valid or not. We also applied a throttling concept to check this condition every second. You can modify the interval to your preference, but 1 second is optimal for performance.
Things you need to change for production mode:
You need to update the URL used for sending the request in the file client/src/pages/messaging/index.tsx at line 270. For production, the URL should be https://chat-e2ee-2.azurewebsites.net/api/chat-link/status/${channelID}. Make sure you use backticks for this. For now, I’ve used localhost for running it locally.
Please refer to the commits for a better understanding. You can also use an .env file and access the URL via process.env.BACKEND_URL, but for now, I’ve added the URL directly to reduce confusion.
I have also tested the app locally. Only one test failed due to the absence of an absolute link, which we declared as undefined. This is causing the following error:
The detailed failed test case can be found in the file backend/api/chatLink/utils/link.test.ts at line 19. The error is caused by absoluteLink: undefined, which is why the test failed.
i think this is not matter that much . and console when we start the app looks like
I implemented the feature where the throttling function runs every second and renders the component when linkActive becomes invalid. After 2 seconds, it will navigate to the / route (home route). You can adjust this timing based on your preference.
I chose 2 seconds as an optimal delay to inform the user that the link is invalid. Using a longer delay might send more requests to the backend, leading to performance issues. On the other hand, if we use a 1-second delay, there might not be enough time for the user to read the message. Therefore, I think 2 seconds strikes a good balance, but you can modify it according to your needs.
encrypt.mp4
now Related to delete link feature #352 issue has been solved
fell free to as anything , to ask regarding this issue impentation . Thank you for guiding at every step @muke1908