Skip to content

Commit

Permalink
Improve time formatting in transactions list
Browse files Browse the repository at this point in the history
Fix random crashes when saving transactions
Closes #431
  • Loading branch information
codinguser committed Dec 1, 2015
1 parent 13ef64a commit 93cc38d
Show file tree
Hide file tree
Showing 5 changed files with 45 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -794,9 +794,13 @@ private Money computeBalance(String accountUID, long startTimestamp, long endTim
* @param endTimestamp the end timestamp of the time range
* @return Money balance of account list
*/
public Money getAccountsBalance(List<String> accountUIDList, long startTimestamp, long endTimestamp) {
public Money getAccountsBalance(@NonNull List<String> accountUIDList, long startTimestamp, long endTimestamp) {
String currencyCode = GnuCashApplication.getDefaultCurrencyCode();
Money balance = Money.createZeroInstance(currencyCode);

if (accountUIDList.isEmpty())
return balance;

boolean hasDebitNormalBalance = getAccountType(accountUIDList.get(0)).hasDebitNormalBalance();

SplitsDbAdapter splitsDbAdapter = SplitsDbAdapter.getInstance();
Expand Down
4 changes: 2 additions & 2 deletions app/src/main/java/org/gnucash/android/export/Exporter.java
Original file line number Diff line number Diff line change
Expand Up @@ -201,11 +201,11 @@ public String getExportMimeType(){
public static class ExporterException extends RuntimeException{

public ExporterException(ExportParams params){
super("Failed to generate " + params.getExportFormat().toString());
super("Failed to generate export with parameters: " + params.toString());
}

public ExporterException(@NonNull ExportParams params, @NonNull String msg) {
super("Failed to generate " + params.getExportFormat().toString() + "-" + msg);
super("Failed to generate export with parameters: " + params.toString() + " - " + msg);
}

public ExporterException(ExportParams params, Throwable throwable){
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -785,8 +785,8 @@ private void saveNewTransaction() {
mTransaction.addSplit(split);

String transferAcctUID;
if (mUseDoubleEntry) {
long transferAcctId = mTransferAccountSpinner.getSelectedItemId();
long transferAcctId = mTransferAccountSpinner.getSelectedItemId();
if (mUseDoubleEntry || transferAcctId < 0) {

This comment has been minimized.

Copy link
@rivaldi8

rivaldi8 Jan 28, 2016

Collaborator

Hi @codinguser! I've arrived here from this crash report. It seems lots crashes go through this code path.

I was trying to understand the condition under which the application crashes, but I got confused with the change in the conditional. Could you explain it? For me, it would make more sense something like this:

if (mUseDoubleEntry && transferAcctId != Spinner.INVALID_ROW_ID) {

Even so, I still doubt about this. getSelectedItemId() should only return INVALID_ROW_ID if nothing has been selected in the spinner. However, if it has items, I don't see any way to arrive at this condition. I'd say it could only happen in the case the spinner was empty but, again, I'm not sure it's possible with a sensible accounts structure.

What do you think?

This comment has been minimized.

Copy link
@codinguser

codinguser via email Jan 29, 2016

Author Owner

This comment has been minimized.

Copy link
@codinguser

codinguser Feb 5, 2016

Author Owner

@rivaldi8 actually, looking at that code again, the fix is much simpler. It should not be if (mUseDoubleEntry || transferAcctId < 0) but rather if (mUseDoubleEntry && transferAcctId > 0). We only consider transfer account when in double entry mode and a valid account is selected. I just tested it and it fixes the problem. Will upload commits soon.

This comment has been minimized.

Copy link
@rivaldi8

rivaldi8 Feb 5, 2016

Collaborator

Are you sure you want to execute the else branch when mUseDoubleEntry is true and transferAcctId is Spinner.INVALID_ROW_ID? It looks like it's meant for the mUseDoubleEntry == false case. If so, wouldn't it be better to throw an exception to avoid continuing?

This comment has been minimized.

Copy link
@codinguser

codinguser via email Feb 5, 2016

Author Owner

This comment has been minimized.

Copy link
@codinguser

codinguser via email Feb 8, 2016

Author Owner

This comment has been minimized.

Copy link
@rivaldi8

rivaldi8 Feb 8, 2016

Collaborator

Yes, I think that's good. We avoid the user having to create the account by hand. I wasn't sure that was what you wanted, so I asked just in case :)

transferAcctUID = mAccountsDbAdapter.getUID(transferAcctId);
} else {
transferAcctUID = mAccountsDbAdapter.getOrCreateImbalanceAccountUID(currency);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import android.os.AsyncTask;
import android.os.Build;
import android.os.Bundle;
import android.support.annotation.NonNull;
import android.support.design.widget.FloatingActionButton;
import android.support.design.widget.TabLayout;
import android.support.v4.app.Fragment;
Expand All @@ -32,6 +33,7 @@
import android.support.v4.view.PagerAdapter;
import android.support.v4.view.ViewPager;
import android.support.v7.widget.Toolbar;
import android.text.format.DateUtils;
import android.util.Log;
import android.util.SparseArray;
import android.view.Menu;
Expand Down Expand Up @@ -60,8 +62,12 @@
import org.gnucash.android.ui.util.OnTransactionClickedListener;
import org.gnucash.android.ui.util.Refreshable;
import org.gnucash.android.util.QualifiedAccountNameCursorAdapter;
import org.joda.time.LocalDate;

import java.math.BigDecimal;
import java.text.DateFormat;
import java.text.SimpleDateFormat;
import java.util.Date;

import butterknife.Bind;
import butterknife.ButterKnife;
Expand Down Expand Up @@ -92,6 +98,7 @@ public class TransactionsActivity extends BaseDrawerActivity implements
* Number of pages to show
*/
private static final int DEFAULT_NUM_PAGES = 2;
private static SimpleDateFormat mDayMonthDateFormat = new SimpleDateFormat("EEE, d MMM");

/**
* GUID of {@link Account} whose transactions are displayed
Expand Down Expand Up @@ -153,7 +160,6 @@ public void onNothingSelected(AdapterView<?> parent) {
private PagerAdapter mPagerAdapter;



/**
* Adapter for managing the sub-account and transaction fragment pages in the accounts view
*/
Expand Down Expand Up @@ -486,6 +492,29 @@ public static void displayBalance(TextView balanceTextView, Money balance){
balanceTextView.setTextColor(fontColor);
}

/**
* Formats the date to show the the day of the week if the {@code dateMillis} is within 7 days
* of today. Else it shows the actual date formatted as short string. <br>
* It also shows "today", "yesterday" or "tomorrow" if the date is on any of those days
* @param dateMillis
* @return
*/
@NonNull
public static String getPrettyDateFormat(Context context, long dateMillis) {
LocalDate transactionTime = new LocalDate(dateMillis);
LocalDate today = new LocalDate();
String prettyDateText = null;
if (transactionTime.compareTo(today.minusDays(1)) >= 0 && transactionTime.compareTo(today.plusDays(1)) <= 0){
prettyDateText = DateUtils.getRelativeTimeSpanString(dateMillis, System.currentTimeMillis(), DateUtils.DAY_IN_MILLIS).toString();
} else if (transactionTime.getYear() == today.getYear()){
prettyDateText = mDayMonthDateFormat.format(new Date(dateMillis));
} else {
prettyDateText = DateUtils.formatDateTime(context, dateMillis, DateUtils.FORMAT_ABBREV_MONTH | DateUtils.FORMAT_SHOW_YEAR);
}

return prettyDateText;
}

@Override
public void createNewTransaction(String accountUID) {
Intent createTransactionIntent = new Intent(this.getApplicationContext(), FormActivity.class);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import android.content.res.Configuration;
import android.database.Cursor;
import android.os.Bundle;
import android.support.annotation.NonNull;
import android.support.v4.app.Fragment;
import android.support.v4.app.LoaderManager.LoaderCallbacks;
import android.support.v4.content.Loader;
Expand All @@ -42,6 +43,7 @@
import android.widget.TextView;

import org.gnucash.android.R;
import org.gnucash.android.app.GnuCashApplication;
import org.gnucash.android.db.AccountsDbAdapter;
import org.gnucash.android.db.DatabaseCursorLoader;
import org.gnucash.android.db.DatabaseSchema;
Expand All @@ -57,8 +59,10 @@
import org.gnucash.android.ui.util.CursorRecyclerAdapter;
import org.gnucash.android.ui.util.Refreshable;
import org.gnucash.android.ui.util.widget.EmptyRecyclerView;
import org.joda.time.LocalDate;

import java.text.DateFormat;
import java.text.SimpleDateFormat;
import java.util.Date;
import java.util.List;

Expand Down Expand Up @@ -196,7 +200,6 @@ public void onLoaderReset(Loader<Cursor> loader) {
mTransactionRecyclerAdapter.swapCursor(null);
}


/**
* {@link DatabaseCursorLoader} for loading transactions asynchronously from the database
* @author Ngewi Fet <ngewif@gmail.com>
Expand All @@ -221,8 +224,6 @@ public Cursor loadInBackground() {

public class TransactionRecyclerAdapter extends CursorRecyclerAdapter<TransactionRecyclerAdapter.ViewHolder>{

DateFormat simpleDateFormat = DateFormat.getDateInstance(DateFormat.MEDIUM);

public TransactionRecyclerAdapter(Cursor cursor) {
super(cursor);
}
Expand Down Expand Up @@ -264,7 +265,8 @@ public void onBindViewHolderCursor(ViewHolder holder, Cursor cursor) {
holder.transactionNote.setText(text);

long dateMillis = cursor.getLong(cursor.getColumnIndexOrThrow(DatabaseSchema.TransactionEntry.COLUMN_TIMESTAMP));
String dateText = DateUtils.getRelativeTimeSpanString(dateMillis, System.currentTimeMillis(), DateUtils.DAY_IN_MILLIS).toString();
String dateText = TransactionsActivity.getPrettyDateFormat(getActivity(), dateMillis);

holder.transactionDate.setText(dateText);

final long id = holder.transactionId;
Expand All @@ -288,7 +290,6 @@ public void onClick(View v) {

}


public class ViewHolder extends RecyclerView.ViewHolder implements PopupMenu.OnMenuItemClickListener{
@Bind(R.id.primary_text) public TextView transactionDescription;
@Bind(R.id.secondary_text) public TextView transactionNote;
Expand Down

0 comments on commit 93cc38d

Please sign in to comment.