Skip to content
This repository has been archived by the owner on Jun 19, 2023. It is now read-only.

Instant upload #666

Closed
wants to merge 59 commits into from
Closed

Instant upload #666

wants to merge 59 commits into from

Conversation

Jon-Schneider
Copy link
Contributor

@Jon-Schneider Jon-Schneider commented May 24, 2016

This Pull Request Does the Following:

  1. Moves Instant Upload Management logic out of SettingsViewController into a new InstantUpload class.
  2. Separates Background Instant Upload from Instant Upload. If Background Instant Upload is disabled and Instant Upload enabled, upload just takes place when you first open the app, the app enters the foreground, or new photos are taken while the app is running in the background.
  3. Adds a migration to add a background instant upload enabled field to the users table, used to implement the user preference for Background Instant Upload.
  4. Migrates Instant Upload to use the new PHPhotoLibrary api rather than the deprecated ALAssetLibrary API for photo access. This means Instant Upload can now work independent of whether location permission has been granted to the app or not.
  5. Deletes ManageLocation class. InstantUpload.m implements background upload by subscribing to CLLocationManager significant location change updates.

NEEDS:

Translations:

  • "message_location_not_enabled" has been changed in English to "Please go to Settings and turn on Location Services to enable instant photo uploads in the background." This is the text of the alert when a user tries to enable Background Instant Upload and location permissions are denied.
  • Added string "title_background_instant_upload", which in English is "Background Instant Upload". This is the text of the Background Instant Upload cell on the SettingsViewController.

Possible Needs:

  • Will not work with iOS 7 due to use of PHPhotoLibrary. If you want iOS 7 to still be supported in the app I can some preprocessing directives that disables Instant Upload on SettingsViewController, but with iOS 10 around the corner I don't see a need to maintain a whole additional code path using ALAssetLibrary.

TASKS

nasli and others added 30 commits March 1, 2016 17:52
Setted the text color of the navigation bar on the share cells
Fixed a problem on the branding colors of the buttons on the share view
…th to use generic ones. Move temp file to local path after the upload finished
[Doc] user manual updates for 3.4.9 release
@ghost
Copy link

ghost commented May 31, 2016

User of the commit 9d503e3 is unknown - cannot determine CLA

// Get the old instant upload date so it can be migrated to an NSTimestamp
__block long lastDateInstantUpload;
[queue inDatabase:^(FMDatabase *db) {
FMResultSet *rs = [db executeQuery:@"SELECT date_instant_upload FROM users WHERE activeaccount=1"];
Copy link
Contributor

Choose a reason for hiding this comment

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

We could get also here if the instant upload was active to avoid the previous access to the DB

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@nasli Direct DB querying is necessary here because the migrations take place before the current user is initialize.

Copy link
Contributor

@nasli nasli May 31, 2016

Choose a reason for hiding this comment

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

I mean, use this query adding the instant_upload column

 [db executeQuery:@"SELECT date_instant_upload, instant_upload FROM users WHERE activeaccount=1"];

to avoid this line:

  BOOL defaultBackgroundInstantUploadValue = (BOOL)[ManageAppSettingsDB isInstantUpload];

that is doing another query to the same table.

@DeepDiver1975
Copy link
Member

@Jon-Schneider can you please add the email address of your first commit to your github profile? Otherwise the commit cannot be identified as yours - THX

@Jon-Schneider
Copy link
Contributor Author

@DeepDiver1975 Done.

@DeepDiver1975
Copy link
Member

@DeepDiver1975 Done.

THX

@nasli
Copy link
Contributor

nasli commented May 31, 2016

There are conflicts with this branch, please rebase it @Jon-Schneider

if ([PHPhotoLibrary authorizationStatus] == PHAuthorizationStatusAuthorized) {
ACTIVE_USER.instantUpload = YES;
[ManageAppSettingsDB updateInstantUploadTo:YES];
[self attemptUpload];
Copy link
Contributor

Choose a reason for hiding this comment

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

You should init here timestampInstantUpload in DB and activeUserDto with the current date, as you did with instantUpload.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So that images taken before instant upload is enabled aren't uploaded? I believe the behavior of Dropbox and similar apps (Google Photos) is to upload your entire history, but I'm not sure what OwnCloud has done in the past.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, that is how Owncloud works, only upload new photos taken, that it is the meaning of instant uploads.

Upload all the images of the camera should be a new and different feature.

javiergonzper and others added 13 commits May 31, 2016 13:34
… background upload preference has now been seperated from the InstantUpload preference. Instant Upload now uses the Photos Library instead of the Assets library for photo access."
…nagement from PrepareFilesToUpload to InstantUpload. Instant upload last uploaded timestamp is now stored as an NSTimeInterval rather than a long. Migrates the storage of last instant upload from long to NSTimeInterval in both storage and on User objects.
@Jon-Schneider
Copy link
Contributor Author

Jon-Schneider commented May 31, 2016

I just destroyed my branch with an accidental rebase off master. I'm going to quickly reimplement and re-pull.

@Jon-Schneider
Copy link
Contributor Author

Reimplemented in Pull Request #681

@nasli nasli mentioned this pull request Jun 10, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants