Skip to content

show used space in user list - #5342

Merged
MorrisJobke merged 3 commits into
masterfrom
userlist-used-space
Jul 5, 2017
Merged

show used space in user list#5342
MorrisJobke merged 3 commits into
masterfrom
userlist-used-space

Conversation

@icewind1991

@icewind1991icewind1991 commented Jun 12, 2017

Copy link
Copy Markdown
Member

Fixes#117

Makes the quota dropdown into a progress bar, used space is shown as tooltip on hover

preview

@soamz

Copy link
Copy Markdown

I dont see it.
Is it another latest version ?

@MorrisJobke

Copy link
Copy Markdown
Member

Is it another latest version ?

This is a pull request. That means the code on ready for comments before it gets into the product.

@MorrisJobke

Copy link
Copy Markdown
Member

beside this little nitpick that looks really good 👍

MorrisJobke
MorrisJobke previously requested changes Jun 12, 2017

@MorrisJobkeMorrisJobke left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

  • tooltip is escaped weirdly:

bildschirmfoto 2017-06-12 um 11 58 54

@MorrisJobkeMorrisJobke added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Jun 12, 2017
@enoch85

enoch85 commented Jun 12, 2017

Copy link
Copy Markdown
Member

"Unlimited" will disappear right? I mean this implementation will show the same values as this PR #5305?

@enoch85

Copy link
Copy Markdown
Member

Also, would it be possible to show the % of the total used storage?

@codecov

codecovBot commented Jun 12, 2017

Copy link
Copy Markdown

Codecov Report

Merging #5342 into master will decrease coverage by 9.37%.
The diff coverage is n/a.

@@ Coverage Diff @@## master #5342 +/- ##
============================================
- Coverage 54.15% 44.77% -9.38% + Complexity 22326 1678 -20648 
============================================
Files 1380 157 -1223 Lines 85535 13426 -72109 Branches 1329 1329 ============================================
- Hits 46320 6012 -40308 + Misses 39215 7414 -31801
Impacted FilesCoverage ΔComplexity Δ
lib/private/Files/Storage/DAV.php64.6% <0%> (-0.22%)161% <0%> (ø)
apps/dav/lib/Connector/Sabre/Node.php
settings/ajax/setquota.php
lib/private/Preview/MSOfficeDoc.php
lib/private/SystemTag/SystemTag.php
apps/systemtags/appinfo/routes.php
apps/dav/lib/DAV/CustomPropertiesBackend.php
lib/private/Encryption/Keys/Storage.php
lib/private/Updater/VersionCheck.php
.../Exceptions/EncryptionHeaderKeyExistsException.php
... and 1212 more

LukasReschke
LukasReschke previously requested changes Jun 12, 2017

@LukasReschkeLukasReschke left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nice stuff and works as expected, see comments for remarks :)

$query->execute();
}

public function getUsedSpaceForUsers(array $userIds) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can I haz test? 🍪 😉

* Note that this only includes the space in their home directory,
* not any incoming shares or external storages.
*
* @param string[] $userIds

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is there a reason that this is an array of userIds instead of IUser[]? The other functions all take the user objects :)

Manager $keyManager,
IJobList $jobList) {
IJobList $jobList,
IUserMountCache $userMountCache) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Also adjust the PHPDocs 😉

@icewind1991

Copy link
Copy Markdown
MemberAuthor

@LukasReschke all fixed

$this->assertEmpty($cachedMounts);
}

public function testGtUsedSpaceForUsers() {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

testGetUsedSpaceForUsers? 😉

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

fixed

Signed-off-by: Robin Appelman <robin@icewind.nl>
@MorrisJobkeMorrisJobke added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Jun 15, 2017

@MorrisJobkeMorrisJobke left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Tested and works 👍

@linucksrox

linucksrox commented Jun 21, 2017

Copy link
Copy Markdown

I've been using this, and it works nicely 👍
I reported an issue #5470 which was a result of this pull request. Something in /settings/css/settings.css has changed causing a layout issue on the Personal configuration page. I haven't had a chance to dig more into what the fix could be, but wanted to mention this here.

Signed-off-by: Morris Jobke <hey@morrisjobke.de>
@MorrisJobke

Copy link
Copy Markdown
Member

I fixed the missing PHPDoc for the public interface. Now the CI should be fine :)

@MorrisJobkeMorrisJobke added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Jun 21, 2017
@codecov-io

codecov-io commented Jul 4, 2017

Copy link
Copy Markdown

Codecov Report

Merging #5342 into master will decrease coverage by 22.87%.
The diff coverage is 0%.

@@ Coverage Diff @@## master #5342 +/- ##
=============================================
- Coverage 54.03% 31.15% -22.88% + Complexity 22465 22463 -2 
=============================================
Files 1389 1388 -1 Lines 85957 85373 -584 Branches 1329 1329 =============================================
- Hits 46449 26602 -19847 - Misses 39508 58771 +19263
Impacted FilesCoverage ΔComplexity Δ
settings/Controller/UsersController.php0% <0%> (-69.2%)114 <20> (+2)
settings/templates/users/part.userlist.php0% <0%> (ø)0 <0> (ø)⬇️
lib/private/Files/Config/UserMountCache.php3.44% <0%> (-85.69%)39 <2> (-1)
apps/files_versions/lib/Command/Expire.php0% <0%> (-100%)3% <0%> (ø)
lib/private/DB/OCSqlitePlatform.php0% <0%> (-100%)5% <0%> (ø)
apps/user_ldap/lib/Migration/UUIDFix.php0% <0%> (-100%)5% <0%> (ø)
apps/user_ldap/lib/Mapping/GroupMapping.php0% <0%> (-100%)1% <0%> (ø)
apps/provisioning_api/lib/AppInfo/Application.php0% <0%> (-100%)6% <0%> (ø)
apps/user_ldap/lib/BackendUtility.php0% <0%> (-100%)1% <0%> (ø)
lib/private/DB/SQLiteSessionInit.php0% <0%> (-100%)4% <0%> (ø)
... and 373 more

@MorrisJobke

Copy link
Copy Markdown
Member

I fixed the not updated unit tests 😉

@MorrisJobke
MorrisJobkeforce-pushed the userlist-used-space branch 3 times, most recently from 01da4b6 to 4b518f5CompareJuly 5, 2017 09:27
Signed-off-by: Morris Jobke <hey@morrisjobke.de>
@MorrisJobke
MorrisJobke merged commit 7d58bb7 into masterJul 5, 2017
@MorrisJobke
MorrisJobke deleted the userlist-used-space branch July 5, 2017 13:15
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

4. to releaseReady to be released and/or waiting for tests to finish

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants

@icewind1991@soamz@MorrisJobke@enoch85@linucksrox@codecov-io@LukasReschke