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

Adds multiple accounts support #90

Closed
wants to merge 1 commit into from
Closed

Adds multiple accounts support #90

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
7 changes: 7 additions & 0 deletions app/src/main/AndroidManifest.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,13 @@
android:theme="@style/DankTheme.DialogLikeActivity"
android:windowSoftInputMode="adjustNothing" />

<activity
android:name=".ui.accountmanager.AccountManagerActivity"
android:launchMode="singleTask"
android:theme="@style/DankTheme.DialogLikeActivity"
android:windowSoftInputMode="adjustNothing" />


<activity
android:name=".deeplinks.DeepLinkHandlingActivity"
android:theme="@android:style/Theme.NoDisplay">
Expand Down
9 changes: 8 additions & 1 deletion app/src/main/java/me/saket/dank/data/DankSqliteOpenHelper.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import android.database.sqlite.SQLiteDatabase
import android.database.sqlite.SQLiteOpenHelper

import me.saket.dank.reply.PendingSyncReply
import me.saket.dank.ui.accountmanager.AccountManager
import me.saket.dank.ui.appshortcuts.AppShortcut
import me.saket.dank.ui.subscriptions.SubredditSubscription
import me.saket.dank.ui.user.messages.CachedMessage
Expand All @@ -17,6 +18,7 @@ class DankSqliteOpenHelper(context: Context) : SQLiteOpenHelper(context, DB_NAME
db.execSQL(CachedMessage.QUERY_CREATE_TABLE)
db.execSQL(PendingSyncReply.QUERY_CREATE_TABLE)
db.execSQL(AppShortcut.QUERY_CREATE_TABLE)
db.execSQL(AccountManager.QUERY_CREATE_TABLE)
}

override fun onUpgrade(db: SQLiteDatabase, oldVersion: Int, newVersion: Int) {
Copy link
Owner

Choose a reason for hiding this comment

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

The upgrade method will need some fixing. I can see that you followed the convention in the first if statement but it won't work correctly.

Let's say that the user is on the version of app where the database has version 1. Now when they update to latest version this method will be called with parameters oldVersion=1 and newVersion=3. In that case by following the logic inside of if statements we can see that none of them will be executed and the upgrade won't work correctly.

In the first statement you want to keep on check on oldVersion only. That is, we should simply delete cached messages table when upgrading from that version.

In the second statement you want to check only on newVersion since you need to create the table also when upgrading from older versions. Additionally I'm not sure but perhaps we should go a bit further and check whether new version is equal to or greater than 3 to also take into account future versions.

Other than that it would be a good idea to add tests for the whole upgrade since that is an important operation. Of course tests for other operations on the database would be nice too but I feel like this one is the minimum.

Expand All @@ -26,11 +28,16 @@ class DankSqliteOpenHelper(context: Context) : SQLiteOpenHelper(context, DB_NAME
Timber.d("Resetting cached-message rows")
// JRAW was bumped to v1.0.
db.execSQL("DELETE FROM ${CachedMessage.TABLE_NAME}")
db.execSQL(AccountManager.QUERY_CREATE_TABLE)
}

if (oldVersion == 2 && newVersion == 3) {
db.execSQL(AccountManager.QUERY_CREATE_TABLE)
}
}

companion object {
private const val DB_VERSION = 2
private const val DB_VERSION = 3
private const val DB_NAME = "Dank"
Tunous marked this conversation as resolved.
Show resolved Hide resolved
}
}
10 changes: 10 additions & 0 deletions app/src/main/java/me/saket/dank/data/InboxRepository.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,16 @@ private Completable removeAllMessages(InboxFolder folder) {
});
}

@CheckResult
public Completable clearMessages() {
return Completable.fromAction(() -> {
try (BriteDatabase.Transaction transaction = briteDatabase.newTransaction()) {
briteDatabase.delete(CachedMessage.TABLE_NAME, "");
Copy link
Owner

Choose a reason for hiding this comment

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

Based on documentation passing null as whereClause parameter will delete all rows. I would suggest to do that instead of passing empty string.

Suggested change
briteDatabase.delete(CachedMessage.TABLE_NAME, "");
briteDatabase.delete(CachedMessage.TABLE_NAME, null);

Copy link
Author

Choose a reason for hiding this comment

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

Ah OK. Anyways I wasn't 100% sure about deleting the cache table but switching to other account would end in merging PM from all your accounts. It's seems to be more a hack than a fix to me.

transaction.markSuccessful();
}
});
}

// ======== READ STATUS ======== //

/**
Expand Down
3 changes: 3 additions & 0 deletions app/src/main/java/me/saket/dank/di/RootComponent.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import me.saket.dank.reddit.RedditModule;
import me.saket.dank.reply.RetryReplyJobService;
import me.saket.dank.ui.PlaygroundActivity;
import me.saket.dank.ui.accountmanager.AccountManagerActivity;
import me.saket.dank.ui.appshortcuts.AppShortcutRepository;
import me.saket.dank.ui.appshortcuts.ConfigureAppShortcutsActivity;
import me.saket.dank.ui.authentication.LoginActivity;
Expand Down Expand Up @@ -162,4 +163,6 @@ public interface RootComponent {
void inject(SubmissionSwipeActionPreferenceChoicePopup target);

void inject(IndentedLayout target);

void inject(AccountManagerActivity target);
}
5 changes: 3 additions & 2 deletions app/src/main/java/me/saket/dank/di/RootModule.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import me.saket.dank.data.OnLoginRequireListener;
import me.saket.dank.reply.ReplyRepository;
import me.saket.dank.ui.UrlRouter;
import me.saket.dank.ui.accountmanager.AccountManagerActivity;
import me.saket.dank.ui.authentication.LoginActivity;
import me.saket.dank.ui.submission.DraftStore;
import me.saket.dank.ui.submission.LinkOptionsPopup;
Expand Down Expand Up @@ -184,8 +185,8 @@ DraftStore provideReplyDraftStore(ReplyRepository replyRepository) {
@Provides
OnLoginRequireListener provideOnLoginRequireListener(Application appContext) {
return () -> {
Intent loginIntent = LoginActivity.intent(appContext);
loginIntent.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK);
Intent loginIntent = new Intent(appContext, AccountManagerActivity.class);
Copy link
Owner

Choose a reason for hiding this comment

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

AccountManagerActivity has intent function which you could use.

loginIntent.setFlags(Intent.FLAG_ACTIVITY_NEW_TASK);
appContext.startActivity(loginIntent);
};
}
Expand Down
8 changes: 5 additions & 3 deletions app/src/main/java/me/saket/dank/notifs/CheckUnreadMessagesJobService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import me.saket.dank.data.ResolvedError;
import me.saket.dank.di.Dank;
import me.saket.dank.ui.preferences.NetworkStrategy;
import me.saket.dank.ui.user.UserSessionRepository;
import me.saket.dank.ui.user.messages.InboxFolder;
import me.saket.dank.utils.Arrays2;
import me.saket.dank.utils.PersistableBundleUtils;
Expand All @@ -44,6 +45,7 @@ public class CheckUnreadMessagesJobService extends DankJobService {
@Inject InboxRepository inboxRepository;
@Inject ErrorResolver errorResolver;
@Inject MessagesNotificationManager messagesNotifManager;
@Inject UserSessionRepository userSessionRepository;

/**
* Schedules two recurring sync jobs:
Expand Down Expand Up @@ -132,7 +134,7 @@ public void onCreate() {

@Override
public JobStartCallback onStartJob2(JobParameters params) {
displayDebugNotification("Checking for unread messages");
displayDebugNotification("Checking for unread messages for " + userSessionRepository.loggedInUserName());

//Timber.i("Fetching unread messages. JobID: %s", params.getJobId());
boolean shouldRefreshMessages = PersistableBundleUtils.getBoolean(params.getExtras(), KEY_REFRESH_MESSAGES);
Expand Down Expand Up @@ -181,7 +183,7 @@ public JobStartCallback onStartJob2(JobParameters params) {
error -> {
ResolvedError resolvedError = errorResolver.resolve(error);
if (resolvedError.isUnknown()) {
Timber.e(error, "Unknown error while fetching unread messages.");
Timber.e(error, "Unknown error while fetching unread messages for " + userSessionRepository.loggedInUserName());
}

boolean needsReschedule = resolvedError.isNetworkError() || resolvedError.isRedditServerError();
Expand All @@ -201,7 +203,7 @@ private Completable notifyUnreadMessages(List<Message> unreadMessages) {
return messagesNotifManager.filterUnseenMessages(unreadMessages)
.flatMapCompletable(unseenMessages -> {
if (unseenMessages.isEmpty()) {
displayDebugNotification("No unread messages found");
displayDebugNotification("No unread messages found for " + userSessionRepository.loggedInUserName());
return messagesNotifManager.dismissAllNotifications(getBaseContext());
} else {
removeDebugNotification();
Expand Down
68 changes: 68 additions & 0 deletions app/src/main/java/me/saket/dank/ui/accountmanager/AccountManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
package me.saket.dank.ui.accountmanager;

import android.content.ContentValues;
import android.database.Cursor;

import com.google.auto.value.AutoValue;
import io.reactivex.functions.Function;

import me.saket.dank.ui.accountmanager.AccountManagerScreenUiModel;
import me.saket.dank.ui.accountmanager.AutoValue_AccountManager;
import me.saket.dank.utils.Cursors;

@AutoValue
public abstract class AccountManager implements AccountManagerScreenUiModel {

static final String TABLE_NAME = "Account";
static final String COLUMN_USERNAME = "username";
Copy link
Owner

Choose a reason for hiding this comment

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

This column is not clear to me.

  1. The type in create table query is set to text
  2. Mapper maps it to int
  3. Mapper reads it as user
  4. Create method uses it as rank
  5. toValues method also uses it as rank
  6. WHERE_USERNAME doesn't use it but uses label column

You see the inconsistencies? I guess what you want here are columns named rank and username instead. You should use the same naming everywhere.

static final String COLUMN_LABEL = "label";

public static final String QUERY_CREATE_TABLE =
"CREATE TABLE " + TABLE_NAME + " ("
+ COLUMN_LABEL + " TEXT NOT NULL PRIMARY KEY, "
+ COLUMN_USERNAME + " TEXT NOT NULL)";

public static final String QUERY_GET_ALL_ORDERED_BY_USER =
"SELECT * FROM " + TABLE_NAME;

public static final String WHERE_USERNAME =
COLUMN_LABEL + " = ?";

public static AccountManager create(int rank, String label) {
return new AutoValue_AccountManager(rank, label);
}

public static AccountManager create(String username) {
return new AutoValue_AccountManager(1, username);
}

public static final Function<Cursor, AccountManager> MAPPER = cursor -> {
int user = Cursors.intt(cursor, COLUMN_USERNAME);
String label = Cursors.string(cursor, COLUMN_LABEL);
return create(user, label);
};

public abstract int rank();

public abstract String label();

public AccountManager withRank(int newRank) {
return create(newRank, label());
}

public String id() {
return label();
}

@Override
public long adapterId() {
return label().hashCode();
}

public ContentValues toValues() {
ContentValues values = new ContentValues(2);
values.put(COLUMN_USERNAME, rank());
values.put(COLUMN_LABEL, label());
return values;
}
}
Loading