Add ability to clone personal vault items by vincentsalucci · Pull Request #734 · bitwarden/android

vincentsalucci · 2020-02-16T19:20:00Z

Objective

Add ability to clone personal vault items on both iOS and Android
Place 'Clone' option in the respective action sheet/dropdown
Do not allow cloning of items owned by the organization
Attachments, Delete, and Share are not allowable actions during the clone flow
Allow ownership and collection assignment

Code Changes

AddEditPage.xaml: Adjusted conditional for when to show ownership options.
AddEditPage.xaml.cs: Added a clone mode boolean along with an instance of ViewPage into the constructor. Assigned above fields to the view model instance and adjusted visibility of toolbar items.
AddEditPageViewModel.cs: Added CloneMode property. Added AllowOwnershipChanges property that includes clone mode and preservers original conditional of not being in edit mode. Adjusted ShowCollections property to include clone mode. Added ViewPage property that is used to update existing instance of the ViewPage with the newly clone cipher ID. Adjusted load/submit procedures to handle Clone mode existence.
ViewPage.xaml: Added 'Clone' toolbar item
ViewPage.xaml.cs: Added Clone_Clicked function which is triggered via toolbar item click. Adjusted toolbar items when More_Clicked is fired and when AdjustToolbar is called.
AppResources.resx: Added 'Clone' string

Android Images

iOS Images

vincentsalucci · 2020-02-16T19:22:42Z

@kspearrin The majority of the changes within this PR are format changes. I ran the basic formatting command within Rider and the shown output is the result: we may need to link up our formatting settings (I know @mportune-bw has been slowly crafting his setup in response to your feedback).

AppResources.Designer - I did not manually touch this file. Is this something that needs to be ignored or is this expected when adjusting a string?

vincentsalucci · 2020-02-16T19:32:17Z

src/App/Pages/Vault/AddEditPage.xaml.cs

-            AppOptions appOptions = null)
+            AppOptions appOptions = null,
+            bool cloneMode = false,
+            ViewPage viewPage = null)


@kspearrin Is this how you envisioned the instance of ViewPage being passed into the constructor?

Yes, looks good

kspearrin

A lot of formatting issues. I stopped commenting on them after I realized there were so many. We need to figure this out. Hard to see where the real changes are.

kspearrin · 2020-02-17T04:05:11Z

src/App/Pages/Vault/AddEditPage.xaml.cs

-            var options = new List<string> { AppResources.Attachments };
-            if(_vm.EditMode)
+
+            var options = new List<string> {AppResources.Attachments};


This formatting change isn't correct. Did your IDE do this?

kspearrin · 2020-02-17T04:06:05Z

src/App/Pages/Vault/AddEditPage.xaml.cs

                    else if(Device.RuntimePlatform == Device.Android &&
-                        !_deviceActionService.AutofillAccessibilityServiceRunning() &&
-                        !_deviceActionService.AutofillServiceEnabled())
+                            !_deviceActionService.AutofillAccessibilityServiceRunning() &&


same, shouldn't format like this

kspearrin · 2020-02-17T04:06:25Z

src/App/Pages/Vault/AddEditPage.xaml.cs

are you adding all of these new lines?

kspearrin · 2020-02-17T04:06:54Z

src/App/Pages/Vault/AddEditPageViewModel.cs

-                {
-                    nameof(ShowPasswordIcon)
-                });
+                additionalPropertyNames: new string[] {nameof(ShowPasswordIcon)});


we need to fix all of this formatting. this isn't right

kspearrin · 2020-02-17T04:07:43Z

src/App/Pages/Vault/AddEditPageViewModel.cs

+
+        public bool ShowCollections => (!EditMode || CloneMode) && Cipher.OrganizationId != null;
        public bool EditMode => !string.IsNullOrWhiteSpace(CipherId);
+        public bool ShowOwnershipOptions => (!EditMode || CloneMode);


Don't need parens here

kspearrin · 2020-02-17T04:11:44Z

AppResources.Designer - I did not manually touch this file. Is this something that needs to be ignored or is this expected when adjusting a string?

Yes, that is expected.

kspearrin · 2020-02-18T17:46:06Z

src/App/Pages/Vault/AddEditPageViewModel.cs

                {
+                    if(CloneMode && ViewPage != null)
+                    {
+                        ViewPage.ViewModel.CipherId = this.Cipher.Id;


Let's make a public method on the ViewPage that lets you pass in the new id. Then you can set the ViewModel and the private _cipherId member as well.

kspearrin · 2020-02-18T17:47:49Z

src/App/Resources/AppResources.resx

    -->
-  <xsd:schema id="root" xmlns="" xmlns:xsd="http://www.w3.org/2001/XMLSchema" xmlns:msdata="urn:schemas-microsoft-com:xml-msdata">
-    <xsd:import namespace="http://www.w3.org/XML/1998/namespace" />
+  <xsd:schema id="root" xmlns="" xmlns:xsd="http://www.w3.org/2001/XMLSchema"


Not sure why this formatting all changed. I guess something Rider did? Does Rider have a resx editor or did you have to modify this file manually?

Rider did do this, but I believe its because it exceeded the line length.

…er formatter

kspearrin · 2020-02-18T20:17:02Z

src/App/Pages/Vault/ViewPage.xaml.cs


        public ViewPageViewModel ViewModel => _vm;

+        public void updateCipherId(string cipherId)


Method names in c# are PascalCased.

kspearrin · 2020-02-18T20:29:46Z

src/App/Pages/Vault/AddEditPageViewModel.cs

                {
+                    if(CloneMode)
+                    {
+                        ViewPage?.updateCipherId(this.Cipher.Id);


need to update it here too. this wont compile anymore.

Add clone ability to personal vault items

6af0d53

vincentsalucci requested a review from kspearrin February 16, 2020 19:20

vincentsalucci added enhancement android labels Feb 16, 2020

vincentsalucci commented Feb 16, 2020

View reviewed changes

kspearrin requested changes Feb 17, 2020

View reviewed changes

Fixed formatter

38b3c56

vincentsalucci requested a review from kspearrin February 18, 2020 17:40

kspearrin requested changes Feb 18, 2020

View reviewed changes

Made requested changes and removed some extra whitespace added by Rid…

23d8783

…er formatter

vincentsalucci requested a review from kspearrin February 18, 2020 20:00

kspearrin requested changes Feb 18, 2020

View reviewed changes

vincentsalucci added 2 commits February 18, 2020 14:22

Removed formatting on AppResources file

9f3930c

Fixed casing on UpdateCipherId method

f2d1ca1

kspearrin requested changes Feb 18, 2020

View reviewed changes

Update calling method

4d4f8d3

kspearrin approved these changes Feb 18, 2020

View reviewed changes

vincentsalucci merged commit 36fb23d into master Feb 18, 2020

delete-merged-branch bot deleted the feature-clone-item-android branch February 18, 2020 21:48

vvolkgang pushed a commit that referenced this pull request Jun 20, 2024

Add overflow options to the listing screen and the search screen (#734)

336e8a9

vvolkgang pushed a commit that referenced this pull request Jun 20, 2024

Add overflow options to the listing screen and the search screen (#734)

ed38cd6


		public ViewPageViewModel ViewModel => _vm;

		public void updateCipherId(string cipherId)

Conversation

vincentsalucci commented Feb 16, 2020

Objective

Code Changes

Android Images

iOS Images

Uh oh!

vincentsalucci commented Feb 16, 2020

Uh oh!

vincentsalucci Feb 16, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kspearrin left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kspearrin commented Feb 17, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

vincentsalucci Feb 16, 2020 •

edited

Loading