microsoft-graph-toolkit icon indicating copy to clipboard operation
microsoft-graph-toolkit copied to clipboard

Refactor GraphNotificationClient subscription renewal process

Open andrewDoing opened this issue 1 year ago • 2 comments

Closes #3072

PR Type

Bugfix

Refactoring (no functional changes, no api changes)

Description of the changes

  • Removed cleanup timer
  • Added renewal loop
    • All renewal logic occurs in this loop, allowing for easier debugging when issues arise.
    • GraphNotificationClient now relies on a single timer (ex. renewalTimeout) to check for chat subscription renewals on a loop. Only active chats qualify for subscription renewals. Inactive chats will naturally expire.
  • Move sessionId generation into the GraphChatClient where its used.
  • Shortened renewal timer interval from 20 seconds to 3. This timer is shared across chats so it needed to be faster
  • Renewal will trigger on a chatId change based on a faster timer

PR checklist

  • [x] Project builds (yarn build) and changes have been tested in at least two supported browsers (Edge + non-Chromium based browser)
  • [x] All public APIs (classes, methods, etc) have been documented following the jsdoc syntax
  • [x] Stories have been added and existing stories have been tested
  • [x] Added appropriate documentation. Docs PR:
  • [x] License header has been added to all new source files (yarn setLicense)
  • [x] Contains NO breaking changes

Other information

Authored by MSFT FTEs: andoing and dexterw

andrewDoing avatar Mar 15 '24 20:03 andrewDoing

Thank you for creating a Pull Request @andrewDoing.

This is a checklist for the PR reviewer(s) to complete before approving and merging this PR:

  • [ ] I have verified a documentation PR has been linked and is approved (or not applicable)
  • [ ] I have ran this PR locally and have tested the fix/feature
  • [ ] I have verified that stories have been added to storybook (or not applicable)
  • [ ] I have tested existing stories in storybook to verify no regression has occured
  • [ ] I have tested the solution in at least two browsers (Edge + 1 non-Chromium based browser)

This probably also closes #3059 . No longer able to reproduce this issue.

TechnicallyWilliams avatar Mar 19 '24 15:03 TechnicallyWilliams