Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Show the used quota inside the app #337

Merged
merged 14 commits into from
Apr 4, 2019
Merged

Show the used quota inside the app #337

merged 14 commits into from
Apr 4, 2019

Conversation

mneuwert
Copy link
Contributor

@mneuwert mneuwert commented Mar 28, 2019

Description

  • Changed SDK which retrieves quota if available and stores it in the instance of OCItem
  • Changed the UI to show quota in the prompt of the navigation bar
  • Right now space occupied by the files in the current folder is shown as bytes used information

Related Issue

#228

Motivation and Context

As a user I want to see how much storage space is already in use

How Has This Been Tested?

Screenshots (if appropriate):

out

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

Test plan

https://github.com/owncloud/QA/blob/master/Mobile/iOS-app/User%20Quota.md

Bugs & improvements

@mneuwert
Copy link
Contributor Author

@michaelstingl @jesmrec please check out the video showing how current implementation is working. As you can see if user navigates through the folder structure the quota in use changes and the value is corresponding to the total size of files in that particular folder, so only in the root folder you would see the total space allocated by the files of the user. I think on one hand it is useful information (per folder total size) but on the other hand I am not sure if it is intuitive. Also any other suggestions / wishes / ideas are welcome

@mneuwert mneuwert self-assigned this Mar 28, 2019
@mneuwert mneuwert added this to the 1.1.0-Next milestone Mar 28, 2019
@hosy
Copy link
Collaborator

hosy commented Mar 28, 2019

@mneuwert thank you for your implementation. I thought the quota should shown in the account list, below the Server URL. Your current implementation can be misunderstood, because user can think the used quota is the size of the current folder. But I like the idea. Maybe you can change it, to only show the size of the folder without "of 5,21 GB" -> "679 kb used" and display the total used size of quota in the account list.

Maybe it is possible to use the new custom OCQuery by @felix-schwarz to get the total size of Documents, Photos, Videos, Others, (...) like the iCloud Storage overview:

iCloud Storage

@michaelstingl
Copy link
Contributor

Vertical space is valuable. Not sure if we should show it on top of every folder. But quota could be different in every folder, so we could show it with the folder-info…

@felix-schwarz
Copy link
Contributor

A few thoughts:

  • I think of quota as a global number. I find it confusing when I start in the root folder with "161.9 MB of 5.21 GB used" and then get to see "135.6 MB of 5.21 GB used" in the Documents folder. The latter gives me the impression that I have more free space available than I actually have.
  • If more than one quota is possible (I guess in shared folders (cc @michaelstingl)?), then we should have a concept of "quota roots", so that I get to see the quota numbers of the "quota root". So f.ex. "161.9 MB of 5.21 GB used" for all directories in my account and then "20 MB of 100 MB" for the shared folder with quota and all its subfolders.
  • thinking of where to put the quota, I have no favorite, but also think that it shouldn't occupy vertical space permanently (unless, maybe, the remaining space is less than a percentage or fixed amount of total storage). Maybe a cell at the end of the table view? Or a new "Show quota" action that shows a nice table that lists all "quota roots" and their available and total space.

mneuwert and others added 2 commits April 1, 2019 15:38
@codecov
Copy link

codecov bot commented Apr 2, 2019

Codecov Report

Merging #337 into master will increase coverage by 0.07%.
The diff coverage is 71.42%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #337      +/-   ##
==========================================
+ Coverage   30.25%   30.32%   +0.07%     
==========================================
  Files         234      234              
  Lines       15910    15938      +28     
==========================================
+ Hits         4813     4833      +20     
- Misses      11097    11105       +8
Impacted Files Coverage Δ
ownCloud/SDK Extensions/OCItem+Extension.swift 79.05% <0%> (-3.01%) ⬇️
ownCloud/Theming/NSObject+ThemeApplication.swift 88.4% <100%> (+0.08%) ⬆️
ownCloud/Client/ClientQueryViewController.swift 33.01% <95%> (+1.53%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d2d2b62...2fe289a. Read the comment docs.

@codecov
Copy link

codecov bot commented Apr 2, 2019

Codecov Report

Merging #337 into master will increase coverage by 0.07%.
The diff coverage is 71.42%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #337      +/-   ##
==========================================
+ Coverage   30.25%   30.32%   +0.07%     
==========================================
  Files         234      234              
  Lines       15910    15938      +28     
==========================================
+ Hits         4813     4833      +20     
- Misses      11097    11105       +8
Impacted Files Coverage Δ
ownCloud/SDK Extensions/OCItem+Extension.swift 79.05% <0%> (-3.01%) ⬇️
ownCloud/Theming/NSObject+ThemeApplication.swift 88.4% <100%> (+0.08%) ⬆️
ownCloud/Client/ClientQueryViewController.swift 33.01% <95%> (+1.53%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d2d2b62...ee16ffa. Read the comment docs.

@jesmrec
Copy link
Contributor

jesmrec commented Apr 2, 2019

(1) [FIXED]

  • Root folder is not showing the quota:

Screen Shot 2019-04-02 at 10 57 45

@codecov-io
Copy link

codecov-io commented Apr 2, 2019

Codecov Report

❗ No coverage uploaded for pull request base (master@82c42ec). Click here to learn what that means.
The diff coverage is 76.47%.

Impacted file tree graph

@@           Coverage Diff            @@
##             master    #337   +/-   ##
========================================
  Coverage          ?     30%           
========================================
  Files             ?     234           
  Lines             ?   16316           
  Branches          ?       0           
========================================
  Hits              ?    4896           
  Misses            ?   11420           
  Partials          ?       0
Impacted Files Coverage Δ
ownCloud/Client/ClientQueryViewController.swift 32.73% <76.47%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 82c42ec...99820a9. Read the comment docs.

@jesmrec jesmrec requested a review from hosy April 3, 2019 14:43
@jesmrec
Copy link
Contributor

jesmrec commented Apr 3, 2019

(2)

I checked the different cases with some findings:

  • An user with Default quota displays in root folder:

Screenshot 2019-04-03 at 18 34 20

the propfind returns:

<d:quota-used-bytes>348737755</d:quota-used-bytes>
<d:quota-available-bytes>-3</d:quota-available-bytes>

quota-used-bytes is the number of bytes used in the account, but if you display in MB, the value is not correctly transformed, (348737755 bytes are 332.6 MB)

in case of -3 nothing should be displayed as limit, like 332.6 MB used

  • An user with limited quota. For example, 1GB:

Screenshot 2019-04-03 at 18 10 23

in such case the propfind responses something like

<d:quota-used-bytes>348737755</d:quota-used-bytes>
<d:quota-available-bytes>725004069</d:quota-available-bytes>

The result should be something like: 332.6 MB of 1 GB used

  • An user with Unlimited quota:

Screenshot 2019-04-03 at 18 12 48

same case as Default

  • An user with Other quota:

same case as limited quota.

@jesmrec jesmrec self-assigned this Apr 3, 2019
@mneuwert
Copy link
Contributor Author

mneuwert commented Apr 3, 2019

@jesmrec strictly speaking, the MB calculation is correct. There was an effort to unify data amount units, and actually as in your comment 348737755 bytes are 332.6 MiB (Mebibyte) but not MB (Megabyte). macOS I think is used mixed formats and decimal style (SI prefixes) is used to reflect how much storage is occupied by files and the "old" style (where KB is 1024 Bytes) is used to e.g. show how much memory is used say by a running process.

@jesmrec
Copy link
Contributor

jesmrec commented Apr 3, 2019

thanks for the explanation @mneuwert. Calculation was already fixed.

@jesmrec
Copy link
Contributor

jesmrec commented Apr 3, 2019

Everything fixed. Approved.

@hosy hosy removed the discussion label Apr 3, 2019
- Update ClientQueryViewController to updated OCCore.rootQuota property layout
- Make the footer show always (fixes issues where new accounts would first show separator lines - and then suddenly get replaced with the quota)
- Make quota label use correct color (the previous was unreadable with the Classic theme)
@CLAassistant
Copy link

CLAassistant commented Apr 3, 2019

CLA assistant check
Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
2 out of 3 committers have signed the CLA.

✅ hosy
✅ mneuwert
❌ felix-schwarz
You have signed the CLA already but the status is still pending? Let us recheck it.

@jesmrec jesmrec added the Approved by QA Approved by QA label Apr 4, 2019
@hosy hosy merged commit 6583d9f into master Apr 4, 2019
@delete-merged-branch delete-merged-branch bot deleted the feature/user_quota branch April 4, 2019 08:03
@jesmrec jesmrec mentioned this pull request May 3, 2019
45 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants