diff --git a/app/src/androidTest/java/com/orgzly/android/espresso/BooksTest.java b/app/src/androidTest/java/com/orgzly/android/espresso/BooksTest.java index 04d1b75b6..d23d4dd42 100644 --- a/app/src/androidTest/java/com/orgzly/android/espresso/BooksTest.java +++ b/app/src/androidTest/java/com/orgzly/android/espresso/BooksTest.java @@ -11,8 +11,12 @@ import static androidx.test.espresso.intent.Intents.intending; import static androidx.test.espresso.intent.matcher.IntentMatchers.hasAction; import static androidx.test.espresso.intent.matcher.IntentMatchers.hasExtra; +import static androidx.test.espresso.matcher.ViewMatchers.hasChildCount; +import static androidx.test.espresso.matcher.ViewMatchers.isChecked; import static androidx.test.espresso.matcher.ViewMatchers.isDisplayed; import static androidx.test.espresso.matcher.ViewMatchers.isEnabled; +import static androidx.test.espresso.matcher.ViewMatchers.isNotChecked; +import static androidx.test.espresso.matcher.ViewMatchers.withClassName; import static androidx.test.espresso.matcher.ViewMatchers.withId; import static androidx.test.espresso.matcher.ViewMatchers.withText; import static com.orgzly.android.espresso.util.EspressoUtils.contextualToolbarOverflowMenu; @@ -21,7 +25,9 @@ import static com.orgzly.android.espresso.util.EspressoUtils.onNoteInBook; import static com.orgzly.android.espresso.util.EspressoUtils.onSnackbar; import static com.orgzly.android.espresso.util.EspressoUtils.replaceTextCloseKeyboard; +import static com.orgzly.android.espresso.util.EspressoUtils.sync; import static org.hamcrest.Matchers.allOf; +import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.endsWith; import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.startsWith; @@ -41,6 +47,7 @@ import com.orgzly.R; import com.orgzly.android.BookFormat; import com.orgzly.android.OrgzlyTest; +import com.orgzly.android.repos.RepoType; import com.orgzly.android.ui.main.MainActivity; import org.junit.Before; @@ -299,4 +306,106 @@ public void testBackPressClosesSelectionMenu() { // Make sure we're still in the app onBook(0, R.id.item_book_title).check(matches(withText("book-1"))); } + + @Test + public void testSetLinkOnSingleBookCurrentRepoIsSelected() { + testUtils.setupRepo(RepoType.MOCK, "mock://repo"); + sync(); + onBook(0, R.id.item_book_link_repo).check(matches(withText("mock://repo"))); + onBook(0).perform(longClick()); + contextualToolbarOverflowMenu().perform(click()); + onView(withText(R.string.books_context_menu_item_set_link)).perform(click()); + onView(withText("mock://repo")).check(matches(isChecked())); + } + + /** + * When setting the link of multiple books, no repo should be pre-selected, + * no matter how many repos there are, and no matter whether the books + * already have a link or not. The reason for this is that we have no + * intuitive way of displaying links to multiple repos. + */ + @Test + public void testSetLinkOnMultipleBooksNoRepoIsSelected() { + testUtils.setupRepo(RepoType.MOCK, "mock://repo"); + sync(); + onBook(0, R.id.item_book_link_repo).check(matches(withText("mock://repo"))); + onBook(1, R.id.item_book_link_repo).check(matches(withText("mock://repo"))); + onBook(0).perform(longClick()); + onBook(1).perform(click()); + contextualToolbarOverflowMenu().perform(click()); + onView(withText(R.string.books_context_menu_item_set_link)).perform(click()); + onView(withText("mock://repo")).check(matches(isNotChecked())); + } + + @Test + public void testDeleteSingleBookLinkedUrlIsShown() { + testUtils.setupRepo(RepoType.MOCK, "mock://repo"); + sync(); + onBook(0, R.id.item_book_link_repo).check(matches(withText("mock://repo"))); + onBook(0).perform(longClick()); + contextualToolbarOverflowMenu().perform(click()); + onView(withText(R.string.delete)).perform(click()); + onView(withText(R.string.also_delete_linked_book)).check(matches(isDisplayed())); + onView(withId(R.id.delete_linked_url)).check(matches(withText("mock://repo/book-1.org"))); + } + + @Test + public void testDeleteMultipleBooksLinkedUrlIsNotShown() { + testUtils.setupRepo(RepoType.MOCK, "mock://repo"); + sync(); + onBook(0, R.id.item_book_link_repo).check(matches(withText("mock://repo"))); + onBook(1, R.id.item_book_link_repo).check(matches(withText("mock://repo"))); + onBook(0).perform(longClick()); + onBook(1).perform(click()); + contextualToolbarOverflowMenu().perform(click()); + onView(withText(R.string.delete)).perform(click()); + onView(withText(R.string.also_delete_linked_books)).check(matches(isDisplayed())); + onView(withId(R.id.delete_linked_url)).check(matches(withText(""))); + } + + @Test + public void testDeleteMultipleBooksWithNoLinks() { + onBook(0).perform(longClick()); + onBook(1).perform(click()); + contextualToolbarOverflowMenu().perform(click()); + onView(withText(R.string.delete)).perform(click()); + onView(withText(R.string.delete)).perform(click()); + assert dataRepository.getBooks().size() == 1; + } + + @Test + public void testDeleteMultipleBooksAndRooks() { + testUtils.setupRepo(RepoType.MOCK, "mock://repo"); + sync(); + onBook(0, R.id.item_book_link_repo).check(matches(withText("mock://repo"))); + onBook(1, R.id.item_book_link_repo).check(matches(withText("mock://repo"))); + onBook(0).perform(longClick()); + onBook(1).perform(click()); + contextualToolbarOverflowMenu().perform(click()); + onView(withText(R.string.delete)).perform(click()); + onView(withId(R.id.delete_linked_checkbox)).perform(click()); + onView(withText(R.string.delete)).perform(click()); + assert dataRepository.getBooks().size() == 1; + } + + /** + * When multiple books are selected, the "rename" and "export" actions should be removed from + * the context menu. By also testing that only the expected number of actions are shown, we + * protect against someone later adding actions to the menu without fully considering the support for + * multiple selected books. When such support is added, this test will need to be updated. + */ + @Test + public void testMultipleBooksSelectedContextMenuShowsSupportedActionsOnly() { + onBook(0).perform(longClick()); + contextualToolbarOverflowMenu().perform(click()); + onView(withText(R.string.rename)).check(matches(isDisplayed())); + onView(withText(R.string.export)).check(matches(isDisplayed())); + onView(withClassName(containsString("MenuDropDownListView"))).check(matches(hasChildCount(4))); + pressBack(); + onBook(1).perform(click()); + contextualToolbarOverflowMenu().perform(click()); + onView(withText(R.string.rename)).check(doesNotExist()); + onView(withText(R.string.export)).check(doesNotExist()); + onView(withClassName(containsString("MenuDropDownListView"))).check(matches(hasChildCount(2))); + } } diff --git a/app/src/androidTest/java/com/orgzly/android/espresso/SyncingTest.java b/app/src/androidTest/java/com/orgzly/android/espresso/SyncingTest.java index eecbd1d24..d8ad9b7db 100644 --- a/app/src/androidTest/java/com/orgzly/android/espresso/SyncingTest.java +++ b/app/src/androidTest/java/com/orgzly/android/espresso/SyncingTest.java @@ -5,7 +5,6 @@ import static androidx.test.espresso.action.ViewActions.click; import static androidx.test.espresso.action.ViewActions.longClick; import static androidx.test.espresso.assertion.ViewAssertions.matches; -import static androidx.test.espresso.contrib.DrawerActions.close; import static androidx.test.espresso.contrib.DrawerActions.open; import static androidx.test.espresso.matcher.ViewMatchers.isDescendantOfA; import static androidx.test.espresso.matcher.ViewMatchers.isDisplayed; @@ -14,7 +13,6 @@ import static androidx.test.espresso.matcher.ViewMatchers.withText; import static com.orgzly.android.espresso.util.EspressoUtils.clickSetting; import static com.orgzly.android.espresso.util.EspressoUtils.contextualToolbarOverflowMenu; -import static com.orgzly.android.espresso.util.EspressoUtils.grantAlarmsAndRemindersPermission; import static com.orgzly.android.espresso.util.EspressoUtils.onActionItemClick; import static com.orgzly.android.espresso.util.EspressoUtils.onBook; import static com.orgzly.android.espresso.util.EspressoUtils.onListItem; @@ -24,6 +22,7 @@ import static com.orgzly.android.espresso.util.EspressoUtils.recyclerViewItemCount; import static com.orgzly.android.espresso.util.EspressoUtils.replaceTextCloseKeyboard; import static com.orgzly.android.espresso.util.EspressoUtils.settingsSetTodoKeywords; +import static com.orgzly.android.espresso.util.EspressoUtils.sync; import static org.hamcrest.Matchers.allOf; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.endsWith; @@ -69,16 +68,6 @@ public void tearDown() throws Exception { } } - /** - * Utility method for starting sync using drawer button. - */ - private void sync() { - grantAlarmsAndRemindersPermission(); - onView(withId(R.id.drawer_layout)).perform(open()); - onView(withId(R.id.sync_button_container)).perform(click()); - onView(withId(R.id.drawer_layout)).perform(close()); - } - @Test public void testRunSync() { scenario = ActivityScenario.launch(MainActivity.class); diff --git a/app/src/androidTest/java/com/orgzly/android/espresso/util/EspressoUtils.java b/app/src/androidTest/java/com/orgzly/android/espresso/util/EspressoUtils.java index 66141e453..cc401eaba 100644 --- a/app/src/androidTest/java/com/orgzly/android/espresso/util/EspressoUtils.java +++ b/app/src/androidTest/java/com/orgzly/android/espresso/util/EspressoUtils.java @@ -7,6 +7,8 @@ import static androidx.test.espresso.action.ViewActions.click; import static androidx.test.espresso.action.ViewActions.pressKey; import static androidx.test.espresso.action.ViewActions.replaceText; +import static androidx.test.espresso.contrib.DrawerActions.close; +import static androidx.test.espresso.contrib.DrawerActions.open; import static androidx.test.espresso.matcher.ViewMatchers.hasDescendant; import static androidx.test.espresso.matcher.ViewMatchers.isAssignableFrom; import static androidx.test.espresso.matcher.ViewMatchers.isDisplayed; @@ -512,4 +514,14 @@ public static void grantAlarmsAndRemindersPermission() { getInstrumentation().getUiAutomation().executeShellCommand(shellCmd); } } + + /** + * Utility method for starting sync using drawer button. + */ + public static void sync() { + grantAlarmsAndRemindersPermission(); + onView(withId(R.id.drawer_layout)).perform(open()); + onView(withId(R.id.sync_button_container)).perform(click()); + onView(withId(R.id.drawer_layout)).perform(close()); + } } diff --git a/app/src/main/java/com/orgzly/android/ui/books/BooksFragment.kt b/app/src/main/java/com/orgzly/android/ui/books/BooksFragment.kt index c10abc9ff..b025568dd 100644 --- a/app/src/main/java/com/orgzly/android/ui/books/BooksFragment.kt +++ b/app/src/main/java/com/orgzly/android/ui/books/BooksFragment.kt @@ -10,6 +10,7 @@ import android.text.TextUtils import android.text.TextWatcher import android.util.Log import android.view.LayoutInflater +import android.view.Menu import android.view.View import android.view.ViewGroup import androidx.activity.OnBackPressedCallback @@ -155,7 +156,7 @@ class BooksFragment : CommonFragment(), DrawerItem, OnViewHolderClickListener - val bookId = viewAdapter.getSelection().getOnly() + val bookIds = viewAdapter.getSelection().getIds() - if (bookId == null) { + if (bookIds.isEmpty()) { Log.e(TAG, "Cannot handle action when there are no items selected") return@setOnMenuItemClickListener true } when (menuItem.itemId) { R.id.books_context_menu_rename -> { - viewModel.renameBookRequest(bookId) + // N.B. Menu item is hidden when multiple books are selected + viewModel.renameBookRequest(bookIds.first()) } R.id.books_context_menu_set_link -> { - viewModel.setBookLinkRequest(bookId) + viewModel.setBookLinksRequest(bookIds) } R.id.books_context_menu_force_save -> { @@ -254,7 +257,7 @@ class BooksFragment : CommonFragment(), DrawerItem, OnViewHolderClickListener - viewModel.forceSaveBookRequest(bookId) + viewModel.forceSaveBookRequest(bookIds) } .setNegativeButton(R.string.cancel, null) .show() @@ -265,18 +268,19 @@ class BooksFragment : CommonFragment(), DrawerItem, OnViewHolderClickListener - viewModel.forceLoadBookRequest(bookId) + viewModel.forceLoadBookRequest(bookIds) } .setNegativeButton(R.string.cancel, null) .show() } R.id.books_context_menu_export -> { - viewModel.exportBookRequest(bookId, BookFormat.ORG) + // N.B. Menu item is hidden when multiple books are selected + viewModel.exportBookRequest(bookIds.first(), BookFormat.ORG) } R.id.books_context_menu_delete -> { - viewModel.deleteBookRequest(bookId) + viewModel.deleteBooksRequest(bookIds) } } @@ -291,6 +295,13 @@ class BooksFragment : CommonFragment(), DrawerItem, OnViewHolderClickListener if (uri != null) { @@ -305,8 +316,17 @@ class BooksFragment : CommonFragment(), DrawerItem, OnViewHolderClickListener) { val dialogBinding = DialogBookDeleteBinding.inflate(LayoutInflater.from(context)) + val dialogTitle: String + val book: BookView? + if (books.size == 1) { + book = books.first() + dialogTitle = getString(R.string.delete_with_quoted_argument, book.book.name) + } else { + book = null + dialogTitle = getString(R.string.delete_amount_of_books, books.size) + } dialogBinding.deleteLinkedCheckbox.setOnCheckedChangeListener { _, isChecked -> dialogBinding.deleteLinkedUrl.isEnabled = isChecked @@ -316,20 +336,23 @@ class BooksFragment : CommonFragment(), DrawerItem, OnViewHolderClickListener { val deleteLinked = dialogBinding.deleteLinkedCheckbox.isChecked - viewModel.deleteBook(book.book.id, deleteLinked) + for (book in books) { + viewModel.deleteBook(book.book.id, deleteLinked) + } } } } val builder = MaterialAlertDialogBuilder(requireContext()) - .setTitle(getString(R.string.delete_with_quoted_argument, book.book.name)) + .setTitle(dialogTitle) .setPositiveButton(R.string.delete, dialogClickListener) .setNegativeButton(R.string.cancel, dialogClickListener) - if (book.syncedTo != null) { + if (book?.syncedTo != null) { dialogBinding.deleteLinkedUrl.text = book.syncedTo.uri.toString() - builder.setView(dialogBinding.root) + dialogBinding.deleteLinkedCheckbox.text = getString(R.string.also_delete_linked_book) } + builder.setView(dialogBinding.root) dialog = builder.show() } @@ -421,9 +444,9 @@ class BooksFragment : CommonFragment(), DrawerItem, OnViewHolderClickListener - if (bookView != null) { - deleteBookDialog(bookView) + viewModel.booksToDeleteEvent.observeSingle(viewLifecycleOwner, Observer { bookViews -> + if (bookViews.isNotEmpty()) { + deleteBooksDialog(bookViews) } }) @@ -445,7 +468,7 @@ class BooksFragment : CommonFragment(), DrawerItem, OnViewHolderClickListener + viewModel.setBookLinkRequestEvent.observeSingle(this) { (bookIds, links, urls, checked) -> if (links.isEmpty()) { activity?.showSnackbar(getString(R.string.no_repos), R.string.repositories) { activity?.let { @@ -454,7 +477,6 @@ class BooksFragment : CommonFragment(), DrawerItem, OnViewHolderClickListener - viewModel.setBookLink(book.id, links[which]) + viewModel.setBookLinks(bookIds, links[which]) dialog?.dismiss() dialog = null } - .setNeutralButton(R.string.remove_notebook_link) { dialog, which -> - viewModel.setBookLink(book.id) + .setNeutralButton(R.string.remove_notebook_link) { _, _ -> + viewModel.setBookLinks(bookIds) } .setNegativeButton(R.string.cancel, null) .show() diff --git a/app/src/main/java/com/orgzly/android/ui/books/BooksViewModel.kt b/app/src/main/java/com/orgzly/android/ui/books/BooksViewModel.kt index 7a45cc552..3046c4d3d 100644 --- a/app/src/main/java/com/orgzly/android/ui/books/BooksViewModel.kt +++ b/app/src/main/java/com/orgzly/android/ui/books/BooksViewModel.kt @@ -15,7 +15,6 @@ import com.orgzly.android.ui.CommonViewModel import com.orgzly.android.ui.SingleLiveEvent import com.orgzly.android.usecase.* import com.orgzly.android.util.LogUtils -import java.io.File class BooksViewModel(private val dataRepository: DataRepository) : CommonViewModel() { @@ -24,7 +23,7 @@ class BooksViewModel(private val dataRepository: DataRepository) : CommonViewMod // Book being operated on (deleted, renamed, etc.) private var lastBook = MutableLiveData>() - val bookToDeleteEvent: SingleLiveEvent = SingleLiveEvent() + val booksToDeleteEvent: SingleLiveEvent> = SingleLiveEvent() val bookDeletedEvent: SingleLiveEvent = SingleLiveEvent() val bookToRenameEvent: SingleLiveEvent = SingleLiveEvent() val bookToExportEvent: SingleLiveEvent> = SingleLiveEvent() @@ -64,9 +63,10 @@ class BooksViewModel(private val dataRepository: DataRepository) : CommonViewMod } } - fun deleteBookRequest(bookId: Long) { + fun deleteBooksRequest(bookIds: Set) { + val bookViews = bookIds.map { requireNotNull(dataRepository.getBookView(it)) }.toSet() App.EXECUTORS.diskIO().execute { - bookToDeleteEvent.postValue(dataRepository.getBookView(bookId)) + booksToDeleteEvent.postValue(bookViews) } } @@ -94,29 +94,28 @@ class BooksViewModel(private val dataRepository: DataRepository) : CommonViewMod } data class BookLinkOptions( - val book: Book, val links: List, val urls: List, val selected: Int) + val bookIds: Set, val links: List, val urls: List, val selected: Int) - fun setBookLinkRequest(bookId: Long) { + fun setBookLinksRequest(bookIds: Set) { App.EXECUTORS.diskIO().execute { - val bookView = dataRepository.getBookView(bookId) - - if (bookView == null) { - errorEvent.postValue(Throwable("Book not found")) - + if (bookIds.isEmpty()) { + errorEvent.postValue(Throwable("No books found")) } else { val repos = dataRepository.getRepos() val options = if (repos.isEmpty()) { - BookLinkOptions(bookView.book, emptyList(), emptyList(), -1) - + BookLinkOptions(bookIds, emptyList(), emptyList(), -1) } else { - val currentLink = bookView.linkRepo - - val selectedLink = repos.indexOfFirst { - it.url == currentLink?.url + if (bookIds.size == 1) { + val bookView = dataRepository.getBookView(bookIds.first()) + val currentLink = bookView?.linkRepo + val selectedLink = repos.indexOfFirst { + it.url == currentLink?.url + } + BookLinkOptions(bookIds, repos, repos.map { it.url }, selectedLink) + } else { + BookLinkOptions(bookIds, repos, repos.map { it.url }, -1) } - - BookLinkOptions(bookView.book, repos, repos.map { it.url }, selectedLink) } setBookLinkRequestEvent.postValue(options) @@ -124,26 +123,32 @@ class BooksViewModel(private val dataRepository: DataRepository) : CommonViewMod } } - fun setBookLink(bookId: Long, repo: Repo? = null) { - App.EXECUTORS.diskIO().execute { - catchAndPostError { - UseCaseRunner.run(BookLinkUpdate(bookId, repo)) + fun setBookLinks(bookIds: Set, repo: Repo? = null) { + for (bookId in bookIds) { + App.EXECUTORS.diskIO().execute { + catchAndPostError { + UseCaseRunner.run(BookLinkUpdate(bookId, repo)) + } } } } - fun forceSaveBookRequest(bookId: Long) { - App.EXECUTORS.diskIO().execute { - catchAndPostError { - UseCaseRunner.run(BookForceSave(bookId)) + fun forceSaveBookRequest(bookIds: Set) { + for (bookId in bookIds) { + App.EXECUTORS.diskIO().execute { + catchAndPostError { + UseCaseRunner.run(BookForceSave(bookId)) + } } } } - fun forceLoadBookRequest(bookId: Long) { - App.EXECUTORS.diskIO().execute { - catchAndPostError { - UseCaseRunner.run(BookForceLoad(bookId)) + fun forceLoadBookRequest(bookIds: Set) { + for (bookId in bookIds) { + App.EXECUTORS.diskIO().execute { + catchAndPostError { + UseCaseRunner.run(BookForceLoad(bookId)) + } } } } diff --git a/app/src/main/res/layout/dialog_book_delete.xml b/app/src/main/res/layout/dialog_book_delete.xml index db9a77214..d5e9acbcd 100644 --- a/app/src/main/res/layout/dialog_book_delete.xml +++ b/app/src/main/res/layout/dialog_book_delete.xml @@ -14,7 +14,7 @@ android:layout_height="wrap_content" android:layout_gravity="start|center_vertical" android:gravity="start|center_vertical" - android:text="@string/also_delete_linked_book" + android:text="@string/also_delete_linked_books" android:paddingStart="8dp" android:paddingEnd="8dp" /> diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 8e66dfab4..eaf761f93 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -24,6 +24,7 @@ Edit note Delete Delete ā€œ%sā€ + Delete %d notebooks Name Query Done @@ -290,8 +291,9 @@ Renamed from ā€œ%sā€ Delete linked remote notebook - Deleting notebook failed: %s - Notebook deleted + Delete any linked remote notebooks + Deleting notebook(s) failed: %s + Notebook(s) deleted Priority %s Default priority @@ -614,8 +616,8 @@ Notifications Sound, vibrate, notification dot Hide metadata - Overwrite local notebook? - Overwrite remote notebook? + Overwrite local notebook(s)? + Overwrite remote notebook(s)? Drawers folded Fold drawers by default Log to drawer on time shift