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

Include registeredOn date in pubSub user creation messages to replace loginTime creation in Meteor #11413

Merged
merged 4 commits into from Feb 17, 2021

Conversation

gustavotrott
Copy link
Collaborator

  • Makes Akka include registeredOn date on pubSub messages: UserRegisteredRespMsg, GuestsWaitingForApprovalEvtMsg and ValidateAuthTokenRespMsg.

  • Makes Meteor use the registeredOn date received from Akka to define loginTime instead of create a new date using Date.now().

…created on Akka. Makes Akka send the registeredOn date throught PubSub messages: UserRegisteredRespMsg, GuestsWaitingForApprovalEvtMsg and ValidateAuthTokenRespMsg.
@pedrobmarin
Copy link
Collaborator

Hi @gustavotrott ,

Does this change is related to an issue? And why is registeredOn value a string? Couldn't be an epoch from start?

@gustavotrott
Copy link
Collaborator Author

Hi @gustavotrott ,

Does this change is related to an issue? And why is registeredOn value a string? Couldn't be an epoch from start?

Hey @pedrobmarin, thanks for your review.

There is no github issue for this change.
It's a refactor to avoid manipulating data outside of akka ( for consistency and better component roles separation ).

And you are right about the registeredOn type. I was thinking it should be human readable. But it seems the loginTime in Meteor is used just for comparison conditions. So I made the changes to keep the original type (timestamp epoch instead of ISO ).

@pedrobmarin
Copy link
Collaborator

Hi @gustavotrott , looks better already.

I have one last concern regarding the registeredOn for waiting users. Is this value used for anything besides logging things at Meteor? I'm asking because waiting users already have a parallel routine at bbb-web to check if a waiting user should be discarded or not.

@gustavotrott
Copy link
Collaborator Author

gustavotrott commented Feb 17, 2021

Hi @gustavotrott , looks better already.

I have one last concern regarding the registeredOn for waiting users. Is this value used for anything besides logging things at Meteor? I'm asking because waiting users already have a parallel routine at bbb-web to check if a waiting user should be discarded or not.

https://github.com/bigbluebutton/bigbluebutton/blob/develop/bigbluebutton-html5/imports/ui/components/waiting-users/alert/component.jsx

The loginTime of the guest is used as condition to show the pendingGuestAlert or not.
I think this is the only place where this is used (for guest users).

@pedrobmarin
Copy link
Collaborator

Hi @gustavotrott , looks better already.
I have one last concern regarding the registeredOn for waiting users. Is this value used for anything besides logging things at Meteor? I'm asking because waiting users already have a parallel routine at bbb-web to check if a waiting user should be discarded or not.

https://github.com/bigbluebutton/bigbluebutton/blob/develop/bigbluebutton-html5/imports/ui/components/waiting-users/alert/component.jsx

The loginTime of the guest is used as condition to show the pendingGuestAlert or not.
I think this is the only place where this is used (for guest users).

Hmmm, I think we're fine then. Thanks @gustavotrott

@pedrobmarin pedrobmarin self-requested a review February 17, 2021 21:29
@antobinary antobinary merged commit 49cdf9f into bigbluebutton:develop Feb 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants