Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[AC-1119] [PM-1923] [AC-701] Import into a specified folder or collection #5683

Merged
merged 51 commits into from
Aug 4, 2023

Conversation

djsmith85
Copy link
Contributor

@djsmith85 djsmith85 commented Jun 26, 2023

Type of change

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

Objective

Flexible collections AC-1119

  • Enable users to select their target-vault(individual and organizational vaults) for importing, without needing to navigate between (Tools ->Import data and Organization -> Select Org -> Settings -> Import data)
  • Enable users to select a folder/collection as a new root for the import (optional)
    • If nothing is selected, the import will proceed as before, importing cipher into their assigned folders/collections
    • If selected, all items without a folder or collection assigned to them will be mapped onto the selected target
    • Any folder/collections present in the import file will be moved to a sub-folder/sub-collection of the selected target

Tech debt PM-1923 [AC-701]

Code changes

UI rework:

  • apps/web/src/app/tools/import-export/import.component.html: Reactive forms and
  • apps/web/src/app/tools/import-export/import.component.ts: Retrieve vaults (individual/orgs), folders and collections for the new dropdowns. Pass on selected import target(folder/collection) on to import.service
  • apps/web/src/app/admin-console/organizations/tools/import-export/org-import.component.ts: Import Formbuilder and pass into to ImportComponent ctor
  • libs/common/src/admin-console/abstractions/organization/organization.service.abstraction.ts: Helper function to filter out any organizations from the dropwdown, where a member does not have the right permissions to either import or export.

Enabling import into a specific folder or collections

  • libs/importer/src/services/import.service.ts: Add ability to pass in an optional folderId/collectionId which will be used as new root for any items being imported.
  • libs/importer/src/services/import.service.abstraction.ts: Extend interface to receive selected import target (null | folderId | collectionId)
  • libs/importer/src/services/import.service.spec.ts: Added unit tests for old behaviour when no target is selected, and with a selected target.
  • apps/web/src/app/tools/import-export/import.component.ts: Retrieve vaults (individual/orgs), folders and collections for the new dropdowns. Pass on selected import target(folder/collection) on to import.service
  • apps/web/src/app/admin-console/organizations/tools/import-export/org-import.component.ts: Import folder-, org- and collection-service and pass onto ImportComponent ctor

Screenshots

BEFORE:
image

AFTER:
Vault selector:
image

Individual vault:
image

Organizational vault:
image

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

djsmith85 and others added 21 commits June 14, 2023 11:06
Extended import.service and abstraction to receive importTarget on import()
Pass selectedImportTarget to importService.import()
Wrote unit tests
Map ciphers with no folder/no collection to the new rootFolder when selected by the user
Modified and added unit tests
@github-actions github-actions bot added the needs-qa Marks a PR as requiring QA approval label Jun 26, 2023
@djsmith85 djsmith85 changed the title [AC-1119] [PM-1923] Import into a specified folder or collection [AC-1119] [PM-1923] [AC-701] Import into a specified folder or collection Jun 26, 2023
Rename old submit() to performImport()
Create submit arrow function calling performImport() (which can be overridden/called by org-import.component)
Remove #form and ngNativeValidate
Add bitSubmit and bitFormButton directives
Remove now unneeded loading variable
Copy link
Member

@shane-melton shane-melton left a comment

Choose a reason for hiding this comment

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

Nice work! I only have a few more comments/suggestions and then I think we'll be all set!

shane-melton
shane-melton previously approved these changes Jul 5, 2023
Copy link
Member

@shane-melton shane-melton left a comment

Choose a reason for hiding this comment

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

Changes look good to me! Nice job!

@djsmith85 djsmith85 requested a review from aj-rosado July 6, 2023 10:25
aj-rosado
aj-rosado previously approved these changes Jul 6, 2023
eliykat
eliykat previously approved these changes Jul 7, 2023
Hide the `My Vault` entry when policy is active
Always check if the policy applies and disable the formGroup if no vault-target is selectable
@djsmith85 djsmith85 dismissed stale reviews from aj-rosado, eliykat, and shane-melton via 641a05d August 1, 2023 11:04
@bitwarden-bot
Copy link

bitwarden-bot commented Aug 1, 2023

Logo
Checkmarx One – Scan Summary & Detailsb1e94a7d-86b8-44cc-8ca3-d443e2792b55

No New Or Fixed Issues Found

aj-rosado
aj-rosado previously approved these changes Aug 1, 2023
Copy link
Contributor

@aj-rosado aj-rosado left a comment

Choose a reason for hiding this comment

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

Changes are looking good to me!

* Display select folder/collection in targetSelector
Filter the no-folder entry from the folderViews-observable
Add labels for the targetSelector placeholders

* Update importTargetHint and remove importTargetOrgHint

* Update language on importUnassignedItemsError

* Add help icon with link to the import documentation
@djsmith85
Copy link
Contributor Author

Re-requesting review from @aj-rosado due to merging PR #5933

Copy link
Contributor

@aj-rosado aj-rosado left a comment

Choose a reason for hiding this comment

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

LGTM

@djsmith85 djsmith85 enabled auto-merge (squash) August 4, 2023 21:15
Copy link
Member

@vvolkgang vvolkgang left a comment

Choose a reason for hiding this comment

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

Given that @shane-melton and @eliykat previously approved it, I went through the recent commits and none had Admin Console related changes, approving on behalf of them.

@djsmith85 djsmith85 removed the needs-qa Marks a PR as requiring QA approval label Aug 4, 2023
@djsmith85 djsmith85 merged commit e98cbed into master Aug 4, 2023
66 of 70 checks passed
@djsmith85 djsmith85 deleted the AC-1119-flexible-collections-import-page branch August 4, 2023 22:05
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.

6 participants