Skip to content

Conversation

@ghost
Copy link

@ghost ghost commented Sep 15, 2020

No description provided.

Shahak Yosef and others added 9 commits July 19, 2020 14:19
After releasing JS SDK, models and report authoring we can update the playground to use the latest report authoring version.
Fixing a bug where /report/embed message is sent before bootstrap is complete.

**Repo:**
1) Call bootstrap.
2) Call report.embed immediately.

**Timing:**
1) Bootstrap starts.
2) iframe is not loaded yet ('load') event is not fired to the SDK.
3) powerbi.embed starts
4) powerbi.embed calls embedExisting because iframe already exists - it's fine.
5) embedExisting calls embed::load method which sends /report/load message to the SDK
6) iframe is loaded (bootstrap is done).
7) ('load') event handler calls embed::load method which sends /report/load message to the SDK - AGAIN

**Expected:**
/report/load is called one time only.

**Acutal**
/report/load is called twice - first time before iframe is ready is a BUG.

**Fix**
Adding a flag which saves the state of the iframe (loaded/not loaded).
If iframe is not loaded, avoid sending the postMessage.
We need the load method to send the right config when it's called from 'load' event. For that, we save the last config per component, and post the load message with that config.
Currently playground sandbox overrides element.createNode and powerbi JS service.handleEvent methods to allow safe evaluation of user code.

Instead of overriding these general methods, added 2 custom methods to js SDK service.ts
Remove default report settings
We want to release a fix for the JS SDK but we don't want to include the playground safe eval changes.
Temporarily reverting the PR so we can create a release.
It will be checked-in again afterwards.
…ready sent

A client has been reporting "content is not available" issues for some time.
We notice in their logs that several load requests are sent to the front end with only 66 millisecond between them.
We believe that this is causing a timing issue in the front end and so we add a throttle on the load requests.

## Notes for reviewers:
I am not sure if there can be any valid use case for multiple load requests in short intervals.
Can it negatively affect valid uses cases of bootstrap -> embed -> potentially reload flows?
@ghost ghost requested a review from ali-hamud September 15, 2020 11:51
@ali-hamud ali-hamud merged commit c87eb45 into master Sep 15, 2020
anant-k-singh pushed a commit that referenced this pull request Feb 12, 2021
commit 53c1d1e
Merge: b87e94d 039dd38
Author: ali-hamud <[email protected]>
Date:   Wed Dec 23 11:00:42 2020 +0200

    Merge pull request #369 from guyinacube/videoupdate

    👊 Update old content video

commit b87e94d
Merge: c87eb45 6a67f3b
Author: ali-hamud <[email protected]>
Date:   Wed Dec 23 11:00:26 2020 +0200

    Merge pull request #367 from snehaldalvi/bashwoman

    Spelling Corrected

commit 039dd38
Author: Adam Saxton <[email protected]>
Date:   Wed Dec 2 15:13:38 2020 -0600

    Update old content video

commit 6a67f3b
Author: Snehal <[email protected]>
Date:   Thu Oct 1 09:27:10 2020 +0530

    Typo Changed

Related work items: #351, #357, #366, #367, #369, #371, #372
@ghost ghost deleted the releases-2.14.1 branch March 22, 2021 15:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant