Implement a refresh for presence

[plasne]

Closes #264

PR Type

• Feature

Description of the changes

This PR creates a PresenceService that allows PresenceAwareComponents (currently only MgtPerson) to register a userID for presence changes and then receive notification on those changes. This design ensures that all registered components get updates at the same time. The original method of setting presence is replaced by this method provided "show-presence" and "refresh-presence" are both true.

There is configuration that allows for setting the initial and refresh times in milliseconds.

The PR includes unit tests covering the new functionality.

PR checklist

• Project builds (yarn build) and changes have been tested in at least two supported browsers (Edge + non-Chromium based browser)
• All public APIs (classes, methods, etc) have been documented following the jsdoc syntax
• Stories have been added and existing stories have been tested
• Added appropriate documentation. Docs PR:

• License header has been added to all new source files (yarn setLicense)
• Contains NO breaking changes

Other information

This should not be a breaking change. If iteration is not changed, there is no change to presence.

[sebastienlevert]

Thanks for the PR @plasne. I'm wondering if we shouldn't do the work upstream, in the mgt-person component instead. This would allow reusing this behavior for all types of scenarios. We've got some requests on this in the past. Now the question is how can we ensure all person components sharing the same person could be in sync...

[plasne]

Thanks for the PR @plasne. I'm wondering if we shouldn't do the work upstream, in the mgt-person component instead. This would allow reusing this behavior for all types of scenarios. We've got some requests on this in the past. Now the question is how can we ensure all person components sharing the same person could be in sync...

The issue with doing it in the mgt-person component is that you could get out-of-sync (a person showing "away" from one component and "available" in another). Thoughts?

[sebastienlevert]

We had a good conversation about this feature and here is what we think could deliver the value directly at the component level with the help of some global service that would act as an "orchestrator".

• A service (PresenceService) is available globally. This service and be used to register a new user presence request and emits updated user presence. A dictionnary of userId and count would be kept global to ensure its state.
• The PresenceService is the only gateway to presence in all of MGT. A mutex is used to ensure only a single request to update the presence, all the rest automatically hits the cache.
• The configurable timeout is global and will be shared by all the person components.
• The person components will individually register their need for presence to the PresenceService and subscribe to updated statuses. This would only be true if show-presence is set and refresh-presence is true (which by default would be true).

Open questions:

• What about multiple tabs? What happens with the cache?

[plasne]

We had a good conversation about this feature and here is what we think could deliver the value directly at the component level with the help of some global service that would act as an "orchestrator".

• A service (PresenceService) is available globally. This service and be used to register a new user presence request and emits updated user presence. A dictionnary of userId and count would be kept global to ensure its state.
• The PresenceService is the only gateway to presence in all of MGT. A mutex is used to ensure only a single request to update the presence, all the rest automatically hits the cache.
• The configurable timeout is global and will be shared by all the person components.
• The person components will individually register their need for presence to the PresenceService and subscribe to updated statuses. This would only be true if show-presence is set and refresh-presence is true (which by default would be true).

Open questions:

• What about multiple tabs? What happens with the cache?

OK, that all makes sense and I can rework it:

1. Is the Presence service making a call to the Graph API or creating a subscription to get presence updates?

• If it is using a subscription, it seems like we will have to keep track of the subscription so we can remove and recreate every time there is a new person that is being tracked.

2. Do we unregister or expire registrations? For example, if we click on a group chat that has 5 people, the person components all register. Then we click on a 1:1 chat, we may not need those other presence notifications any longer.
3. If we use IndexedDB for the cache instead of a dictionary, we could save some Graph calls. If we are using the subscription, we would need to see those notifications on both connections.

When I return after Christmas, I can put some of this together and maybe we can chat about some of those implementation details.

[plasne]

@sebastienlevert I have changed the implementation to use a PresenceService. Can you review again and let me know what else you might want different?

[gavinbarron]

This is great work, thank you so much for this.

There's a few things that need to be changed but the core here is solid.

One thing that I didn't add a comment for is that yarn build needs to be run to update the generated React wrapper component and it's available props.

[plasne]

@sebastienlevert and @gavinbarron, please review again. I have made all the requested changes. Thanks for the informative and timely review!

[sebastienlevert]

Functionnaly, this is amazing! Works great, very snappy, visually appealing and only the presence icon changes so this is a BIG win!

Based on the limits of this API, I'm wondering if we should be test a little bit more.

• Maximum of 650 user IDs are supported per API request.
• The maximum request rate of this API is 1500 API requests in a 30 second period, per application per tenant.

That means we couldn't make more than 1500 API requests every 30 seconds. So first, I'd align our polling to be 30 seconds to ensure we don't fall into these limits. This also means that a "maximum" of 1500 people can use the presence service at the same time in the same tenant. I think it's fair but let's think about this. And ensure we have a good story about backing off if this goes off track.

[plasne]

Functionnaly, this is amazing! Works great, very snappy, visually appealing and only the presence icon changes so this is a BIG win!

Based on the limits of this API, I'm wondering if we should be test a little bit more.

• Maximum of 650 user IDs are supported per API request.
• The maximum request rate of this API is 1500 API requests in a 30 second period, per application per tenant.

That means we couldn't make more than 1500 API requests every 30 seconds. So first, I'd align our polling to be 30 seconds to ensure we don't fall into these limits. This also means that a "maximum" of 1500 people can use the presence service at the same time in the same tenant. I think it's fair but let's think about this. And ensure we have a good story about backing off if this goes off track.

I will add batching for no more than 650 users in a call (see graph.userWithPhoto).

[sebastienlevert]

Do you want to make this an active PR @plasne? I think we highly aligned here so the need for a draft is not required anymore! Thanks!

[plasne]

Do you want to make this an active PR @plasne? I think we highly aligned here so the need for a draft is not required anymore! Thanks!

I removed DRAFT. I will hopefully get these changes in later today, but if not, early next week.

[plasne]

@sebastienlevert , @gavinbarron , ready for another review.

[sebastienlevert]

This works great! I think I found a bug, though when a mgt-person is configured to be disabled from a and another mgt-person is enabled, both are becoming enabled.

First is enabled, second is the mock, third is disabled, but now it's becoming green!

[plasne]

This works great! I think I found a bug, though when a mgt-person is configured to be disabled from a and another mgt-person is enabled, both are becoming enabled.

First is enabled, second is the mock, third is disabled, but now it's becoming green!

I think the confusion is the story is NOT a mock. This is just some JavaScript to fake the behavior. I didn't see any samples in storybook that were mocks and after talking it over with other team members that used storybook, they suggesting not mocking. If we want this to be a mock, then I have to figure out how we even do that.

[sebastienlevert]

I think the confusion is the story is NOT a mock. This is just some JavaScript to fake the behavior. I didn't see any samples in storybook that were mocks and after talking it over with other team members that used storybook, they suggesting not mocking. If we want this to be a mock, then I have to figure out how we even do that.

Let's call it fake! It works great with the Sandbox tenant and I love that it's there. Some other stories have been using something similar in the past, so it's absolutely fine! What I'm suggesting is that we add another component on this story that is just the normal component. That way, when signed in, it will actually poll for presence changes!

[gavinbarron]

Thanks for the changes, I made a small correction to the disabled refresh component in the story, but otherwise this looks good to me