[PM-950] Cannot save new login if personal ownership is disabled #3265
Conversation
| _organizationService = ServiceContainer.Resolve<IOrganizationService>("organizationService"); | ||
| _collectionService = ServiceContainer.Resolve<ICollectionService>("collectionService"); | ||
| _policyService = ServiceContainer.Resolve<IPolicyService>("policyService"); |
There was a problem hiding this comment.
⛏️ No need to use the string key.
| if (height.HasValue) | ||
| { | ||
| ContentView.AddConstraint( | ||
| NSLayoutConstraint.Create(TextField, NSLayoutAttribute.Height, NSLayoutRelation.Equal, null, NSLayoutAttribute.NoAttribute, 1f, height.Value)); | ||
| } | ||
|
|
There was a problem hiding this comment.
❓ why was this removed from here? Doesn't it break some case for the TextField?
| }, | ||
| CollectionIds = CollectionCell.SelectedItem.ToString() == AppResources.NoCollectionsToList ? | ||
| null : new HashSet<string> { _collections.ElementAtOrDefault(CollectionCell.SelectedIndex)?.Id }, | ||
| OrganizationId = OrganizationCell.SelectedItem != null ? _organizations.ElementAtOrDefault(CollectionCell.SelectedIndex)?.Id : null, |
There was a problem hiding this comment.
SelectedIndex of the OrganizationCell?
| CollectionIds = CollectionCell.SelectedItem.ToString() == AppResources.NoCollectionsToList ? | ||
| null : new HashSet<string> { _collections.ElementAtOrDefault(CollectionCell.SelectedIndex)?.Id }, |
There was a problem hiding this comment.
⛏️ Format
| CollectionIds = CollectionCell.SelectedItem.ToString() == AppResources.NoCollectionsToList ? | |
| null : new HashSet<string> { _collections.ElementAtOrDefault(CollectionCell.SelectedIndex)?.Id }, | |
| CollectionIds = CollectionCell.SelectedItem.ToString() == AppResources.NoCollectionsToList | |
| ? null | |
| : new HashSet<string> { _collections.ElementAtOrDefault(CollectionCell.SelectedIndex)?.Id }, |
| @@ -121,6 +179,19 @@ public override void ViewDidLoad() | |||
| base.ViewDidLoad(); | |||
| } | |||
|
|
|||
| private void Type_ValueChanged(object sender, EventArgs e) | |||
There was a problem hiding this comment.
🎨 Rename to something more meaningful of what this method does, like OrganizationCell_ValueChanged
| var collectionsNames = _writeableCollections.Where(c => c.OrganizationId == _organizations.ElementAt(OrganizationCell.SelectedIndex).Id).Select(s => s.Name).ToList(); | ||
| if (collectionsNames.Count == 0) | ||
| { | ||
| collectionsNames.Insert(0, AppResources.NoCollectionsToList); | ||
| } | ||
|
|
||
| CollectionCell.Items = collectionsNames; |
There was a problem hiding this comment.
🎨 Extract to method for reuse; given this is the same as in above lines
|
|
||
| _collections = _collectionService.GetAllDecryptedAsync().GetAwaiter().GetResult(); | ||
| _writeableCollections = _collections.Where(c => !c.ReadOnly).OrderBy(c => c.Name).ToList(); | ||
| var collectionsNames = _writeableCollections.Where(c => c.OrganizationId == _organizations.ElementAt(OrganizationCell.SelectedIndex).Id).Select(s => s.Name).ToList(); |
There was a problem hiding this comment.
⛏️ Format
| var collectionsNames = _writeableCollections.Where(c => c.OrganizationId == _organizations.ElementAt(OrganizationCell.SelectedIndex).Id).Select(s => s.Name).ToList(); | |
| var collectionsNames = _writeableCollections | |
| .Where(c => c.OrganizationId == _organizations.ElementAt(OrganizationCell.SelectedIndex).Id) | |
| .Select(s => s.Name) | |
| .ToList(); |
| @@ -113,6 +135,42 @@ public override void ViewDidLoad() | |||
| folderNames.Insert(0, AppResources.FolderNone); | |||
| FolderCell.Items = folderNames; | |||
|
|
|||
| _personalOwnershipPolicyApplies = _policyService.PolicyAppliesToUser(PolicyType.PersonalOwnership).GetAwaiter().GetResult(); | |||
| var index = 0; | |||
| if (_personalOwnershipPolicyApplies) | |||
There was a problem hiding this comment.
🎨 This method is pretty huge, IMO personal ownership logic should be extracted to a different method we can call here so it doesn't make this method even bigger than it is.
| OrganizationCell.Items = _organizations.Select(o => o.Name).ToList(); | ||
| OrganizationCell.ValueChanged += Type_ValueChanged; | ||
|
|
||
| _collections = _collectionService.GetAllDecryptedAsync().GetAwaiter().GetResult(); |
There was a problem hiding this comment.
🤔 Could you test if _collectionService.GetAllDecryptedAsync takes too long the app crashes? You can mock this by just adding a task delay of some seconds in the first line inside GetAllDecryptedAsync() which will be mocking the case of a really large number of collections to decrypt.
We shouldn't be using GetAwaiter().GerResult() and just put some loading on the view and performing the work asynchronously updating the table view afterwards but well I know we already have other calls that use that like Folders; so I guess it's "fine" for now as long as this loads "fast" and doesn't crash the app because ViewDidLoad took to long to finish.
Type of change
Objective
Fixes #1874
Add the ability to select a collection for the new item if personal ownership is disabled.
Code changes
_personalOwnershipPolicyApplies. Added cells to warn the user the personal ownership is disabled and allow organisation and collection selection.Screenshots
BEFORE
AFTER
Before you submit
dotnet format --verify-no-changes) (required)