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

Defer scalar API calls until they are needed #3115

Merged
merged 12 commits into from Jun 18, 2019

Conversation

Projects
None yet
2 participants
@turt2live
Copy link
Member

commented Jun 17, 2019

Fixes vector-im/riot-web#5846

In practice this means not calling scalar until one of the following things happens:

  • The user opens the integration manager
  • The user starts a conference call
  • The user has a widget open

We no longer eagerly check for scalar auth everywhere, which was causing multiple calls to various endpoints at useless times. This means that the manage integrations button doesn't report errors on itself anymore (but the integration manager dialog does), the sticker picker doesn't check auth until needed, and the app drawer doesn't check for auth (instead letting the widgets do that).

After this PR, we make zero calls to scalar when switching rooms (unless the destination room has a widget) and we make zero calls on startup (unless the landing room has a widget).

The behaviour of having an empty (not undefined) integrations_ui_url in the config still works - the integrations server is considered disabled (see screenshots below).

Tested with both Modular (Scalar) and Dimension - both are happy with this change as far as I can tell.

Integrations enabled:
image

Integrations disabled:
image
image

New integration manager dialog states:
image
image

turt2live added some commits Jun 17, 2019

Make the Manage Integrations Button defer scalar auth to the manager
This moves the responsibility of creating a URL to open from the button (and other components) to the integrations manager dialog itself.

By doing this, we also cut down on scalar API calls because we don't pick up on account information until the user opens the dialog.

@turt2live turt2live marked this pull request as ready for review Jun 17, 2019

@turt2live turt2live requested a review from matrix-org/riot-web Jun 17, 2019

@bwindels bwindels requested review from bwindels and removed request for matrix-org/riot-web Jun 18, 2019

@bwindels
Copy link
Contributor

left a comment

thanks for this, looks great! Just a few questions.

Also wanted to double check if we need to change anything in TextualBody.onStarterLinkClick ... probably not as the scalarclient didn't change API...

Show resolved Hide resolved src/components/views/rooms/Stickerpicker.js Outdated
Show resolved Hide resolved src/components/views/rooms/Stickerpicker.js Outdated
Show resolved Hide resolved src/IntegrationManager.js Outdated
@turt2live

This comment has been minimized.

Copy link
Member Author

commented Jun 18, 2019

onStarterLinkClick is unaffected by this change, and doesn't spam calls when it doesn't need to.

@turt2live turt2live requested a review from bwindels Jun 18, 2019

@bwindels
Copy link
Contributor

left a comment

lgtm!

@bwindels

This comment has been minimized.

Copy link
Contributor

commented Jun 18, 2019

Got a lint error though:

/src/components/views/rooms/Stickerpicker.js 22:8 error 'SdkConfig' is defined but never used no-unused-vars

@turt2live turt2live merged commit 7b514b0 into develop Jun 18, 2019

3 of 6 checks passed

buildkite/matrix-react-sdk/pr Build #1199 started
Details
buildkite/matrix-react-sdk/pr/chains-end-to-end-tests Started...
Details
buildkite/matrix-react-sdk/pr/karma-tests Started...
Details
buildkite/matrix-react-sdk/pr/eslint-lint Passed (2 minutes, 27 seconds)
Details
buildkite/matrix-react-sdk/pr/pipeline Passed (9 seconds)
Details
buildkite/matrix-react-sdk/pr/riot-tests Passed (5 minutes, 2 seconds)
Details

@turt2live turt2live deleted the travis/reduce-scalar-calls branch Jun 18, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.