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

Jump to bottom

[FEATURE REQUEST] "Apply to all" when many name conflicts arise #4138

Merged
merged 9 commits into from
Sep 13, 2023
Merged

[FEATURE REQUEST] "Apply to all" when many name conflicts arise #4138

merged 9 commits into from
Sep 13, 2023

Conversation

Aitorbp
Copy link
Collaborator

@Aitorbp Aitorbp commented Aug 22, 2023 •
edited by jesmrec
Loading

Related Issues

App:

  • Added changelog files for the fixed issues in folder changelog/unreleased. More info here

QA

Reports & improvements

@Aitorbp Aitorbp self-assigned this Aug 22, 2023
@Aitorbp Aitorbp linked an issue Aug 22, 2023 that may be closed by this pull request
[FEATURE REQUEST] "Apply to all" when many name conflicts arise #4078
Closed
9 tasks
manuelplazaspalacio
manuelplazaspalacio suggested changes Aug 23, 2023
View reviewed changes
Copy link
Contributor

@manuelplazaspalacio manuelplazaspalacio left a comment

Choose a reason for hiding this comment

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

Some change here @Aitorbp

owncloudApp/src/main/java/com/owncloud/android/ui/activity/FileDisplayActivity.kt Outdated Show resolved Hide resolved
owncloudApp/src/main/java/com/owncloud/android/ui/activity/FileDisplayActivity.kt Outdated Show resolved Hide resolved
owncloudApp/src/main/java/com/owncloud/android/ui/activity/FileDisplayActivity.kt Outdated Show resolved Hide resolved
owncloudApp/src/main/java/com/owncloud/android/ui/activity/FileDisplayActivity.kt Outdated Show resolved Hide resolved
owncloudApp/src/main/java/com/owncloud/android/ui/activity/FileDisplayActivity.kt Outdated Show resolved Hide resolved
owncloudApp/src/main/java/com/owncloud/android/ui/dialog/FileAlreadyExistsDialog.kt Outdated Show resolved Hide resolved
owncloudApp/src/main/java/com/owncloud/android/ui/dialog/FileAlreadyExistsDialog.kt Show resolved Hide resolved
owncloudApp/src/main/java/com/owncloud/android/ui/dialog/FileAlreadyExistsDialog.kt Outdated Show resolved Hide resolved
owncloudApp/src/main/java/com/owncloud/android/ui/dialog/FileAlreadyExistsDialog.kt Outdated Show resolved Hide resolved
owncloudApp/src/main/java/com/owncloud/android/ui/dialog/FileAlreadyExistsDialog.kt Outdated Show resolved Hide resolved
manuelplazaspalacio
manuelplazaspalacio suggested changes Aug 25, 2023
View reviewed changes
owncloudApp/src/main/java/com/owncloud/android/ui/dialog/FileAlreadyExistsDialog.kt Outdated
fun build() = FileAlreadyExistsDialog(
titleText = titleText,
descriptionText = descriptionText,
checkboxText = checkboxText)
Copy link
Contributor

Choose a reason for hiding this comment

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

You forgot the coma here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

owncloudApp/src/main/java/com/owncloud/android/ui/dialog/FileAlreadyExistsDialog.kt Outdated Show resolved Hide resolved
@Aitorbp Aitorbp force-pushed the feature/apply_to_all_when_many_name_conflicts_arise branch from dc31ca9 to 4f35192 Compare August 28, 2023 09:31
manuelplazaspalacio
manuelplazaspalacio reviewed Aug 29, 2023
View reviewed changes
Copy link
Contributor

@manuelplazaspalacio manuelplazaspalacio left a comment

Choose a reason for hiding this comment

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

LGTM!! 🎉

@jesmrec
Copy link
Collaborator

jesmrec commented Sep 1, 2023 •
edited
Loading

(1) [FIXED]

  1. Copy some items to another place including items in the same name to trigger the conflict dialog
  2. When the conflict dialog is displayed, change device orientation

Current: app crashes
Expected: no crash, dialog is shown in portrait/landscape

Pixel 2 Android 11
4f35192d4

manuelplazaspalacio
manuelplazaspalacio suggested changes Sep 6, 2023
View reviewed changes
Copy link
Contributor

@manuelplazaspalacio manuelplazaspalacio left a comment

Choose a reason for hiding this comment

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

Some change here @Aitorbp

owncloudApp/src/main/java/com/owncloud/android/ui/activity/FileDisplayActivity.kt Show resolved Hide resolved
owncloudApp/src/main/java/com/owncloud/android/ui/activity/FileDisplayActivity.kt Outdated Show resolved Hide resolved
owncloudApp/src/main/java/com/owncloud/android/ui/dialog/FileAlreadyExistsDialog.kt Outdated Show resolved Hide resolved
owncloudApp/src/main/java/com/owncloud/android/ui/dialog/FileAlreadyExistsDialog.kt Outdated Show resolved Hide resolved
manuelplazaspalacio
manuelplazaspalacio suggested changes Sep 7, 2023
View reviewed changes
Copy link
Contributor

@manuelplazaspalacio manuelplazaspalacio left a comment

Choose a reason for hiding this comment

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

Some changes here

owncloudApp/src/main/java/com/owncloud/android/ui/activity/FileDisplayActivity.kt Outdated Show resolved Hide resolved
owncloudApp/src/main/java/com/owncloud/android/ui/activity/FileDisplayActivity.kt Outdated Show resolved Hide resolved
owncloudApp/src/main/java/com/owncloud/android/ui/activity/FileDisplayActivity.kt Outdated Show resolved Hide resolved
owncloudApp/src/main/java/com/owncloud/android/ui/activity/FileDisplayActivity.kt Show resolved Hide resolved
@jesmrec
Copy link
Collaborator

jesmrec commented Sep 13, 2023

QA

  • No conflicts (copy - move)
  • 1 conflict -> no new dialog (copy - move)
  • More than 1 conflict
    • Correct number of conflicts (copy - move)
    • Use skip for all conflicts (copy - move)
    • Use replace for all conflicts (copy - move)
    • Use keep both for all conflicts (copy - move)
    • Use skip for the last conflicts (copy - move)
    • Use replace for the last conflicts (copy - move)
    • Use keep both for the last conflicts (copy - move)
    • Conflict counter decreases after every fix (copy - move)

@jesmrec
Copy link
Collaborator

jesmrec commented Sep 13, 2023

From my side, this one could be approved. The only thing i miss here is adding some new tests for the feature.

@Aitorbp
Copy link
Collaborator Author

Aitorbp commented Sep 13, 2023

UI tests will not be done at the moment. A meeting will be held to know how to proceed with these tests.

@Aitorbp Aitorbp force-pushed the feature/apply_to_all_when_many_name_conflicts_arise branch from c2c4d4d to 2df6be0 Compare September 13, 2023 09:50
@Aitorbp Aitorbp force-pushed the feature/apply_to_all_when_many_name_conflicts_arise branch from 2df6be0 to d8f14a7 Compare September 13, 2023 09:55
@Aitorbp Aitorbp merged commit bf9e477 into master Sep 13, 2023
2 checks passed
@Aitorbp Aitorbp deleted the feature/apply_to_all_when_many_name_conflicts_arise branch September 13, 2023 10:27
jesmrec pushed a commit that referenced this pull request Oct 20, 2023
@Aitorbp @jesmrec
Merge pull request #4138 from owncloud/feature/apply_to_all_when_many…
dcfd790 
…_name_conflicts_arise

[FEATURE REQUEST] "Apply to all" when many name conflicts arise
Aitorbp added a commit that referenced this pull request Feb 5, 2024
@Aitorbp
Merge pull request #4138 from owncloud/feature/apply_to_all_when_many…
bfd82f9 
…_name_conflicts_arise

[FEATURE REQUEST] "Apply to all" when many name conflicts arise
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@manuelplazaspalacio manuelplazaspalacio manuelplazaspalacio approved these changes

Assignees

@Aitorbp Aitorbp

Labels
Approved by qa
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[FEATURE REQUEST] "Apply to all" when many name conflicts arise
3 participants
@Aitorbp @jesmrec @manuelplazaspalacio