clients icon indicating copy to clipboard operation
clients copied to clipboard

[PS-1478] Fix #3169 - The Title value in Identity-type vault items does not carry over to clients using different languages

Open schang1146 opened this issue 2 years ago • 1 comments

Type of change

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

Objective

Fix for #3169 where an identity's "Title" field isn't being respected if language preference changes.

Code changes

  • libs/angular/src/components/add-edit.component.ts I changed the values of the options to be the title's key values ("mr", "mrs", "ms", "dr") so that the field would always be able to grab an option regardless of language preference change.

IMPORTANT: One concern I had with this implementation is that it'd break ALL current identities to show blank (regardless of language) until a user updates it. Not sure if there is the option to update all currently saved values with their corresponding key values ("mr", "mrs", "ms", "dr") or if another fix would be preferred. Just thought this solution would be the simplest in the long run.

Screenshots

(Sorry for awkward window cropping lol first time trying this out) https://user-images.githubusercontent.com/20074034/190029456-486a3dc8-7420-4cfc-9f2b-a80fd56a56ed.mp4

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

schang1146 avatar Sep 13 '22 23:09 schang1146

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

bitwarden-bot avatar Sep 13 '22 23:09 bitwarden-bot

FYI/mental-note: The same issue is present in the mobile app -> CipherAddEditPageViewModel.cs

djsmith85 avatar Sep 24 '22 18:09 djsmith85

I think I understand the ask but I think I'm confused about the suggested code block.

Say a user already created an Identity object with the title "Srta" (while their language was set to Español). If we are trying to convert that CipherView property value in the load()-method in add-edit.component.ts to the global "ms" after the user has switched their language preference to say English for example, none of the if-statement matches would ever be hit. In my example, the cipher.identity.title would be "Srta" but i18nService.t("ms") would be "Ms" since it doesn't know what the user had as their language preference when setting the Title property.

I'll keep thinking about it but was a bit blocked when trying to get that fixIdentityTitle util function to work.

schang1146 avatar Sep 29 '22 04:09 schang1146

I think I understand the ask but I think I'm confused about the suggested code block.

Say a user already created an Identity object with the title "Srta" (while their language was set to Español). If we are trying to convert that CipherView property value in the load()-method in add-edit.component.ts to the global "ms" after the user has switched their language preference to say English for example, none of the if-statement matches would ever be hit. In my example, the cipher.identity.title would be "Srta" but i18nService.t("ms") would be "Ms" since it doesn't know what the user had as their language preference when setting the Title property.

I'll keep thinking about it but was a bit blocked when trying to get that fixIdentityTitle util function to work.

The code-block I came up with was not intended as a catch-all. I was assuming a user was still in the selected language they originally saved the entry in and this would fix the saved value ("Srta" to "Ms"(untranslated key)). The majority case. If a user already changed languages then there won't be much we can do to fix this, as we don't know what the previously selected user-language was. In this case the originally reported bug (#3169) of it showing up empty would occur, but would be easy to fix by the user.

I'll close this PR for now, as there is no work to review, but feel free to update the PR and we'll re-open it.

djsmith85 avatar Oct 04 '22 18:10 djsmith85