Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Exported filename ISO date format #9142

Closed
wants to merge 3 commits into from

Conversation

yaya-usman
Copy link
Contributor

@yaya-usman yaya-usman commented Aug 6, 2022

A breakout PR from #8558 PR as requested by @kittykat


Here's what your changelog entry will look like:

✨ Features

  • Exported filename ISO date format (#9142).

@yaya-usman yaya-usman added the T-Enhancement New features, changes in functionality, performance boosts, user-facing improvements label Aug 6, 2022
@yaya-usman yaya-usman requested a review from kittykat August 6, 2022 08:35
@yaya-usman yaya-usman requested a review from a team as a code owner August 6, 2022 08:35
@yaya-usman yaya-usman requested review from turt2live, t3chguy and justjanne and removed request for turt2live, t3chguy and justjanne August 6, 2022 08:35
@@ -77,7 +77,7 @@ export default abstract class Exporter {

protected async downloadZIP(): Promise<string | void> {
const brand = SdkConfig.get().brand;
const filenameWithoutExt = `${brand} - Chat Export - ${formatFullDateNoDay(new Date())}`;
const filenameWithoutExt = `${brand} - ${this.room.name} - Chat Export -${formatFullDateNoDayISO(new Date())}`;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we still need to consider the room name having invalid characters in it?

applies to later on as well.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@turt2live there is another PR which does that, I believe: #7992

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can't land this code at the moment without that code, given the safety risk we'd be taking on. It feels like it might have been split up too much?

@turt2live
Copy link
Member

Thanks for the PR! Sorry it took so long to get considered. We've decided to merge the changes from this and #7992 into a single PR to ease the requirements raised by the Product team, and are closing this PR as a result. Hopefully this should mean the code goes live soon.

You can see the new PR here: #9440

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
T-Enhancement New features, changes in functionality, performance boosts, user-facing improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants