[PM-328] Move exporter to tools #5070

djsmith85 · 2023-03-24T01:06:43Z

Type of change

- [ ] Bug fix
- [ ] New feature development
- [X] Tech debt (refactoring, code cleanup, dependency upgrades, etc)
- [ ] Build/deploy pipeline (DevOps)
- [ ] Other

Objective

Re-structure code in preparation for code ownership of the tools-team

Extract common/export.services and related files into their own lib -> libs/exporter

Code changes

Create and register new libs/exporter - 573c0b0
- Create package.json
- Create tsconfig
- Create jest.config
- Extend shared and root tsconfig and jest.configs
- Register with eslint
Migrate exportService to libs/exporter - b46479c
- Move exportService (abstraction and impl) into libs/exporter
- Refactored exportService to be split into vault-export and event-export
- Created barrel-files for both exports
- Moved export.service.spec.ts into vault-export
- Created an export-helper, which helps build the filename (extract method refactor from ExportService)
Move components in libs/angular into tools-subfolder - 5126642
- Moved components
- Updated imports in jslib-services.module and jslib.module
Register libs/exporter with browser and fix imports - 25bfc3c
- Move export.component into tools-subfolder
Register libs/exporter with cli and fix imports - 91fe936
- Move export.command into tools-subfolder
Register libs/exporter with desktop and fix imports - 1d9ff87
- Move export.component into tools-subfolder
Move export models to libs/exporter - 0720f3e
Update web imports - 6320e0c
Update package-lock.json - 6a145fa
Move export models back as it would create circular dependency - 4972870
- Reponse models in common rely on export models which are in libs/exporter, which relies on common
Fix up web for event-export - 2cf40e9

Before you submit

Please add unit tests where it makes sense to do so (encouraged but not required)
If this change requires a documentation update - notify the documentation team
If this change has particular deployment requirements - notify the DevOps team
Ensure that all UI additions follow WCAG AA requirements

Create package.json Create tsconfig Create jest.config Extend shared and root tsconfig and jest.configs Register with eslint

Move exportService (abstraction and impl) into libs/exporter Refactored exportService to be split into vault-export and event-export Created barrel-files for both exports Moved export.service.spec.ts into vault-export Created an export-helper, which helps build the filename (extract method refactor from ExportService)

Moved components Updated imports in jslib-services.module and jslib.module

Move export.component into tools-subfolder

Move export.command into tools-subfolder

Move export.component into tools-subfolder

…-328-move-exporter-to-tools

Reponse models in common rely on export models which are in libs/exporter, which relies on common

Hinton · 2023-03-24T10:36:45Z

libs/common/src/models/export/index.ts

+export { CardExport } from "./card.export";
+export { CipherWithIdExport } from "./cipher-with-ids.export";
+export { CipherExport } from "./cipher.export";
+export { CollectionWithIdExport } from "./collection-with-id.export";
+export { CollectionExport } from "./collection.export";
+export { FieldExport } from "./field.export";
+export { FolderWithIdExport } from "./folder-with-id.export";
+export { FolderExport } from "./folder.export";
+export { IdentityExport } from "./identity.export";
+export { LoginUriExport } from "./login-uri.export";
+export { SecureNoteExport } from "./secure-note.export";


Shouldn't these be moved to the export module? Feels like internal implementation details for the export lib.

As discussed previously with @MGibson1 and you, ideally these would move over into the export-package, but this would create a weird reference for the CLI which extends the export-models. As we currently do not plan to change this, due to the low-benefit to high work ratio, I'm proposing to keep them in libs/common/models/export.

Hinton · 2023-03-24T10:38:04Z

libs/exporter/src/event-export/event-export.service.abstraction.ts

@@ -0,0 +1,6 @@
+import { EventView } from "@bitwarden/common/models/view/event.view";
+
+export abstract class EventExportServiceAbstraction {


Do we consider event exporters part of the export module? Those feel like a separate domain.

I split it off, as it previously was part of the export.service. Which it definitely shouldn't be. Currently the event-export is only possible in the web-vault and could be moved their, if desired.
On the other hand, it might be benefical to have it in a lib to be also used in the CLI. This has not been discussed with Product, but it could be useful to have a CLI way of exporting and piping the output into a different tool (log-analyzer)

I was more thinking that libs/exporter should encompass the vault exporting capabilities, and events belongs to another module.

…-328-move-exporter-to-tools

Hinton · 2023-04-18T12:38:02Z

libs/common/src/models/export/field.export.ts

@@ -1,7 +1,7 @@
 import { FieldType, LinkedIdType } from "../../enums";
+import { EncString } from "../../models/domain/enc-string";


Suggested change

import { EncString } from "../../models/domain/enc-string";

import { EncString } from "../domain/enc-string";

Hinton · 2023-04-18T12:38:20Z

libs/common/src/models/export/login-uri.export.ts

@@ -1,7 +1,7 @@
 import { UriMatchType } from "../../enums";
+import { EncString } from "../../models/domain/enc-string";


Suggested change

import { EncString } from "../../models/domain/enc-string";

import { EncString } from "../domain/enc-string";

Hinton · 2023-04-18T12:39:18Z

libs/exporter/src/event-export/event-export.service.abstraction.ts

@@ -0,0 +1,6 @@
+import { EventView } from "@bitwarden/common/models/view/event.view";
+
+export abstract class EventExportServiceAbstraction {


I was more thinking that libs/exporter should encompass the vault exporting capabilities, and events belongs to another module.

Hinton · 2023-04-18T12:39:43Z

libs/exporter/src/event-export/event-export.service.ts

+
+export class EventExportService implements EventExportServiceAbstraction {


Suggested change

export class EventExportService implements EventExportServiceAbstraction {

@Injectable()

export class EventExportService implements EventExportServiceAbstraction {

djsmith85 added 11 commits March 16, 2023 01:21

Create and register new libs/exporter

573c0b0

Create package.json Create tsconfig Create jest.config Extend shared and root tsconfig and jest.configs Register with eslint

Move components in libs/angular into tools-subfolder

5126642

Moved components Updated imports in jslib-services.module and jslib.module

Register libs/exporter with browser and fix imports

25bfc3c

Move export.component into tools-subfolder

Register libs/exporter with cli and fix imports

91fe936

Move export.command into tools-subfolder

Register libs/exporter with desktop and fix imports

1d9ff87

Move export.component into tools-subfolder

Merge branch 'master' of https://github.com/bitwarden/clients into pm…

d5b4701

…-328-move-exporter-to-tools

Merge branch 'master' of https://github.com/bitwarden/clients into pm…

b3ae5f3

…-328-move-exporter-to-tools

Move export models to libs/exporter

0720f3e

Update web imports

6320e0c

Update package-lock.json

6a145fa

github-actions bot added the needs-qa Marks a PR as requiring QA approval label Mar 24, 2023

djsmith85 added 2 commits March 24, 2023 02:23

Move export models back as it would create circular dependency

4972870

Reponse models in common rely on export models which are in libs/exporter, which relies on common

Fix up web for event-export

2cf40e9

djsmith85 marked this pull request as ready for review March 24, 2023 01:44

djsmith85 requested review from a team as code owners March 24, 2023 01:44

djsmith85 requested a review from a team March 24, 2023 01:45

Update CODEOWNERS

6761303

Hinton reviewed Mar 24, 2023

View reviewed changes

Merge branch 'master' of https://github.com/bitwarden/clients into pm…

b6745f1

…-328-move-exporter-to-tools

djsmith85 requested a review from a team as a code owner April 4, 2023 12:30

djsmith85 mentioned this pull request Apr 12, 2023

[PM-1144] Fix last active (auto logout issue) #4835

Merged

djsmith85 added 2 commits April 13, 2023 16:30

Merge branch 'master' of https://github.com/bitwarden/clients into pm…

033b354

…-328-move-exporter-to-tools

Add export-models to team-tools-dev

41e5a25

djsmith85 requested a review from Hinton April 17, 2023 17:41

Merge branch 'master' of https://github.com/bitwarden/clients into pm…

87b8403

…-328-move-exporter-to-tools

Hinton reviewed Apr 18, 2023

View reviewed changes

djsmith85 added 2 commits April 18, 2023 15:32

Simplify domain import

ec86811

Moving EventExport into web

ac6cdd9

djsmith85 requested a review from Hinton April 18, 2023 15:26

Hinton approved these changes Apr 19, 2023

View reviewed changes

djsmith85 removed the needs-qa Marks a PR as requiring QA approval label Apr 19, 2023

djsmith85 merged commit 192bb5a into master Apr 19, 2023

djsmith85 deleted the pm-328-move-exporter-to-tools branch April 19, 2023 09:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[PM-328] Move exporter to tools #5070

[PM-328] Move exporter to tools #5070

djsmith85 commented Mar 24, 2023 •

edited

Loading

Hinton Mar 24, 2023

djsmith85 Apr 17, 2023

Hinton Mar 24, 2023

djsmith85 Apr 17, 2023

Hinton Apr 18, 2023

Hinton Apr 18, 2023

Hinton Apr 18, 2023

Hinton Apr 18, 2023

Hinton Apr 18, 2023

		@@ -0,0 +1,6 @@
		import { EventView } from "@bitwarden/common/models/view/event.view";

		export abstract class EventExportServiceAbstraction {

		@@ -1,7 +1,7 @@
		import { FieldType, LinkedIdType } from "../../enums";
		import { EncString } from "../../models/domain/enc-string";

	import { EncString } from "../../models/domain/enc-string";
	import { EncString } from "../domain/enc-string";

		@@ -1,7 +1,7 @@
		import { UriMatchType } from "../../enums";
		import { EncString } from "../../models/domain/enc-string";


		export class EventExportService implements EventExportServiceAbstraction {

[PM-328] Move exporter to tools #5070

[PM-328] Move exporter to tools #5070

Conversation

djsmith85 commented Mar 24, 2023 • edited Loading

Type of change

Objective

Code changes

Before you submit

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

djsmith85 commented Mar 24, 2023 •

edited

Loading