Show shared user by tobiasKaminsky · Pull Request #3320 · nextcloud/android

tobiasKaminsky · 2018-12-05T12:16:48Z

Shows shared users on file list.

Needs feedback from nextcloud/server#12874

AndyScherzinger · 2018-12-07T07:19:21Z

+ private String ownerId;
+ @Getter
+ @Setter
+ private String ownerDisplayName;


should be formatted as single lines

Damn Android Studio is always changing this.
I had to do this on a different branch without AndroidStudio… :/

Please see #3325 for this

AndyScherzinger · 2018-12-07T07:19:33Z

+ try {
+ return TextDrawable.createAvatar(mAccount.name, mAvatarRadius);
+ } catch (NoSuchAlgorithmException e) {
+ e.printStackTrace();


should be a proper log statement

AndyScherzinger · 2018-12-07T07:21:26Z

 if (holder instanceof OCFileListItemViewHolder) {
 OCFileListItemViewHolder itemViewHolder = (OCFileListItemViewHolder) holder;

+ if (!TextUtils.isEmpty(file.getOwnerId()) && !mAccount.name.split("@")[0].equals(file.getOwnerId())) {


we should probably move the split method based on the "@" into a utils class (not sure if we already have one for account/name things since I believe we do splitting based on the @ sign already in some other places so we should centralize that part.

AndyScherzinger · 2018-12-07T07:21:47Z

 }
- sharedIconView.setOnClickListener(view -> ocFileListFragmentInterface.onShareIconClick(file));
+
+ if (!TextUtils.isEmpty(file.getOwnerId()) && !mAccount.name.split("@")[0].equals(file.getOwnerId())) { // TODO refactor


see comment abpve regarding handdling of @

AndyScherzinger · 2018-12-07T07:22:16Z

+
+ @Override
+ public boolean shouldCallGeneratedCallback(String tag, Object callContext) {
+// return ((ImageView) callContext).getTag().equals(tag);


remove commented out code in case we alsways return false

AndyScherzinger · 2018-12-07T07:22:52Z

 fileSize = itemView.findViewById(R.id.file_size);
 lastModification = itemView.findViewById(R.id.last_mod);
 overflowMenu = itemView.findViewById(R.id.overflow_menu);
+ sharedAvatar = itemView.findViewById(R.id.sharedAvatar);


we should replace this with butterknife :)

AndyScherzinger · 2018-12-07T07:23:43Z

 }

+ private void setShareWithYou() {
+ if (!TextUtils.isEmpty(file.getOwnerId()) && !account.name.split("@")[0].equals(file.getOwnerId())) {


see comment about creation of a split @ method

AndyScherzinger · 2018-12-07T07:25:38Z


 private static final String ARG_FILE = "FILE";
 private static final String ARG_ACCOUNT = "ACCOUNT";
+ private static final String TAG = FileDetailSharingFragment.class.getSimpleName();


TAG isn't used atm

AndyScherzinger · 2018-12-07T07:26:16Z

did an initial review of the code, just some minor things / cosmetics :)

codecov · 2019-02-26T10:19:23Z

Codecov Report

Merging #3320 into master will increase coverage by 0.17%.
The diff coverage is 12.5%.

@@ Coverage Diff @@## master #3320 +/- ##
===========================================
+ Coverage 6% 6.18% +0.17% 
Complexity 1 1 ===========================================
Files 318 317 -1 Lines 31024 30575 -449 Branches 4447 4394 -53 ===========================================
+ Hits 1863 1890 +27 + Misses 28884 28400 -484 - Partials 277 285 +8

Impacted Files	Coverage Δ	Complexity Δ
...ain/java/com/owncloud/android/db/ProviderMeta.java	`88% <ø> (ø)`	`0 <0> (ø)`	⬇️
...a/com/owncloud/android/utils/FileStorageUtils.java	`12.4% <0%> (+4.28%)`	`0 <0> (ø)`	⬇️
...owncloud/android/ui/adapter/OCFileListAdapter.java	`0% <0%> (ø)`	`0 <0> (ø)`	⬇️
...loud/android/datamodel/ThumbnailsCacheManager.java	`15.44% <0%> (+0.36%)`	`0 <0> (ø)`	⬇️
...wncloud/android/providers/FileContentProvider.java	`15.44% <0%> (+0.16%)`	`0 <0> (ø)`	⬇️
...android/ui/fragment/FileDetailSharingFragment.java	`0% <0%> (ø)`	`0 <0> (ø)`	⬇️
...ncloud/android/ui/fragment/OCFileListFragment.java	`0% <0%> (ø)`	`0 <0> (ø)`	⬇️
...in/java/com/owncloud/android/datamodel/OCFile.java	`59.19% <100%> (+0.2%)`	`0 <0> (ø)`	⬇️
...loud/android/datamodel/FileDataStorageManager.java	`10.61% <50%> (+0.08%)`	`0 <0> (ø)`	⬇️
.../third_parties/daveKoeller/AlphanumComparator.java	`82.14% <0%> (-1.2%)`	`0% <0%> (ø)`
... and 180 more

codecov · 2019-02-26T10:19:23Z

Codecov Report

Merging #3320 into master will decrease coverage by <.01%.
The diff coverage is 7.86%.

@@ Coverage Diff @@## master #3320 +/- ##
===========================================
- Coverage 6% 6% -0.01% 
Complexity 1 1 ===========================================
Files 318 318 Lines 31008 31077 +69 Branches 4448 4455 +7 ===========================================
+ Hits 1862 1866 +4 - Misses 28869 28934 +65 
Partials 277 277

Impacted Files	Coverage Δ	Complexity Δ
...ain/java/com/owncloud/android/db/ProviderMeta.java	`88% <ø> (ø)`	`0 <0> (ø)`	⬇️
...a/com/owncloud/android/utils/FileStorageUtils.java	`8.04% <0%> (-0.09%)`	`0 <0> (ø)`
.../owncloud/android/authentication/AccountUtils.java	`23.77% <0%> (-0.2%)`	`0 <0> (ø)`
...owncloud/android/ui/adapter/OCFileListAdapter.java	`0% <0%> (ø)`	`0 <0> (ø)`	⬇️
...wncloud/android/providers/FileContentProvider.java	`15.3% <0%> (-0.21%)`	`0 <0> (ø)`
...android/ui/fragment/FileDetailSharingFragment.java	`0% <0%> (ø)`	`0 <0> (ø)`	⬇️
...loud/android/datamodel/ThumbnailsCacheManager.java	`15.05% <0%> (-0.03%)`	`0 <0> (ø)`
...in/java/com/owncloud/android/datamodel/OCFile.java	`59.17% <100%> (+0.18%)`	`0 <0> (ø)`	⬇️
...loud/android/datamodel/FileDataStorageManager.java	`10.78% <50%> (+0.25%)`	`0 <0> (ø)`	⬇️
.../third_parties/daveKoeller/AlphanumComparator.java	`82.14% <0%> (-1.2%)`	`0% <0%> (ø)`

tobiasKaminsky · 2019-02-27T06:42:42Z

2 findbugs increased:

onBindView is longer/complexer and therefore generates a warning
AccountUtils.accountOwnsFile:
STT_STRING_PARSING_A_FIELD: This method parses a String that is a field This method calls a parsing method (indexOf, lastIndexOf, startsWith, endsWith, substring, indexOf) on a String that is a field, or comes from a collection that is a field. This implies that the String in question is holding multiple parts of information inside the string, which would be more maintainable and type safe if that value was a true collection or a first class object with fields, rather than a String.
This is referring to account.name, which we split to get the userId.
We could use:

AccountManager accountManager = AccountManager.get(context);
String userId = accountManager.getUserData(account,
com.owncloud.android.lib.common.accounts.AccountUtils.Constants.KEY_USER_ID);

But this is not that lightweight like a string operation.

nextcloud-android-bot · 2019-02-27T06:52:20Z

Lint

Type	master	PR
Warnings	68	68
Errors	0	0

FindBugs (new)

Warning Type	Number
Bad practice Warnings	33
Correctness Warnings	120
Internationalization Warnings	15
Malicious code vulnerability Warnings	5
Multithreaded correctness Warnings	9
Performance Warnings	119
Security Warnings	56
Dodgy code Warnings	126
Total	483

FindBugs (master)

Warning Type	Number
Bad practice Warnings	33
Correctness Warnings	120
Internationalization Warnings	15
Malicious code vulnerability Warnings	5
Multithreaded correctness Warnings	9
Performance Warnings	119
Security Warnings	56
Dodgy code Warnings	124
Total	481

nextcloud-android-bot · 2019-02-27T09:06:17Z

Lint

Type	master	PR
Warnings	68	68
Errors	0	0

FindBugs (new)

Warning Type	Number
Bad practice Warnings	33
Correctness Warnings	120
Internationalization Warnings	15
Malicious code vulnerability Warnings	5
Multithreaded correctness Warnings	9
Performance Warnings	119
Security Warnings	56
Dodgy code Warnings	126
Total	483

FindBugs (master)

Warning Type	Number
Bad practice Warnings	33
Correctness Warnings	120
Internationalization Warnings	15
Malicious code vulnerability Warnings	5
Multithreaded correctness Warnings	9
Performance Warnings	119
Security Warnings	56
Dodgy code Warnings	124
Total	481

tobiasKaminsky · 2019-02-27T09:43:34Z

@AndyScherzinger ready for review.
Although you did this some time ago, please have a look again, as there were quite some changes.

AndyScherzinger · 2019-02-27T09:55:18Z

Although you did this some time ago, please have a look again, as there were quite some changes.

I tested it, is the idea here to just show avatars from the user who shared it with me?

tobiasKaminsky · 2019-02-27T10:06:39Z

I tested it, is the idea here to just show avatars from the user who shared it with me?

show avatar in list, hide share button as this behaves the same way
show avatar and info in sharing view

Signed-off-by: tobiasKaminsky <tobias@kaminsky.me>

AndyScherzinger · 2019-02-27T10:34:08Z

both cases work just fine 👍

nextcloud-android-bot · 2019-02-27T10:54:54Z

Lint

Type	master	PR
Warnings	68	68
Errors	0	0

FindBugs (new)

Warning Type	Number
Bad practice Warnings	33
Correctness Warnings	120
Internationalization Warnings	15
Malicious code vulnerability Warnings	5
Multithreaded correctness Warnings	9
Performance Warnings	119
Security Warnings	56
Dodgy code Warnings	126
Total	483

FindBugs (master)

Warning Type	Number
Bad practice Warnings	33
Correctness Warnings	120
Internationalization Warnings	15
Malicious code vulnerability Warnings	5
Multithreaded correctness Warnings	9
Performance Warnings	119
Security Warnings	56
Dodgy code Warnings	124
Total	481

tobiasKaminsky added the 2. developing label Dec 5, 2018

tobiasKaminsky force-pushed the showSharedUser branch from 54edd19 to 1bdca81Compare December 6, 2018 07:26

nextcloud deleted a comment Dec 7, 2018

AndyScherzinger reviewed Dec 7, 2018
View reviewed changes

tobiasKaminsky mentioned this pull request Dec 7, 2018
Export code style #3325
Merged

nextcloud deleted a comment Dec 8, 2018

tobiasKaminsky force-pushed the showSharedUser branch from 191e12a to 98830b6Compare February 26, 2019 10:19

This was referenced Feb 26, 2019
Show note #3121
Merged
Combine multiple sharees #3679
Closed

tobiasKaminsky force-pushed the showSharedUser branch from 98830b6 to 9602accCompare February 26, 2019 12:44

nextcloud deleted a comment Feb 26, 2019

tobiasKaminsky force-pushed the showSharedUser branch from 9602acc to 2c884ddCompare February 27, 2019 06:24

tobiasKaminsky added 3. to review and removed 2. developing labels Feb 27, 2019

tobiasKaminsky force-pushed the showSharedUser branch from 2c884dd to 632ea2dCompare February 27, 2019 06:44

nextcloud deleted a comment Feb 27, 2019

AndyScherzinger self-requested a review February 27, 2019 09:56

AndyScherzinger previously approved these changes Feb 27, 2019
View reviewed changes

Show shared user
bf86cf6
Signed-off-by: tobiasKaminsky <tobias@kaminsky.me>

tobiasKaminsky dismissed AndyScherzinger’s stale review via bf86cf6February 27, 2019 10:10

tobiasKaminsky force-pushed the showSharedUser branch from 632ea2d to bf86cf6Compare February 27, 2019 10:10

AndyScherzinger approved these changes Feb 27, 2019
View reviewed changes

AndyScherzinger merged commit 5b0a352 into masterFeb 27, 2019

AndyScherzinger deleted the showSharedUser branch February 27, 2019 10:59

AndyScherzinger added this to the Nextcloud App 3.6.0 milestone Feb 27, 2019

nextcloud deleted a comment Feb 27, 2019

Uh oh!

Uh oh!

Conversation

tobiasKaminsky commented Dec 5, 2018• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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!

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!

AndyScherzinger commented Dec 7, 2018• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecovBot commented Feb 26, 2019

Codecov Report

Uh oh!

codecovBot commented Feb 26, 2019• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

tobiasKaminsky commented Feb 27, 2019

Uh oh!

nextcloud-android-bot commented Feb 27, 2019

Lint

FindBugs (new)

FindBugs (master)

Uh oh!

nextcloud-android-bot commented Feb 27, 2019

Lint

FindBugs (new)

FindBugs (master)

Uh oh!

tobiasKaminsky commented Feb 27, 2019

Uh oh!

AndyScherzinger commented Feb 27, 2019

Uh oh!

tobiasKaminsky commented Feb 27, 2019

Uh oh!

AndyScherzinger commented Feb 27, 2019

Uh oh!

nextcloud-android-bot commented Feb 27, 2019

Lint

FindBugs (new)

FindBugs (master)

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

tobiasKaminsky commented Dec 5, 2018•
edited
Loading

AndyScherzinger commented Dec 7, 2018•
edited
Loading

codecovBot commented Feb 26, 2019•
edited
Loading