jenkins-bot has submitted this change and it was merged. (
https://gerrit.wikimedia.org/r/403764 )
Change subject: Follow-up: Further simplify splitting of reading lists.
......................................................................
Follow-up: Further simplify splitting of reading lists.
Nothing wrong with the previous patch, but I saw a couple more
opportunities for optimization/simplicity:
- Move the logic to show the AlertDialog to BaseActivity, since
BaseActivity already contains the logic to register for other types of
Bus events.
- Simplify the logic of reshuffling super-large lists. It no longer
requires any manipulation of ArrayLists or recursion.
- Don't use a string resource for the renamed names of lists. This is one
particular case where we might not want to "trust" the translations to
follow the formatting that we expect. If a certain localization
translates this string incorrectly, then the procedure of renaming the
list might fail.
Change-Id: I1ded79c3f74273526bfa43b188736518f7850ea3
---
M app/src/main/java/org/wikipedia/activity/BaseActivity.java
M app/src/main/java/org/wikipedia/main/MainActivity.java
M app/src/main/java/org/wikipedia/readinglist/database/ReadingListPageTable.java
M app/src/main/res/values-qq/strings.xml
M app/src/main/res/values/strings.xml
5 files changed, 40 insertions(+), 71 deletions(-)
Approvals:
Sharvaniharan: Looks good to me, approved
jenkins-bot: Verified
diff --git a/app/src/main/java/org/wikipedia/activity/BaseActivity.java
b/app/src/main/java/org/wikipedia/activity/BaseActivity.java
index 0c96404..a74d2ae 100644
--- a/app/src/main/java/org/wikipedia/activity/BaseActivity.java
+++ b/app/src/main/java/org/wikipedia/activity/BaseActivity.java
@@ -12,6 +12,7 @@
import android.support.annotation.Nullable;
import android.support.design.widget.Snackbar;
import android.support.v4.content.ContextCompat;
+import android.support.v7.app.AlertDialog;
import android.support.v7.app.AppCompatActivity;
import android.view.MenuItem;
import android.view.View;
@@ -25,6 +26,7 @@
import org.wikipedia.WikipediaApp;
import org.wikipedia.crash.CrashReportActivity;
import org.wikipedia.events.NetworkConnectEvent;
+import org.wikipedia.events.SplitLargeListsEvent;
import org.wikipedia.events.ThemeChangeEvent;
import org.wikipedia.events.WikipediaZeroEnterEvent;
import org.wikipedia.offline.Compilation;
@@ -226,6 +228,13 @@
@Subscribe public void on(ThemeChangeEvent event) {
recreate();
}
+
+ @Subscribe public void on(SplitLargeListsEvent event) {
+ new AlertDialog.Builder(BaseActivity.this)
+ .setMessage(R.string.split_reading_list_message)
+ .setPositiveButton(R.string.ok, null)
+ .show();
+ }
}
}
diff --git a/app/src/main/java/org/wikipedia/main/MainActivity.java
b/app/src/main/java/org/wikipedia/main/MainActivity.java
index 6ed0e4a..2454a32 100644
--- a/app/src/main/java/org/wikipedia/main/MainActivity.java
+++ b/app/src/main/java/org/wikipedia/main/MainActivity.java
@@ -1,22 +1,16 @@
package org.wikipedia.main;
import android.content.Context;
-import android.content.DialogInterface;
import android.content.Intent;
import android.os.Bundle;
import android.support.annotation.NonNull;
import android.support.annotation.Nullable;
-import android.support.v7.app.AlertDialog;
import android.support.v7.view.ActionMode;
import android.view.View;
-import com.squareup.otto.Subscribe;
-
import org.wikipedia.R;
-import org.wikipedia.WikipediaApp;
import org.wikipedia.activity.SingleFragmentToolbarActivity;
import org.wikipedia.appshortcuts.AppShortcuts;
-import org.wikipedia.events.SplitLargeListsEvent;
import org.wikipedia.navtab.NavTab;
import org.wikipedia.onboarding.InitialOnboardingActivity;
import org.wikipedia.settings.Prefs;
@@ -29,8 +23,6 @@
return new Intent(context, MainActivity.class);
}
- private EventBusMethods busMethods;
-
@Override
public void onCreate(@Nullable Bundle savedInstanceState) {
super.onCreate(savedInstanceState);
@@ -40,14 +32,6 @@
if (Prefs.isInitialOnboardingEnabled() && savedInstanceState == null) {
startActivity(InitialOnboardingActivity.newIntent(this));
}
- busMethods = new EventBusMethods();
- registerBus();
- }
-
- @Override
- public void onDestroy() {
- unregisterBus();
- super.onDestroy();
}
@Override protected MainFragment createFragment() {
@@ -132,30 +116,5 @@
return;
}
finish();
- }
-
- private void registerBus() {
- WikipediaApp.getInstance().getBus().register(busMethods);
- }
-
- private void unregisterBus() {
- if (WikipediaApp.getInstance().getBus() != null) {
- WikipediaApp.getInstance().getBus().unregister(busMethods);
- }
- }
-
- private class EventBusMethods {
- @Subscribe
- public void on(SplitLargeListsEvent event) {
- showLargeListSplitMessage();
- }
- }
-
- private void showLargeListSplitMessage() {
- new AlertDialog.Builder(this)
- .setMessage(R.string.split_reading_list_message)
- .setPositiveButton(R.string.ok,
- (DialogInterface dialogInterface, int i) ->
dialogInterface.dismiss())
- .show();
}
}
diff --git
a/app/src/main/java/org/wikipedia/readinglist/database/ReadingListPageTable.java
b/app/src/main/java/org/wikipedia/readinglist/database/ReadingListPageTable.java
index 5a1eccb..ffa9f77 100644
---
a/app/src/main/java/org/wikipedia/readinglist/database/ReadingListPageTable.java
+++
b/app/src/main/java/org/wikipedia/readinglist/database/ReadingListPageTable.java
@@ -25,7 +25,6 @@
public class ReadingListPageTable extends DatabaseTable<ReadingListPage> {
private static final int DB_VER_INTRODUCED = 18;
private static final int MAX_ARTICLE_LIMIT = 1000;
- private boolean userLargeListsMessageShown = false;
public ReadingListPageTable() {
super(ReadingListPageContract.TABLE, ReadingListPageContract.URI);
@@ -152,6 +151,7 @@
lists.add(list);
}
}
+ boolean shouldShowLargeSplitMessage = false;
try (Cursor cursor = db.rawQuery("SELECT * FROM " +
OldReadingListPageContract.TABLE_PAGE
+ " JOIN " + OldReadingListPageContract.TABLE_DISK + " ON
("
+ OldReadingListPageContract.Col.KEY.qualifiedName() + " =
"
@@ -187,22 +187,45 @@
page.offline(true);
page.status(ReadingListPage.STATUS_SAVED);
}
+ ReadingList origList = null;
for (ReadingList list : lists) {
if (listKeys.contains(getListKey(list.title()))) {
- list.pages().add(page);
+ origList = list;
+ break;
}
}
- }
-
- for (ReadingList list : lists) {
- if (list.pages().size() > MAX_ARTICLE_LIMIT) {
- spiltLargeList(db, list, lists);
+ if (origList == null) {
+ continue;
}
+
+ int splitListIndex = 0;
+ ReadingList newList = origList;
+ while (newList.pages().size() >= MAX_ARTICLE_LIMIT) {
+ shouldShowLargeSplitMessage = true;
+ newList = null;
+ String newListName = origList.title() + " (" +
Integer.toString(++splitListIndex) + ")";
+ for (ReadingList list : lists) {
+ if (list.title().equals(newListName)) {
+ newList = list;
+ break;
+ }
+ }
+ if (newList == null) {
+ newList =
ReadingListDbHelper.instance().createList(db, newListName,
origList.description());
+ lists.add(newList);
+ }
+ }
+
+ newList.pages().add(page);
}
for (ReadingList list : lists) {
ReadingListDbHelper.instance().addPagesToList(db, list,
list.pages());
}
+ }
+
+ if (shouldShowLargeSplitMessage) {
+ WikipediaApp.getInstance().getBus().post(new
SplitLargeListsEvent());
}
db.execSQL("DROP TABLE IF EXISTS " + OldReadingListContract.TABLE);
@@ -217,25 +240,5 @@
@NonNull private static String getListKey(@NonNull String title) {
return Base64.encodeToString(title.getBytes(), Base64.NO_WRAP);
- }
-
- private void spiltLargeList(SQLiteDatabase db, ReadingList readingList,
List<ReadingList> lists) {
- if (!userLargeListsMessageShown) {
- WikipediaApp.getInstance().getBus().post(new
SplitLargeListsEvent());
- userLargeListsMessageShown = true;
- }
- int index = readingList.title().split("_").length == 1 ? 1 :
Integer.valueOf(readingList.title().split("_")[1]) + 1;
- List<ReadingListPage> toBeAddedList = new ArrayList<>();
- if (readingList.pages().size() <= MAX_ARTICLE_LIMIT) {
- return;
- }
- for (int j = MAX_ARTICLE_LIMIT; j < readingList.pages().size(); j++) {
- toBeAddedList.add(readingList.pages().get(j));
- }
- readingList.pages().removeAll(toBeAddedList);
- ReadingList newList = ReadingListDbHelper.instance().createList(db,
String.format(WikipediaApp.getInstance().getString(R.string.split_list_name),
readingList.title().split("_")[0], index), readingList.description());
- newList.pages().addAll(toBeAddedList);
- lists.add(newList);
- spiltLargeList(db, newList, lists);
}
}
diff --git a/app/src/main/res/values-qq/strings.xml
b/app/src/main/res/values-qq/strings.xml
index dfdb9a5..418283e 100644
--- a/app/src/main/res/values-qq/strings.xml
+++ b/app/src/main/res/values-qq/strings.xml
@@ -367,7 +367,6 @@
<string name="no_user_lists_title">Suggestion string for users to Organize
articles into lists</string>
<string name="no_user_lists_msg">Text explaining the advantages of creating
reading lists, to users</string>
<string name="reading_list_saved_list_rename">\"%1$s\" _[string indicating
that the list was created by user]</string>
- <string name="split_list_name">Name string for the newly created lists from
the original large list that will be split. \"%1$s\" is the name of the
original list followed by _ and \"%2$d\" being the postfix number</string>
<string name="split_reading_list_message">Message shown to users with large
reading lists, informing them that this update will enforce an upper limit of
1000 articles per list and hence will be splitting already existing lists that
contain more than 1000 articles</string>
<plurals name="reading_list_article_offline_message">
<item quantity="one">Message shown when a single reading list article is
made available for offline use.</item>
diff --git a/app/src/main/res/values/strings.xml
b/app/src/main/res/values/strings.xml
index 8e5c3a4..9189ee5 100644
--- a/app/src/main/res/values/strings.xml
+++ b/app/src/main/res/values/strings.xml
@@ -392,8 +392,7 @@
<string name="no_user_lists_title">Organize articles into lists</string>
<string name="no_user_lists_msg">Create lists for places to travel to,
favorite topics, and much more</string>
<string name="reading_list_saved_list_rename">%1$s (user created)</string>
- <string name="split_list_name">%1$s_%2$d</string>
- <string name="split_reading_list_message">There is a limit of 1000
articles per synced reading list. Existing lists with more than this limit have
been split into multiple lists.</string>
+ <string name="split_reading_list_message">There is a limit of 1000
articles per reading list. Existing lists with more than this limit have been
split into multiple lists.</string>
<plurals name="reading_list_article_offline_message">
<item quantity="one">This article will now be available offline.</item>
<item quantity="other">These articles will now be available
offline.</item>
--
To view, visit https://gerrit.wikimedia.org/r/403764
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I1ded79c3f74273526bfa43b188736518f7850ea3
Gerrit-PatchSet: 1
Gerrit-Project: apps/android/wikipedia
Gerrit-Branch: master
Gerrit-Owner: Dbrant <[email protected]>
Gerrit-Reviewer: Brion VIBBER <[email protected]>
Gerrit-Reviewer: Cooltey <[email protected]>
Gerrit-Reviewer: Sharvaniharan <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
