Skip to content

Commit 493f3b1

Browse files
author
Matt Mazzola
committed
Refactor reportId extraction from url to be on Report class since it's implementation is specific to report. Update existing tests and add new tests.
1 parent 1ee2eb5 commit 493f3b1

4 files changed

Lines changed: 131 additions & 34 deletions

File tree

‎src/embed.ts‎

Lines changed: 7 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,8 @@ export interface IInternalEventHandler<T> {
4141
}
4242

4343
export abstract class Embed {
44-
public static embedUrlAttribute = 'powerbi-embed-url';
4544
public static accessTokenAttribute = 'powerbi-access-token';
45+
public static embedUrlAttribute = 'powerbi-embed-url';
4646
public static typeAttribute = 'powerbi-type';
4747
public static type: string;
4848

@@ -71,22 +71,9 @@ export abstract class Embed {
7171
this.config = utils.assign({}, config);
7272
this.config.accessToken = this.getAccessToken(service.accessToken);
7373
this.config.embedUrl = this.getEmbedUrl();
74+
this.config.id = this.getId();
7475
this.config.uniqueId = utils.createRandomString();
7576

76-
// TODO: The findIdFromEmbedUrl method is specific to Reports so it should be in the Report class, but it can't be called until
77-
// after we have fetched the embedUrl from the attributes
78-
79-
/**
80-
* This adds backwards compatibility for older config which used the reportId query param to specify report id.
81-
* E.g. http://embedded.powerbi.com/appTokenReportEmbed?reportId=854846ed-2106-4dc2-bc58-eb77533bf2f1
82-
*
83-
* By extracting the id we can ensure id is always explicitly provided as part of the load configuration.
84-
*/
85-
const id = Embed.findIdFromEmbedUrl(this.config.embedUrl);
86-
if(id) {
87-
this.config.id = id;
88-
}
89-
9077
const iframeHtml = `<iframe style="width:100%;height:100%;" src="${this.config.embedUrl}" scrolling="no" allowfullscreen="true"></iframe>`;
9178

9279
this.element.innerHTML = iframeHtml;
@@ -201,6 +188,11 @@ export abstract class Embed {
201188
return embedUrl;
202189
}
203190

191+
/**
192+
* Get report id from first available location: options, attribute.
193+
*/
194+
abstract getId(): string;
195+
204196
/**
205197
* Request the browser to make the component's iframe fullscreen.
206198
*/
@@ -221,17 +213,6 @@ export abstract class Embed {
221213
exitFullscreen.call(document);
222214
}
223215

224-
private static findIdFromEmbedUrl(url: string): string {
225-
const reportIdRegEx = /reportId="?([^&]+)"?/
226-
const reportIdMatch = url.match(reportIdRegEx);
227-
228-
let reportId;
229-
if(reportIdMatch) {
230-
reportId = reportIdMatch[1];
231-
}
232-
233-
return reportId;
234-
}
235216

236217
/**
237218
* Return true if iframe is fullscreen,

‎src/report.ts‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,27 @@ import * as hpm from 'http-post-message';
66

77
export class Report extends embed.Embed {
88
static allowedEvents = ["dataSelected", "filterAdded", "filterUpdated", "filterRemoved", "pageChanged", "error"];
9+
static reportIdAttribute = 'powerbi-report-id';
910
static type = "Report";
1011

12+
/**
13+
* This adds backwards compatibility for older config which used the reportId query param to specify report id.
14+
* E.g. http://embedded.powerbi.com/appTokenReportEmbed?reportId=854846ed-2106-4dc2-bc58-eb77533bf2f1
15+
*
16+
* By extracting the id we can ensure id is always explicitly provided as part of the load configuration.
17+
*/
18+
static findIdFromEmbedUrl(url: string): string {
19+
const reportIdRegEx = /reportId="?([^&]+)"?/
20+
const reportIdMatch = url.match(reportIdRegEx);
21+
22+
let reportId;
23+
if(reportIdMatch) {
24+
reportId = reportIdMatch[1];
25+
}
26+
27+
return reportId;
28+
}
29+
1130
/**
1231
* Add filter to report
1332
* An optional target may be passed to apply the filter to specific page or visual.
@@ -63,6 +82,19 @@ export class Report extends embed.Embed {
6382
});
6483
}
6584

85+
/**
86+
* Get report id from first available location: options, attribute, embed url.
87+
*/
88+
getId(): string {
89+
const reportId = this.config.id || this.element.getAttribute(Report.reportIdAttribute) || Report.findIdFromEmbedUrl(this.config.embedUrl);
90+
91+
if (typeof reportId !== 'string' || reportId.length === 0) {
92+
throw new Error(`Report id is required, but it was not found. You must provide an id either as part of embed configuration or as attribute '${Report.reportIdAttribute}'.`);
93+
}
94+
95+
return reportId;
96+
}
97+
6698
/**
6799
* Get the list of pages within the report
68100
*

‎src/tile.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,4 +2,8 @@ import { Embed } from './embed';
22

33
export class Tile extends Embed {
44
static type = "Tile";
5+
6+
getId(): string {
7+
throw Error('Not implemented. Embedding tiles is not supported yet.');
8+
}
59
}

‎test/test.spec.ts‎

Lines changed: 88 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ declare global {
1919

2020
let logMessages = (window.__karma__.config.args[0] === 'logMessages');
2121

22-
describe('powerbi', function () {
22+
describe('service', function () {
2323
let powerbi: service.Service;
2424
let $element: JQuery;
2525

@@ -47,7 +47,7 @@ describe('powerbi', function () {
4747
// Arrange
4848
const elements = [
4949
'<div powerbi-embed-url="https://embedded.powerbi.com/appTokenReportEmbed?reportId=ABC123" powerbi-type="report"></div>',
50-
'<div powerbi-embed-url="https://app.powerbi.com/embed?dashboardId=D1&tileId=T1" powerbi-type="tile"></div>'
50+
'<div powerbi-embed-url="https://embedded.powerbi.com/appTokenReportEmbed?reportId=XYZ456" powerbi-type="report"></div>',
5151
];
5252

5353
elements.forEach(element => {
@@ -156,6 +156,21 @@ describe('powerbi', function () {
156156
powerbi.accessToken = originalToken;
157157
});
158158

159+
it('if attempting to embed without specifying an id, throw error', function () {
160+
// Arrange
161+
const embedUrl = `https://embedded.powerbi.com/appTokenReportEmbed`;
162+
const $reportContainer = $(`<div powerbi-embed-url="${embedUrl}" powerbi-type="report"></div>`)
163+
.appendTo('#powerbi-fixture');
164+
165+
// Act
166+
const attemptToEmbed = () => {
167+
powerbi.embed($reportContainer[0]);
168+
};
169+
170+
// Assert
171+
expect(attemptToEmbed).toThrowError();
172+
});
173+
159174
it('if component is already embedded in element re-use the existing component by calling load with the new information', function () {
160175
// Arrange
161176
const $element = $('<div powerbi-embed-url="https://app.powerbi.com/reportEmbed?reportId=ABC123" powerbi-type="report"></div>')
@@ -226,9 +241,7 @@ describe('powerbi', function () {
226241

227242
// Assert
228243
var report = powerbi.get($reportContainer[0]);
229-
// TODO: Find way to prevent using private method getAccessToken.
230-
// Need to know what token the report used, but don't have another option?
231-
var accessToken = (<any>report).getAccessToken();
244+
var accessToken = report.getConfig().accessToken;
232245

233246
expect(accessToken).toEqual(testToken);
234247

@@ -251,9 +264,76 @@ describe('powerbi', function () {
251264
expect(iframe.length).toEqual(1);
252265
expect(iframe.attr('src')).toEqual(embedUrl);
253266
});
267+
268+
describe('findIdFromEmbedUrl', function () {
269+
it('should return value of reportId query parameter in embedUrl', function () {
270+
// Arrange
271+
const testReportId = "ABC123";
272+
const testEmbedUrl = `http://embedded.powerbi.com/appTokenReportEmbed?reportId=${testReportId}`;
273+
274+
// Act
275+
const reportId = report.Report.findIdFromEmbedUrl(testEmbedUrl);
276+
277+
// Assert
278+
expect(reportId).toEqual(testReportId);
279+
});
280+
281+
it('should return undefinded if the query parameter is not in the url', function () {
282+
// Arrange
283+
const testEmbedUrl = `http://embedded.powerbi.com/appTokenReportEmbed`;
284+
285+
// Act
286+
const reportId = report.Report.findIdFromEmbedUrl(testEmbedUrl);
287+
288+
// Assert
289+
expect(reportId).toBeUndefined();
290+
});
291+
});
292+
293+
it('should get report id from configuration first', function () {
294+
// Arrange
295+
const testReportId = "ABC123";
296+
const embedUrl = `https://embedded.powerbi.com/appTokenReportEmbed?reportId=DIFFERENTID`;
297+
const $reportContainer = $(`<div powerbi-embed-url="${embedUrl}" powerbi-type="report"></div>`)
298+
.appendTo('#powerbi-fixture');
299+
300+
// Act
301+
const report = powerbi.embed($reportContainer[0], { id: testReportId });
302+
303+
// Assert
304+
expect(report.getConfig().id).toEqual(testReportId);
305+
});
306+
307+
it('should fallback to using id from attribute if not supplied in embed/load configuration', function () {
308+
// Arrange
309+
const testReportId = "ABC123";
310+
const embedUrl = `https://embedded.powerbi.com/appTokenReportEmbed?reportId=DIFFERENTID`;
311+
const $reportContainer = $(`<div powerbi-embed-url="${embedUrl}" powerbi-type="report" powerbi-report-id="${testReportId}"></div>`)
312+
.appendTo('#powerbi-fixture');
313+
314+
// Act
315+
const report = powerbi.embed($reportContainer[0]);
316+
317+
// Assert
318+
expect(report.getConfig().id).toEqual(testReportId);
319+
});
320+
321+
it('should fallback to using id from embedUrl if not supplied in embed/load configuration or attribute', function () {
322+
// Arrange
323+
const testReportId = "ABC123";
324+
const embedUrl = `https://embedded.powerbi.com/appTokenReportEmbed?reportId=${testReportId}`;
325+
const $reportContainer = $(`<div powerbi-embed-url="${embedUrl}" powerbi-type="report" powerbi-report-id></div>`)
326+
.appendTo('#powerbi-fixture');
327+
328+
// Act
329+
const report = powerbi.embed($reportContainer[0]);
330+
331+
// Assert
332+
expect(report.getConfig().id).toEqual(testReportId);
333+
});
254334
});
255335

256-
describe('tiles', function () {
336+
xdescribe('tiles', function () {
257337
it('creates tile iframe from embedUrl', function () {
258338
// Arrange
259339
var embedUrl = 'https://app.powerbi.com/embed?dashboardId=D1&tileId=T1';
@@ -278,7 +358,7 @@ describe('powerbi', function () {
278358
powerbi.embed($element.get(0), {
279359
type: 'report',
280360
embedUrl: 'fakeUrl',
281-
id: undefined,
361+
id: 'fakeId',
282362
accessToken: 'fakeToken'
283363
});
284364

@@ -296,7 +376,7 @@ describe('powerbi', function () {
296376
powerbi.embed($element.get(0), {
297377
type: 'report',
298378
embedUrl: 'fakeUrl',
299-
id: undefined,
379+
id: 'fakeReportId',
300380
accessToken: 'fakeToken'
301381
});
302382

0 commit comments

Comments
 (0)