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

Close #3184: Download Limit Support #3204

Merged
merged 4 commits into from
Dec 20, 2024
Merged

Close #3184: Download Limit Support #3204

merged 4 commits into from
Dec 20, 2024

Conversation

i2h3
Copy link

@i2h3 i2h3 commented Nov 22, 2024

  • Added tableDownloadLimit entity to app database.
  • Extended capability query to also consider download limit app.
  • Extended capabilities list view for display of download limit availability.
  • Extended share detail user interface to mimic the web user interface for managing download limits.
  • Every time WebDAV properties of a file are retrieved, its associated download limits are removed and recreated.

Housekeeping:

  • Outsourced NCShareDateCell into dedicated source code file.
  • Outsourced NCShareToggleCell into dedicated source code file.

Notes:

  • In my first attempt I had a detail view in the download limit row of the advanced share options showing the remaining number of downloads. However, that required to inject and retain the download limit entity object into the complicated share table configuration object. That, in turn, results in inconsistent data state due to invalid and outdated references. To resolve those issues, the assembly of the advanced share options user interface needs some refactoring which appears to expansive at this point and I prefer to leave it as it is for now.

@i2h3 i2h3 self-assigned this Nov 22, 2024
@i2h3
Copy link
Author

i2h3 commented Nov 22, 2024

This depends on the still pending merge request for required changes in NextcloudKit (nextcloud/NextcloudKit#107). Given that is integrated and released as a version, the dependency version specification in this merge request must be adjusted before merging.

@i2h3 i2h3 marked this pull request as draft November 22, 2024 12:03
@i2h3 i2h3 linked an issue Nov 26, 2024 that may be closed by this pull request
@i2h3 i2h3 force-pushed the 3184-download-limit branch from f6886b2 to 33b840f Compare November 27, 2024 16:05
@i2h3 i2h3 changed the title #3184: Download Limit Support Close #3184: Download Limit Support Nov 27, 2024
@i2h3 i2h3 marked this pull request as ready for review November 27, 2024 16:07
@i2h3 i2h3 force-pushed the 3184-download-limit branch from 33b840f to 6e07bc0 Compare December 3, 2024 08:31
i2h3 added 2 commits December 19, 2024 09:01
- Added tableDownloadLimit entity to app database.
- Extended capability query to also consider download limit app.
- Extended capabilities list view for display of download limit availability.
- Extended share detail user interface to mimic the web user interface for managing download limits.
- Every time WebDAV properties of a file are retrieved, its associated download limits are removed and recreated.

Housekeeping:
- Outsourced NCShareDateCell into dedicated source code file.
- Outsourced NCShareToggleCell into dedicated source code file.

Notes:
- In my first attempt I had a detail view in the download limit row of the advanced share options showing the remaining number of downloads. However, that required to inject and retain the download limit entity object into the complicated share table configuration object. That, in turn, results in inconsistent data state due to invalid and outdated references. To resolve those issues, the assembly of the advanced share options user interface needs some refactoring which appears to expansive at this point and I prefer to leave it as it is for now.

Signed-off-by: Iva Horn <[email protected]>
@i2h3 i2h3 force-pushed the 3184-download-limit branch from 6e07bc0 to 1965c6b Compare December 19, 2024 08:01
Copy link
Collaborator

@mpivchev mpivchev left a comment

Choose a reason for hiding this comment

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

image

I see some whitespace warnings, can you enable this if you haven't already?

@mpivchev
Copy link
Collaborator

mpivchev commented Dec 19, 2024

Can you please add an On Off label on the right side of "Limit Downloads" ?

Example:
image

@i2h3
Copy link
Author

i2h3 commented Dec 19, 2024

I see some whitespace warnings, can you enable this if you haven't already?

@mpivchev I have enabled those already. That's one of the first things I always do when setting up Xcode. 😁 But for some reason Xcode really likes its white space lines… 🙄

Can you please add an On Off label on the right side of "Limit Downloads" ?

I first had an accessory view in the share view but I remember faintly it somehow spaghettified state management, so I left it out. I will have another look into it.

@i2h3
Copy link
Author

i2h3 commented Dec 19, 2024

@mpivchev Now I remember what the problem was. It worked only partially due to the complicated UITableView setup. I had an implementation where the initial state was presented but due to the static retention of NCShareConfig, how it is passed through and the metadata and how cells are constructed it appeared to me that I can only get this through with extended refactoring which I considered out of scope.

@i2h3 i2h3 merged commit 888a216 into develop Dec 20, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Download Limit Support
2 participants