Skip to content

Commit

Permalink
Fix stringified promises in HTML exports
Browse files Browse the repository at this point in the history
Fixes element-hq/element-web#24272

HtmlExporter was coercing Promises into strings. This removes the unecessary
async labels on the offending methods.

Signed-off-by: Clark Fischer <[email protected]>
  • Loading branch information
clarkf committed Jan 24, 2023
1 parent 8425fe5 commit 849fcef
Show file tree
Hide file tree
Showing 5 changed files with 147 additions and 15 deletions.
6 changes: 3 additions & 3 deletions src/utils/exportUtils/HtmlExport.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ export default class HTMLExporter extends Exporter {
}
}

protected async getDateSeparator(event: MatrixEvent): Promise<string> {
protected getDateSeparator(event: MatrixEvent): string {
const ts = event.getTs();
const dateSeparator = (
<li key={ts}>
Expand All @@ -248,7 +248,7 @@ export default class HTMLExporter extends Exporter {
return renderToStaticMarkup(dateSeparator);
}

protected async needsDateSeparator(event: MatrixEvent, prevEvent: MatrixEvent | null): Promise<boolean> {
protected needsDateSeparator(event: MatrixEvent, prevEvent: MatrixEvent | null): boolean {
if (!prevEvent) return true;
return wantsDateSeparator(prevEvent.getDate() || undefined, event.getDate() || undefined);
}
Expand Down Expand Up @@ -400,7 +400,7 @@ export default class HTMLExporter extends Exporter {
if (this.cancelled) return this.cleanUp();
if (!haveRendererForEvent(event, false)) continue;

content += (await this.needsDateSeparator(event, prevEvent)) ? this.getDateSeparator(event) : "";
content += this.needsDateSeparator(event, prevEvent) ? this.getDateSeparator(event) : "";
const shouldBeJoined =
!this.needsDateSeparator(event, prevEvent) &&
shouldFormContinuation(prevEvent, event, false, this.threadsEnabled);
Expand Down
4 changes: 2 additions & 2 deletions src/utils/exportUtils/exportCSS.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,8 @@ const getExportCSS = async (usedClasses: Set<string>): Promise<string> => {

// If the light theme isn't loaded we will have to fetch & parse it manually
if (!stylesheets.some(isLightTheme)) {
const href = document.querySelector<HTMLLinkElement>('link[rel="stylesheet"][href$="theme-light.css"]').href;
stylesheets.push(await getRulesFromCssFile(href));
const href = document.querySelector<HTMLLinkElement>('link[rel="stylesheet"][href$="theme-light.css"]')?.href;
if (href) stylesheets.push(await getRulesFromCssFile(href));
}

let css = "";
Expand Down
4 changes: 4 additions & 0 deletions test/test-utils/test-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,10 @@ export function createTestClient(): MatrixClient {
setPassword: jest.fn().mockRejectedValue({}),
groupCallEventHandler: { groupCalls: new Map<string, GroupCall>() },
redactEvent: jest.fn(),

createMessagesRequest: jest.fn().mockResolvedValue({
chunk: [],
}),
} as unknown as MatrixClient;

client.reEmitter = new ReEmitter(client);
Expand Down
62 changes: 52 additions & 10 deletions test/utils/exportUtils/HTMLExport-test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
Copyright 2022 The Matrix.org Foundation C.I.C.
Copyright 2022 - 2023 The Matrix.org Foundation C.I.C.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
Expand All @@ -14,26 +14,36 @@ See the License for the specific language governing permissions and
limitations under the License.
*/

import { mocked } from "jest-mock";
import { EventType, IRoomEvent, MatrixClient, Room } from "matrix-js-sdk/src/matrix";

import { createTestClient, mkStubRoom, REPEATABLE_DATE } from "../../test-utils";
import { mkStubRoom, REPEATABLE_DATE, stubClient } from "../../test-utils";
import { ExportType, IExportOptions } from "../../../src/utils/exportUtils/exportUtils";
import SdkConfig from "../../../src/SdkConfig";
import HTMLExporter from "../../../src/utils/exportUtils/HtmlExport";
import DMRoomMap from "../../../src/utils/DMRoomMap";

jest.mock("jszip");

describe("HTMLExport", () => {
let client: jest.Mocked<MatrixClient>;

beforeEach(() => {
jest.useFakeTimers();
jest.setSystemTime(REPEATABLE_DATE);
});

afterEach(() => {
mocked(SdkConfig.get).mockRestore();
client = stubClient() as jest.Mocked<MatrixClient>;
DMRoomMap.makeShared();
});

function getMessageFile(exporter: HTMLExporter): Blob {
//@ts-ignore private access
const files = exporter.files;
const file = files.find((f) => f.name == "messages.html")!;
return file.blob;
}

it("should have an SDK-branded destination file name", () => {
const roomName = "My / Test / Room: Welcome";
const client = createTestClient();
const stubOptions: IExportOptions = {
attachmentsIncluded: false,
maxSize: 50000000,
Expand All @@ -43,10 +53,42 @@ describe("HTMLExport", () => {

expect(exporter.destinationFileName).toMatchSnapshot();

jest.spyOn(SdkConfig, "get").mockImplementation(() => {
return { brand: "BrandedChat/WithSlashes/ForFun" };
});
SdkConfig.put({ brand: "BrandedChat/WithSlashes/ForFun" });

expect(exporter.destinationFileName).toMatchSnapshot();
});

it("should export", async () => {
const room = new Room("!myroom:example.org", client, "@me:example.org");

const events = [...Array(50)].map<IRoomEvent>((_, i) => ({
event_id: "$1",
type: EventType.RoomMessage,
sender: `@user${i}:example.com`,
origin_server_ts: 5_000 + i * 1000,
content: {
msgtype: "m.text",
body: `Message #${i}`,
},
}));

client.getRoom.mockReturnValue(room);
client.createMessagesRequest.mockResolvedValue({ chunk: events });

const exporter = new HTMLExporter(
room,
ExportType.LastNMessages,
{
attachmentsIncluded: false,
maxSize: 1_024 * 1_024,
numberOfMessages: events.length,
},
() => {},
);

await exporter.export();

const file = getMessageFile(exporter);
expect(await file.text()).toMatchSnapshot();
});
});
86 changes: 86 additions & 0 deletions test/utils/exportUtils/__snapshots__/HTMLExport-test.ts.snap

Large diffs are not rendered by default.

0 comments on commit 849fcef

Please sign in to comment.