Skip to content

Added more bus api events #110

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

Merged
merged 4 commits into from
Aug 24, 2018

Conversation

architectt1
Copy link
Contributor

@architectt1 architectt1 commented Jul 14, 2018

All events are sent when needed.

src/constants.js Outdated
@@ -61,6 +61,12 @@ export const BUS_API_EVENT = {
PROJECT_CANCELED: 'notifications.connect.project.canceled',
PROJECT_ACTIVE: 'notifications.connect.project.active',

PROJECT_PHASE_TRANSITION_ACTIVE: 'project.phase.transition.active',
Copy link

@vikasrohit vikasrohit Aug 23, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@architectt1 @gondzo I think we need to keep notifications.connect as prefix for all new events as well.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agreed, I'll push the change shortly

@vikasrohit
Copy link

@gondzo Although, this code was not made in this PR, but it seems this condition is wrong as per inline doc and rationale of throwing the event mentioned afterwards. Could or should we fix it to _.isEqual(originalWithouDetails, updatedWithouDetails)?

@gondzo
Copy link
Collaborator

gondzo commented Aug 23, 2018

looks like a bug. agreed on the isEqual change.

@vikasrohit
Copy link

Could you please do that change in this same PR @gondzo? I haven't checked out the the architectt1's origin yet. Also, seems like their is conflict as well, which needed to be resolved before merging.

After these changes, I think we are safe to merge this PR into dev.

@vikasrohit
Copy link

vikasrohit commented Aug 24, 2018

@gondzo, resolved conflict, merging the PR to reduce the number of things in flight.

@vikasrohit vikasrohit merged commit ba14b78 into topcoder-platform:dev Aug 24, 2018
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