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-1433] Creating Item Doesn't Auto-Select Current Organization/Collection in Individual Web Vault #5302

Conversation

cagonzalezcs
Copy link
Contributor

Type of change

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

Objective

A bug was found within individual user vaults for users who are part of an organization. When adding a new item, if the user has selected to filter all items by a collection, it is expected that the new item should auto-populate the ownership of the item to the organization who set up the collection. The item should also have the selected collection as its default selected value.

This is currently not happening, and the reason identified is that on selection of the "New Item" button, we are not identifying the organization that is associated with a selected collection filter. This change introduces a fix which ensures we set the organization ID based on the selected collection ID before showing the modal element to the user. This ensures that the organization as well as the selected collection are set by default when the user is attempting to add a new item.

Code changes

  • apps/web/src/app/vault/individual-vault/vault.component.ts: Within the VaultComponent.addCipher() method we are now setting the organizationId for the AddEditComponent if the selected collection ID is not equal to AllCollections. We find the first entry with within the array of collections that contains the selected collection ID and set the organizationId associated with the found collection item.

Screenshots

Screen.Recording.2023-04-27.at.6.25.02.PM.mov

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

@cagonzalezcs cagonzalezcs added bug needs-qa Marks a PR as requiring QA approval labels Apr 27, 2023
@cagonzalezcs cagonzalezcs requested a review from a team as a code owner April 27, 2023 23:26
Copy link
Member

@differsthecat differsthecat left a comment

Choose a reason for hiding this comment

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

Great catch, this is now working for web 🎉

I know the ticket specified only web, but can you see if this is fixable for Desktop as well? Browser does not appear to be experiencing this issue.

@cagonzalezcs
Copy link
Contributor Author

@differsthecat

Sure, I'll review the implementation on the desktop client and add those changes to this PR.

@cagonzalezcs
Copy link
Contributor Author

@differsthecat

So I reviewed the codebase for the desktop application, and found that we already set the organization id when a collection is selected.

Within apps/desktop/src/vault/app/vault/vault.component.ts on line 747, we have a method VaultComponent.prefillNewCipherFromFilter() that is populating the data associated with a filter when creating a new item.

This is doing a check to identify if the collections filter is set, and populating an organization id based on that setting.

I also did a build of the desktop based on the master branch and am seeing the expected behavior In place. The desktop app should already be solid regarding expected behavior.

@differsthecat
Copy link
Member

@differsthecat

So I reviewed the codebase for the desktop application, and found that we already set the organization id when a collection is selected.

Within apps/desktop/src/vault/app/vault/vault.component.ts on line 747, we have a method VaultComponent.prefillNewCipherFromFilter() that is populating the data associated with a filter when creating a new item.

This is doing a check to identify if the collections filter is set, and populating an organization id based on that setting.

I also did a build of the desktop based on the master branch and am seeing the expected behavior In place. The desktop app should already be solid regarding expected behavior.

I just did some more testing and the issue I am seeing is a bit more nuanced than I originally thought and is not quite the same issue as the one in web!

What I'm seeing is that the desktop app is not properly clearing the last selected folder and collection when clicking on the filters between folders, collections, and the My Vault filter. I'll see if there is an existing ticket for this, or create a new one.

This PR is good to go then 🙌

Copy link
Member

@differsthecat differsthecat left a comment

Choose a reason for hiding this comment

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

Great work @cagonzalezcs !

@cagonzalezcs cagonzalezcs removed the needs-qa Marks a PR as requiring QA approval label May 1, 2023
@cagonzalezcs cagonzalezcs merged commit de078c4 into master May 1, 2023
22 of 23 checks passed
@cagonzalezcs cagonzalezcs deleted the Client-Integrations/PM-1433-creating-new-item-does-not-select-current-organization branch May 1, 2023 15:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants