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.

Sign up for GitHub

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

Provide Better Feedback For Failed Uploads #12228

Merged
merged 9 commits into from
Dec 6, 2023
Merged

Provide Better Feedback For Failed Uploads #12228

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
ca9c3f9
Add feedback for missing files
alperozturk96 Dec 4, 2023
98136b9
Add action to go upload list for file conflict notification
alperozturk96 Dec 4, 2023
04e4938
Add fixme
alperozturk96 Dec 4, 2023
d2278a3
Use existing snackbar
alperozturk96 Dec 4, 2023
e80b477
Add feedback
alperozturk96 Dec 4, 2023
21ceeab
Fix kotlin spotless check
alperozturk96 Dec 4, 2023
64e73c7
Merge master
alperozturk96 Dec 4, 2023
39ce4c4
Change content intent
alperozturk96 Dec 5, 2023
34881ce
Merge master
alperozturk96 Dec 6, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 54 additions & 24 deletions app/src/main/java/com/nextcloud/client/jobs/FilesUploadWorker.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
package com.nextcloud.client.jobs

import android.accounts.Account
import android.annotation.SuppressLint
import android.app.NotificationManager
import android.app.PendingIntent
import android.content.Context
Expand Down Expand Up @@ -253,9 +254,54 @@ class FilesUploadWorker(
// TODO generalize for automated uploads
}

private fun createConflictResolveAction(context: Context, uploadFileOperation: UploadFileOperation): PendingIntent {
val intent = ConflictsResolveActivity.createIntent(
uploadFileOperation.file,
uploadFileOperation.user,
uploadFileOperation.ocUploadId,
Intent.FLAG_ACTIVITY_CLEAR_TOP,
context
)

return if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.S) {
PendingIntent.getActivity(context, 0, intent, PendingIntent.FLAG_MUTABLE)
} else {
PendingIntent.getActivity(
context,
0,
intent,
PendingIntent.FLAG_ONE_SHOT or PendingIntent.FLAG_IMMUTABLE
)
}
}

private fun addConflictResolveActionToNotification(uploadFileOperation: UploadFileOperation) {
val intent: PendingIntent = createConflictResolveAction(context, uploadFileOperation)

notificationBuilder.addAction(
R.drawable.ic_cloud_upload,
context.getString(R.string.upload_list_resolve_conflict),
intent
)
}

private fun addUploadListContentIntent(uploadFileOperation: UploadFileOperation) {
val uploadListIntent = createUploadListIntent(uploadFileOperation)

notificationBuilder.setContentIntent(
PendingIntent.getActivity(
context,
System.currentTimeMillis().toInt(),
uploadListIntent,
PendingIntent.FLAG_IMMUTABLE
)
)
}

/**
* adapted from [com.owncloud.android.files.services.FileUploader.notifyUploadResult]
*/
@SuppressLint("RestrictedApi")
private fun notifyUploadResult(
uploadFileOperation: UploadFileOperation,
uploadResult: RemoteOperationResult<Any?>
Expand Down Expand Up @@ -284,6 +330,7 @@ class FilesUploadWorker(
// check file conflict
tickerId = R.string.uploader_upload_failed_sync_conflict_error
}

notificationBuilder
.setTicker(context.getString(tickerId))
.setContentTitle(context.getString(tickerId))
Expand All @@ -294,23 +341,16 @@ class FilesUploadWorker(

val content = ErrorMessageAdapter.getErrorCauseMessage(uploadResult, uploadFileOperation, context.resources)

addUploadListContentIntent(uploadFileOperation)

if (uploadResult.code == ResultCode.SYNC_CONFLICT) {
addConflictResolveActionToNotification(uploadFileOperation)
}

if (needsToUpdateCredentials) {
createUpdateCredentialsNotification(uploadFileOperation.user.toPlatformAccount())
} else {
val intent = if (uploadResult.code == ResultCode.SYNC_CONFLICT) {
createResolveConflictIntent(uploadFileOperation)
} else {
createUploadListIntent(uploadFileOperation)
}
notificationBuilder.setContentIntent(
PendingIntent.getActivity(
context,
System.currentTimeMillis().toInt(),
intent,
PendingIntent.FLAG_IMMUTABLE
)
)
}

notificationBuilder.setContentText(content)
if (!uploadResult.isSuccess) {
notificationManager.notify(SecureRandom().nextInt(), notificationBuilder.build())
Expand All @@ -327,16 +367,6 @@ class FilesUploadWorker(
)
}

private fun createResolveConflictIntent(uploadFileOperation: UploadFileOperation): Intent {
return ConflictsResolveActivity.createIntent(
uploadFileOperation.file,
uploadFileOperation.user,
uploadFileOperation.ocUploadId,
Intent.FLAG_ACTIVITY_CLEAR_TOP,
context
)
}

private fun createUpdateCredentialsNotification(account: Account) {
// let the user update credentials with one click
val updateAccountCredentials = Intent(context, AuthenticatorActivity::class.java)
Expand Down
1 change: 0 additions & 1 deletion app/src/main/java/com/owncloud/android/MainApp.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,6 @@
* Contains methods to build the "static" strings. These strings were before constants in different classes
*/
public class MainApp extends MultiDexApplication implements HasAndroidInjector {

public static final OwnCloudVersion OUTDATED_SERVER_VERSION = NextcloudVersion.nextcloud_23;
public static final OwnCloudVersion MINIMUM_SUPPORTED_SERVER_VERSION = OwnCloudVersion.nextcloud_16;

Expand Down
1 change: 1 addition & 0 deletions app/src/main/java/com/owncloud/android/files/services/FileUploader.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1052,6 +1052,7 @@ public static void retryUpload(@NonNull Context context,
i.putExtra(FileUploader.KEY_ACCOUNT, user.toPlatformAccount());
i.putExtra(FileUploader.KEY_RETRY_UPLOAD, upload);

// FIXME Conflict files not uploading successfully
if (useFilesUploadWorker(context)) {
new FilesUploadHelper().retryUpload(upload, user);
} else if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) {
Expand Down
109 changes: 62 additions & 47 deletions app/src/main/java/com/owncloud/android/ui/adapter/UploadListAdapter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ public class UploadListAdapter extends SectionedRecyclerViewAdapter<SectionedVie
private Clock clock;
private UploadGroup[] uploadGroups;
private boolean showUser;
private final ViewThemeUtils viewThemeUtils;
private final ViewThemeUtils viewThemeUtils;

@Override
public int getSectionCount() {
Expand Down Expand Up @@ -235,7 +235,7 @@ public void onBindViewHolder(SectionedViewHolder holder, int section, int relati
// file size
if (item.getFileSize() != 0) {
itemViewHolder.binding.uploadFileSize.setText(String.format("%s, ",
DisplayUtils.bytesToHumanReadable(item.getFileSize())));
DisplayUtils.bytesToHumanReadable(item.getFileSize())));
} else {
itemViewHolder.binding.uploadFileSize.setText("");
}
Expand Down Expand Up @@ -286,7 +286,7 @@ public void onBindViewHolder(SectionedViewHolder holder, int section, int relati
binder.removeDatatransferProgressListener(
progressListener,
progressListener.getUpload() // the one that was added
);
);
}
// ... then, bind the current progress bar to listen for updates
progressListener = new ProgressListener(item, itemViewHolder.binding.uploadProgressBar);
Expand Down Expand Up @@ -383,16 +383,16 @@ public void onBindViewHolder(SectionedViewHolder holder, int section, int relati
DisplayUtils.showSnackMessage(
v.getRootView().findViewById(android.R.id.content),
R.string.local_file_not_found_message
);
);
}
});
} else if (item.getUploadStatus() == UploadStatus.UPLOAD_SUCCEEDED){
} else if (item.getUploadStatus() == UploadStatus.UPLOAD_SUCCEEDED) {
itemViewHolder.binding.uploadListItemLayout.setOnClickListener(v -> onUploadedItemClick(item));
}


// click on thumbnail to open locally
if (item.getUploadStatus() != UploadStatus.UPLOAD_SUCCEEDED){
if (item.getUploadStatus() != UploadStatus.UPLOAD_SUCCEEDED) {
itemViewHolder.binding.thumbnail.setOnClickListener(v -> onUploadingItemClick(item));
}

Expand All @@ -405,29 +405,29 @@ public void onBindViewHolder(SectionedViewHolder holder, int section, int relati
fakeFileToCheatThumbnailsCacheManagerInterface.setMimeType(item.getMimeType());

boolean allowedToCreateNewThumbnail = ThumbnailsCacheManager.cancelPotentialThumbnailWork(
fakeFileToCheatThumbnailsCacheManagerInterface, itemViewHolder.binding.thumbnail
);
fakeFileToCheatThumbnailsCacheManagerInterface, itemViewHolder.binding.thumbnail
);

// TODO this code is duplicated; refactor to a common place
if (MimeTypeUtil.isImage(fakeFileToCheatThumbnailsCacheManagerInterface)
&& fakeFileToCheatThumbnailsCacheManagerInterface.getRemoteId() != null &&
item.getUploadStatus() == UploadStatus.UPLOAD_SUCCEEDED) {
&& fakeFileToCheatThumbnailsCacheManagerInterface.getRemoteId() != null &&
item.getUploadStatus() == UploadStatus.UPLOAD_SUCCEEDED) {
// Thumbnail in Cache?
Bitmap thumbnail = ThumbnailsCacheManager.getBitmapFromDiskCache(
String.valueOf(fakeFileToCheatThumbnailsCacheManagerInterface.getRemoteId())
);
String.valueOf(fakeFileToCheatThumbnailsCacheManagerInterface.getRemoteId())
);
if (thumbnail != null && !fakeFileToCheatThumbnailsCacheManagerInterface.isUpdateThumbnailNeeded()) {
itemViewHolder.binding.thumbnail.setImageBitmap(thumbnail);
} else {
// generate new Thumbnail
Optional<User> user = parentActivity.getUser();
if (allowedToCreateNewThumbnail && user.isPresent()) {
final ThumbnailsCacheManager.ThumbnailGenerationTask task =
new ThumbnailsCacheManager.ThumbnailGenerationTask(
itemViewHolder.binding.thumbnail,
parentActivity.getStorageManager(),
user.get()
);
new ThumbnailsCacheManager.ThumbnailGenerationTask(
itemViewHolder.binding.thumbnail,
parentActivity.getStorageManager(),
user.get()
);
if (thumbnail == null) {
if (MimeTypeUtil.isVideo(fakeFileToCheatThumbnailsCacheManagerInterface)) {
thumbnail = ThumbnailsCacheManager.mDefaultVideo;
Expand All @@ -436,35 +436,35 @@ public void onBindViewHolder(SectionedViewHolder holder, int section, int relati
}
}
final ThumbnailsCacheManager.AsyncThumbnailDrawable asyncDrawable =
new ThumbnailsCacheManager.AsyncThumbnailDrawable(
parentActivity.getResources(),
thumbnail,
task
);
new ThumbnailsCacheManager.AsyncThumbnailDrawable(
parentActivity.getResources(),
thumbnail,
task
);
itemViewHolder.binding.thumbnail.setImageDrawable(asyncDrawable);
task.execute(new ThumbnailsCacheManager.ThumbnailGenerationTaskObject(
fakeFileToCheatThumbnailsCacheManagerInterface, null));
fakeFileToCheatThumbnailsCacheManagerInterface, null));
}
}

if ("image/png".equals(item.getMimeType())) {
itemViewHolder.binding.thumbnail.setBackgroundColor(parentActivity.getResources()
.getColor(R.color.bg_default));
.getColor(R.color.bg_default));
}


} else if (MimeTypeUtil.isImage(fakeFileToCheatThumbnailsCacheManagerInterface)) {
File file = new File(item.getLocalPath());
// Thumbnail in Cache?
Bitmap thumbnail = ThumbnailsCacheManager.getBitmapFromDiskCache(
String.valueOf(file.hashCode()));
String.valueOf(file.hashCode()));
if (thumbnail != null) {
itemViewHolder.binding.thumbnail.setImageBitmap(thumbnail);
} else {
// generate new Thumbnail
if (allowedToCreateNewThumbnail) {
final ThumbnailsCacheManager.ThumbnailGenerationTask task =
new ThumbnailsCacheManager.ThumbnailGenerationTask(itemViewHolder.binding.thumbnail);
new ThumbnailsCacheManager.ThumbnailGenerationTask(itemViewHolder.binding.thumbnail);

if (MimeTypeUtil.isVideo(file)) {
thumbnail = ThumbnailsCacheManager.mDefaultVideo;
Expand All @@ -484,7 +484,7 @@ public void onBindViewHolder(SectionedViewHolder holder, int section, int relati

if ("image/png".equalsIgnoreCase(item.getMimeType())) {
itemViewHolder.binding.thumbnail.setBackgroundColor(parentActivity.getResources()
.getColor(R.color.bg_default));
.getColor(R.color.bg_default));
}
} else {
if (optionalUser.isPresent()) {
Expand All @@ -503,21 +503,14 @@ private boolean checkAndOpenConflictResolutionDialog(User user,
OCUpload item,
String status) {
String remotePath = item.getRemotePath();
OCFile ocFile = storageManager.getFileByPath(remotePath);
OCFile ocFile = storageManager.getFileByEncryptedRemotePath(remotePath);

if (ocFile == null) {
// Remote file doesn't exist, try to refresh folder
OCFile folder = storageManager.getFileByPath(new File(remotePath).getParent() + "/");
OCFile folder = storageManager.getFileByEncryptedRemotePath(new File(remotePath).getParent() + "/");

if (folder != null && folder.isFolder()) {
this.refreshFolder(itemViewHolder, user, folder, (caller, result) -> {
itemViewHolder.binding.uploadStatus.setText(status);
if (result.isSuccess()) {
OCFile file = storageManager.getFileByPath(remotePath);
if (file != null) {
this.openConflictActivity(file, item);
}
}
});
refreshFolderAndUpdateUI(itemViewHolder, user, folder, remotePath, item, status);
return true;
}

Expand All @@ -533,6 +526,29 @@ private boolean checkAndOpenConflictResolutionDialog(User user,
return false;
}

private void refreshFolderAndUpdateUI(ItemViewHolder holder, User user, OCFile folder, String remotePath, OCUpload item, String status) {
Context context = MainApp.getAppContext();

this.refreshFolder(context, holder, user, folder, (caller, result) -> {
holder.binding.uploadStatus.setText(status);

if (result.isSuccess()) {
OCFile file = storageManager.getFileByEncryptedRemotePath(remotePath);

if (file != null) {
openConflictActivity(file, item);
} else {
displayFileNotFoundError(holder.itemView, context);
}
}
});
}

private void displayFileNotFoundError(View itemView, Context context) {
String message = context.getString(R.string.uploader_file_not_found_message);
DisplayUtils.showSnackMessage(itemView, message);
}

private void showItemConflictPopup(User user,
ItemViewHolder itemViewHolder,
OCUpload item,
Expand Down Expand Up @@ -560,13 +576,13 @@ private void removeUpload(OCUpload item) {
}

private void refreshFolder(
Context context,
ItemViewHolder view,
User user,
OCFile folder,
OnRemoteOperationListener listener) {
view.binding.uploadListItemLayout.setClickable(false);
view.binding.uploadStatus.setText(R.string.uploads_view_upload_status_fetching_server_version);
Context context = MainApp.getAppContext();
new RefreshFolderOperation(folder,
clock.getCurrentTime(),
false,
Expand Down Expand Up @@ -598,8 +614,7 @@ private void openConflictActivity(OCFile file, OCUpload upload) {
}

/**
* Gets the status text to show to the user according to the status and last result of the
* the given upload.
* Gets the status text to show to the user according to the status and last result of the the given upload.
*
* @param upload Upload to describe.
* @return Text describing the status of the given upload.
Expand Down Expand Up @@ -680,8 +695,8 @@ private String getUploadFailedStatusText(UploadResult result) {
case SSL_RECOVERABLE_PEER_UNVERIFIED:
status =
parentActivity.getString(
R.string.uploads_view_upload_status_failed_ssl_certificate_not_trusted
);
R.string.uploads_view_upload_status_failed_ssl_certificate_not_trusted
);
break;
case UNKNOWN:
status = parentActivity.getString(R.string.uploads_view_upload_status_unknown_fail);
Expand All @@ -691,7 +706,7 @@ private String getUploadFailedStatusText(UploadResult result) {
break;
case DELAYED_IN_POWER_SAVE_MODE:
status = parentActivity.getString(
R.string.uploads_view_upload_status_waiting_exit_power_save_mode);
R.string.uploads_view_upload_status_waiting_exit_power_save_mode);
break;
case VIRUS_DETECTED:
status = parentActivity.getString(R.string.uploads_view_upload_status_virus_detected);
Expand Down Expand Up @@ -763,17 +778,17 @@ private void onUploadingItemClick(OCUpload file) {
*/
private void onUploadedItemClick(OCUpload upload) {
final OCFile file = parentActivity.getStorageManager().getFileByEncryptedRemotePath(upload.getRemotePath());
if (file == null){
if (file == null) {
DisplayUtils.showSnackMessage(parentActivity, R.string.error_retrieving_file);
Log_OC.i(TAG, "Could not find uploaded file on remote.");
return;
}

if (PreviewImageFragment.canBePreviewed(file)){
if (PreviewImageFragment.canBePreviewed(file)) {
//show image preview and stay in uploads tab
Intent intent = FileDisplayActivity.openFileIntent(parentActivity, parentActivity.getUser().get(), file);
parentActivity.startActivity(intent);
}else{
} else {
Intent intent = new Intent(parentActivity, FileDisplayActivity.class);
intent.setAction(Intent.ACTION_VIEW);
intent.putExtra(FileDisplayActivity.KEY_FILE_PATH, upload.getRemotePath());
Expand Down
Loading
Loading