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

[PM-11882] Handled identity item and unsupported items during ProtonPass import. #10967

Merged

Conversation

aliaftab612
Copy link
Contributor

@aliaftab612 aliaftab612 commented Sep 9, 2024

🎟️ Tracking

#10654

📔 Objective

  1. Skipped all items unsupported by Bitwarden during ProtonPass import.
  2. Mapped ProtonPass Identity item to Bitwarden Identity.

📸 Screenshots

⏰ Reminders before review

  • Contributor guidelines followed
  • All formatters and local linters executed and passed
  • Written new unit and / or integration tests where applicable
  • Protected functional changes with optionality (feature flags)
  • Used internationalization (i18n) for all UI strings
  • CI builds passed
  • Communicated to DevOps any deployment requirements
  • Updated any necessary documentation (Confluence, contributing docs) or informed the documentation team

🦮 Reviewer guidelines

  • 👍 (:+1:) or similar for great changes
  • 📝 (:memo:) or ℹ️ (:information_source:) for notes or general info
  • ❓ (:question:) for questions
  • 🤔 (:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion
  • 🎨 (:art:) for suggestions / improvements
  • ❌ (:x:) or ⚠️ (:warning:) for more significant problems or concerns needing attention
  • 🌱 (:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt
  • ⛏ (:pick:) for minor or nitpick changes

@aliaftab612 aliaftab612 requested a review from a team as a code owner September 9, 2024 19:15
@bitwarden-bot
Copy link

Thank you for your contribution! We've added this to our internal Community PR board for review.
ID: PM-11882

@bitwarden-bot bitwarden-bot changed the title Handled identity item and unsupported items during ProtonPass import. [PM-11882] Handled identity item and unsupported items during ProtonPass import. Sep 9, 2024
@CLAassistant
Copy link

CLAassistant commented Sep 9, 2024

CLA assistant check
All committers have signed the CLA.

@djsmith85 djsmith85 linked an issue Sep 10, 2024 that may be closed by this pull request
1 task
@Tyrrrz Tyrrrz self-assigned this Sep 10, 2024
@Tyrrrz Tyrrrz added the needs-qa Marks a PR as requiring QA approval label Sep 10, 2024
@Tyrrrz
Copy link

Tyrrrz commented Sep 11, 2024

Note that the linked issue mentions:

Skip alias and identity items if it is not handled, and show skipped items count at last.

Unless I'm mistaken, I don't believe this PR introduces a way to track and report the items that were skipped.

image

Not sure if this is essential though. I believe that introducing it would require changing the whole importer chain and it will affect other providers. @djsmith85 do you have thoughts?

Other than that, I tested the changes in the Web Vault and it seems to work correctly 👍🏻

@Tyrrrz
Copy link

Tyrrrz commented Sep 11, 2024

Thank you @aliaftab612! I'll wait to clarify a few things with @djsmith85 before proceeding further with this PR 🙂

@djsmith85
Copy link
Contributor

Note that the linked issue mentions:

Skip alias and identity items if it is not handled, and show skipped items count at last.

Unless I'm mistaken, I don't believe this PR introduces a way to track and report the items that were skipped.

image Not sure if this is essential though. I believe that introducing it would require changing the whole importer chain and it will affect other providers. @djsmith85 do you have thoughts?

Other than that, I tested the changes in the Web Vault and it seems to work correctly 👍🏻

@Tyrrrz Yes, it does not address showing the skipped items, but I also agree with this being out of scope for this PR.

@aliaftab612 Nice work and thank you for your contribution!

@djsmith85 djsmith85 removed their request for review September 12, 2024 15:17
@Tyrrrz
Copy link

Tyrrrz commented Sep 12, 2024

Moving the PR to QA ✅

@Tyrrrz Tyrrrz added hold do not merge, do not approve yet and removed hold do not merge, do not approve yet needs-qa Marks a PR as requiring QA approval labels Sep 12, 2024
@Tyrrrz Tyrrrz merged commit 0f3d8a6 into bitwarden:main Sep 18, 2024
117 of 130 checks passed
@Tyrrrz
Copy link

Tyrrrz commented Sep 18, 2024

QA passed, merged. Thank you @aliaftab612!

@aliaftab612 aliaftab612 deleted the handled-protonpass-unsupported-items branch September 18, 2024 17:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Skip unhandled item types while importing ProtonPass
5 participants