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

Issue 691 hangout participant count #719

Merged
merged 25 commits into from Dec 17, 2017

Conversation

Projects
None yet
3 participants
@railsstudent
Collaborator

railsstudent commented Nov 18, 2017

Fixes #691.

Create a MongoDB collection app_stats with schema { hangout_id: string, participant_count: number }
A unique index should be created for hangout_id
// add unique index, db.app_stats.createIndex({ hangout_id, 1}, { unique: true });

Server publishes hangoutParticipants and allHangoutParticipants to return number of participants in a specific hangout room

In single study group.js, participant count is shown in a badge in 24/7 hangout tab
In hangout_frame.js, participant count is shown in a well below jitsu hangout. participant count is incremented when user loads jitsu and decremented when user leaves the page
In all study room.js and my study room.js, participant count of 24/7 hangout is listed for each study room; adjacent to laptop icon

lpatmo and others added some commits Oct 15, 2017

Issue-691. Show number of participants in 24/7 hangout
Issue-691. show number of hangout participants in all study group and my study group pages


issue-691.  Only decrement participant count when the logged in user joins the hangout room


Issue-691 Decrement participant count when value is positive

@lpatmo lpatmo added the In Review label Nov 18, 2017

@railsstudent

This comment has been minimized.

Show comment
Hide comment
@railsstudent

railsstudent Nov 18, 2017

Collaborator

all_study_groups_participant_count

my_study_groups_participant_counts
study_group_participant_count

Collaborator

railsstudent commented Nov 18, 2017

all_study_groups_participant_count

my_study_groups_participant_counts
study_group_participant_count

@railsstudent railsstudent requested a review from distalx Nov 18, 2017

@railsstudent

This comment has been minimized.

Show comment
Hide comment
@railsstudent

railsstudent Nov 19, 2017

Collaborator

@distalx Changes made to get rid of the session variable

Collaborator

railsstudent commented Nov 19, 2017

@distalx Changes made to get rid of the session variable

@distalx

Thanks for the PR! Here are some suggestions I hope you'll find helpful.

Show outdated Hide outdated server/hangouts/methods.js Outdated
Show outdated Hide outdated server/hangouts/publications.js Outdated
Show outdated Hide outdated server/hangouts/publications.js Outdated
@railsstudent

This comment has been minimized.

Show comment
Hide comment
@railsstudent

railsstudent Nov 24, 2017

Collaborator

@distalx Changes made

Collaborator

railsstudent commented Nov 24, 2017

@distalx Changes made

@railsstudent

This comment has been minimized.

Show comment
Hide comment
@railsstudent

railsstudent Nov 26, 2017

Collaborator

@distalx Please kindly review this PR when you have time. Thanks.

Collaborator

railsstudent commented Nov 26, 2017

@distalx Please kindly review this PR when you have time. Thanks.

@distalx

This comment has been minimized.

Show comment
Hide comment
@distalx

distalx Dec 3, 2017

Collaborator

Great work @railsstudent! 👏 . Love seeing it all come together! LGTM! 👍

I have made few changes.

  • storing a participant object inside participants list instead of just a count to avoid duplicates.
  • adding a user to participants list when user join the hangout by using a join here link.

_ Although there is a one scenario which is really hard to get around._

When a person join the hangout by using a join here link it will open the Jitsi in a new tab. We are not able to remove that person from the list of participants when that person close down the Jitsi tab. We are only able remove that person from the list of participants when that person either close down the CB app or log off from it.

So there are two possibilities, when user won’t get the accurate participants count.

  1. when a person close down the Jitsi tab and don’t close down the CB app or log off from it.
  2. when a person close down the CB app or log off from it and don’t close down the Jitsi tab.

Possible solutions !

  1. remove the join here link.
    it reduces the complexity of tracking user’s actions out side the CB app. we have added the join here link because sometimes Jitsi fails during loading process and ppl might have to refresh the page to start the load process again.

  2. run a cron job.
    as a last resort we could always implement a cron job which trigger at every couple of hours and reset/update the active participant count. it isn’t directly addresses the issue on hand but it reduces the possibility of stale data which is the by product of our current implementation.

cc @lpatmo , @sergeant-q.

Collaborator

distalx commented Dec 3, 2017

Great work @railsstudent! 👏 . Love seeing it all come together! LGTM! 👍

I have made few changes.

  • storing a participant object inside participants list instead of just a count to avoid duplicates.
  • adding a user to participants list when user join the hangout by using a join here link.

_ Although there is a one scenario which is really hard to get around._

When a person join the hangout by using a join here link it will open the Jitsi in a new tab. We are not able to remove that person from the list of participants when that person close down the Jitsi tab. We are only able remove that person from the list of participants when that person either close down the CB app or log off from it.

So there are two possibilities, when user won’t get the accurate participants count.

  1. when a person close down the Jitsi tab and don’t close down the CB app or log off from it.
  2. when a person close down the CB app or log off from it and don’t close down the Jitsi tab.

Possible solutions !

  1. remove the join here link.
    it reduces the complexity of tracking user’s actions out side the CB app. we have added the join here link because sometimes Jitsi fails during loading process and ppl might have to refresh the page to start the load process again.

  2. run a cron job.
    as a last resort we could always implement a cron job which trigger at every couple of hours and reset/update the active participant count. it isn’t directly addresses the issue on hand but it reduces the possibility of stale data which is the by product of our current implementation.

cc @lpatmo , @sergeant-q.

@distalx

distalx approved these changes Dec 3, 2017

Merge remote-tracking branch 'origin/issue-691-hangout-participant-co…
…unt' into issue-691-hangout-participant-count
@lpatmo

This comment has been minimized.

Show comment
Hide comment
@lpatmo

lpatmo Dec 10, 2017

Member

Thanks for the hard work here, @railsstudent and @distalx! 👏

When a person join the hangout by using a join here link it will open the Jitsi in a new tab. We are not able to remove that person from the list of participants when that person close down the Jitsi tab. We are only able remove that person from the list of participants when that person either close down the CB app or log off from it.

I'm not too worried about this scenario, actually. If they close the Jitsi tab but forget to close the CB app, they'll still technically be in the hangout, right? Of the two proposed solutions, I would lean towards the cron job instead of removing the "click here" link, since I think the user experience is more terrible if you can't get in to the hangout at all. Plus, the external link gives us easy access to the URL for the Jitsi room.

Shall I punt the cron job into a new issue -- and in the meantime, merge to staging?

Member

lpatmo commented Dec 10, 2017

Thanks for the hard work here, @railsstudent and @distalx! 👏

When a person join the hangout by using a join here link it will open the Jitsi in a new tab. We are not able to remove that person from the list of participants when that person close down the Jitsi tab. We are only able remove that person from the list of participants when that person either close down the CB app or log off from it.

I'm not too worried about this scenario, actually. If they close the Jitsi tab but forget to close the CB app, they'll still technically be in the hangout, right? Of the two proposed solutions, I would lean towards the cron job instead of removing the "click here" link, since I think the user experience is more terrible if you can't get in to the hangout at all. Plus, the external link gives us easy access to the URL for the Jitsi room.

Shall I punt the cron job into a new issue -- and in the meantime, merge to staging?

@lpatmo

This comment has been minimized.

Show comment
Hide comment
@lpatmo

lpatmo Dec 10, 2017

Member

One thought: in client/templates/study_groups/all_study_groups.html and in client/templates/study_groups/my_study_groups.html, do you think it might make sense to only show the laptop icon with the number count next to it if there is at least one person in the 24/7 room?

screen shot 2017-12-09 at 10 35 53 pm

I.e.

+ {{# if numParticipants _id }}<small><i class="fa fa-laptop fa-fw"></i>{{ numParticipants _id }}</small>{{/if}}
- <small><i class="fa fa-laptop fa-fw"></i>{{ numParticipants _id }}</small>

That way we won't have a bunch of laptop icons with (0) next to it, and it'd be easier to tell which study group rooms have active people at first glance. Thoughts welcome!

Member

lpatmo commented Dec 10, 2017

One thought: in client/templates/study_groups/all_study_groups.html and in client/templates/study_groups/my_study_groups.html, do you think it might make sense to only show the laptop icon with the number count next to it if there is at least one person in the 24/7 room?

screen shot 2017-12-09 at 10 35 53 pm

I.e.

+ {{# if numParticipants _id }}<small><i class="fa fa-laptop fa-fw"></i>{{ numParticipants _id }}</small>{{/if}}
- <small><i class="fa fa-laptop fa-fw"></i>{{ numParticipants _id }}</small>

That way we won't have a bunch of laptop icons with (0) next to it, and it'd be easier to tell which study group rooms have active people at first glance. Thoughts welcome!

@railsstudent

This comment has been minimized.

Show comment
Hide comment
@railsstudent

railsstudent Dec 12, 2017

Collaborator

@lpatmo Okay. I will work on it in the weekend.
I have slacked off since taking an interest in golang

Collaborator

railsstudent commented Dec 12, 2017

@lpatmo Okay. I will work on it in the weekend.
I have slacked off since taking an interest in golang

@lpatmo

This comment has been minimized.

Show comment
Hide comment
@lpatmo

lpatmo Dec 15, 2017

Member

@railsstudent If you feel like you've slacked off, then I've been... comatose? :P

The change is pretty straightforward -- it would be those two lines I pasted above for all_study_groups.html and my_study_groups.html, but before we make it let me know what you think of the idea! I'm not against showing it at all times either; am mostly worried that potentially having a lot of 0s will be off-putting.

Member

lpatmo commented Dec 15, 2017

@railsstudent If you feel like you've slacked off, then I've been... comatose? :P

The change is pretty straightforward -- it would be those two lines I pasted above for all_study_groups.html and my_study_groups.html, but before we make it let me know what you think of the idea! I'm not against showing it at all times either; am mostly worried that potentially having a lot of 0s will be off-putting.

@railsstudent

This comment has been minimized.

Show comment
Hide comment
@railsstudent

railsstudent Dec 15, 2017

Collaborator

@lpatmo I think it is a good idea to hide the laptop icon when no one is in 24/7 hangout.
Since the study group card is so small, it is prudent to convey only meaningful data.

Collaborator

railsstudent commented Dec 15, 2017

@lpatmo I think it is a good idea to hide the laptop icon when no one is in 24/7 hangout.
Since the study group card is so small, it is prudent to convey only meaningful data.

@lpatmo

This comment has been minimized.

Show comment
Hide comment
@lpatmo

lpatmo Dec 15, 2017

Member

Thanks!

Member

lpatmo commented Dec 15, 2017

Thanks!

@railsstudent

This comment has been minimized.

Show comment
Hide comment
@railsstudent

railsstudent Dec 17, 2017

Collaborator

@lpatmo Templates fixed. Please kindly review

Collaborator

railsstudent commented Dec 17, 2017

@lpatmo Templates fixed. Please kindly review

@lpatmo

lpatmo approved these changes Dec 17, 2017

Excellent! Thank you.

@lpatmo lpatmo merged commit b1b0b20 into codebuddies:staging Dec 17, 2017

@lpatmo lpatmo added closed and removed In Review labels Dec 17, 2017

lpatmo added a commit that referenced this pull request Dec 26, 2017

Release 0.9.90 (#754)
* Issue 691 hangout participant count (#719)

* fix for bug 690: admins should be able to access 24/7 hangout (#693) (#694)

* Issue-691. Show number of participants in 24/7 hangout




Issue-691. show number of hangout participants in all study group and my study group pages


issue-691.  Only decrement participant count when the logged in user joins the hangout room


Issue-691 Decrement participant count when value is positive

* Remove unique index comment

* fix 'participants_count' of undefined console err

* Get rid of session variable

* Move server-side logic to new files

* Enable seed data

* minor improvements

* Show laptop icon when people join 24/7 hangout

* Fix account deletion. (#745)

* fix typo

* Schedule hangout default studygroup name (#743)

* Default study group name 

when schedule a hangout in single study group
duplicate hangout in single study group

* Default study group name 

value is not displayed in dropdown input area unless trigger('change') is fired

* Fixed error

* Issue 667 create slack page (#744)

* Create new Slack page

* Add helpful links

* Minor content edits!

Note: I removed the slackin plugin in favor of a direct link because I noticed some positioning issues with the form that appears after a user clicks on the plugin; it's sometimes out of reach, and will make the user think that nothing happened.

* Show study group name in disabled input box in modal (#748)

* Refactoring 1 (#752)

* fix for bug 690: admins should be able to access 24/7 hangout (#693) (#694)

* refactor (routes) remove subs from router level

* refactor (hangout_logged_out) make use of new syntax

* refactor (hangouts) - remove dead code, restructure single hangout view

lpatmo added a commit that referenced this pull request Jan 1, 2018

New Year's Eve release (#758)
* Issue 691 hangout participant count (#719)

* fix for bug 690: admins should be able to access 24/7 hangout (#693) (#694)

* Issue-691. Show number of participants in 24/7 hangout




Issue-691. show number of hangout participants in all study group and my study group pages


issue-691.  Only decrement participant count when the logged in user joins the hangout room


Issue-691 Decrement participant count when value is positive

* Remove unique index comment

* fix 'participants_count' of undefined console err

* Get rid of session variable

* Move server-side logic to new files

* Enable seed data

* minor improvements

* Show laptop icon when people join 24/7 hangout

* Fix account deletion. (#745)

* fix typo

* Schedule hangout default studygroup name (#743)

* Default study group name 

when schedule a hangout in single study group
duplicate hangout in single study group

* Default study group name 

value is not displayed in dropdown input area unless trigger('change') is fired

* Fixed error

* Issue 667 create slack page (#744)

* Create new Slack page

* Add helpful links

* Minor content edits!

Note: I removed the slackin plugin in favor of a direct link because I noticed some positioning issues with the form that appears after a user clicks on the plugin; it's sometimes out of reach, and will make the user think that nothing happened.

* Show study group name in disabled input box in modal (#748)

* Refactoring 1 (#752)

* fix for bug 690: admins should be able to access 24/7 hangout (#693) (#694)

* refactor (routes) remove subs from router level

* refactor (hangout_logged_out) make use of new syntax

* refactor (hangouts) - remove dead code, restructure single hangout view

* Add 'add to google calendar' button that opens up link with event details from hangout, including meet.jit.si location (#755)

* Display fallback image if image url of contributor is broken (#756)

* Issue 547 redesign of /hangouts page (#757)

* redesigned header area

* hangouts area now uses flexbox + big redesign of cards area

* adjust column display on profile page

* completed hangout cards do not need to be very tall; also, hide meta content on hover

* add popover descriptions for silent/collaboration/teaching icon keys

* grammar correction

* update hangout-faq a bit more, and give it its own route

* capitalization fixes

* adjust hover behavior of btn-block

* fix up hangout cards in study groups

* update faq content
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment