Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Notification system using socket.io, remove user/admin, case-insensitive functionality #135

Merged
merged 4 commits into from
Jun 22, 2020

Conversation

Rupeshiya
Copy link
Member

@Rupeshiya Rupeshiya commented Jun 14, 2020


name: Pull Request template
about: Describe the PR.
title: ''
labels: ''
assignees: ''


Problem

Github Issue Number: #134 #136
As of now, there is no backend for the notification system, so users of the platform would not be able to get any type of real-time notifications. Like if any event is going to be hosted by the org, or org is kept under maintenance, or user A follows user B, etc. So in all those cases, the user will not be able to get any notifications
_

Solution of problem

Implement backend for notifications system using socket.io and node.js and store that in DB for first-time page load.

Type of Change

[ ] Bug fix
[✓] New Feature
[ ] Development of UI/UX prototypes
[✓] Small refactor
[ ] Change in Documentation

Checklist

[✓] My code follows the same style as the codebase
[ ] My Code change requires a change in documentation
[ ] I have updated the Readme accordingly
[✓] I made PR against development branch
[✓] I have run the test cases locally and it's passing.
[✓] I have squashed my commits

@devesh-verma
Copy link
Member

@Rupeshiya conflicts

@Rupeshiya
Copy link
Member Author

@devesh-verma One test case is failing as soket.io is listening on another PORT address, so jest giving open handle error.

@Rupeshiya Rupeshiya force-pushed the notification branch 2 times, most recently from 1ac8cad to f68be0a Compare June 18, 2020 07:28
@Rupeshiya Rupeshiya changed the title Notification system using socket.io Notification system using socket.io, remove user/admin, case-insensitive functionality Jun 18, 2020
app.js Outdated Show resolved Hide resolved
app/controllers/event.js Show resolved Hide resolved
app/controllers/event.js Show resolved Hide resolved
app/controllers/event.js Show resolved Hide resolved
app/controllers/notification.js Show resolved Hide resolved
@vaibhavdaren
Copy link
Member

can you explain this
await new Promise((resolve) => setTimeout(() => resolve(), 500))

@Rupeshiya
Copy link
Member Author

Rupeshiya commented Jun 18, 2020

can you explain this
await new Promise((resolve) => setTimeout(() => resolve(), 500))

@vaibhavdaren during the shutdown of superset, somewhere the thread is not waiting for an async system operation to finish. Thus, the operation is delegated to the system but the execution is not blocked or does not wait for the async operation to finish. In the meantime, the test case execution finishes, jest is shutting down and complains about an open handle. So this is making the jest to wait for some time to finish the execution and disable the open handle issue, during npm run test

@vaibhavdaren
Copy link
Member

can you explain this
await new Promise((resolve) => setTimeout(() => resolve(), 500))

@vaibhavdaren during the shutdown of superset, somewhere the thread is not waiting for an async system operation to finish. Thus, the operation is delegated to the system but the execution is not blocked or does not wait for the async operation to finish. In the meantime, the test case execution finishes, jest is shutting down and complains about an open handle. So this is making the jest to wait for some time to finish the execution and disable the open handle issue, during npm run test

So , jest stops execution before the async call finishes?

@Rupeshiya
Copy link
Member Author

can you explain this
await new Promise((resolve) => setTimeout(() => resolve(), 500))

@vaibhavdaren during the shutdown of superset, somewhere the thread is not waiting for an async system operation to finish. Thus, the operation is delegated to the system but the execution is not blocked or does not wait for the async operation to finish. In the meantime, the test case execution finishes, jest is shutting down and complains about an open handle. So this is making the jest to wait for some time to finish the execution and disable the open handle issue, during npm run test

So , jest stops execution before the async call finishes?

yes, for some cases. And creating open handle issue.

@vaibhavdaren
Copy link
Member

can you explain this
await new Promise((resolve) => setTimeout(() => resolve(), 500))

@vaibhavdaren during the shutdown of superset, somewhere the thread is not waiting for an async system operation to finish. Thus, the operation is delegated to the system but the execution is not blocked or does not wait for the async operation to finish. In the meantime, the test case execution finishes, jest is shutting down and complains about an open handle. So this is making the jest to wait for some time to finish the execution and disable the open handle issue, during npm run test

So , jest stops execution before the async call finishes?

yes, for some cases. And creating open handle issue.

@Rupeshiya
Copy link
Member Author

@vaibhavdaren @devesh-verma Done all the requested changes!

@vaibhavdaren vaibhavdaren merged commit cf9a8ab into codeuino:development Jun 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants