Export Transactions: previously imported transactions are exported forever

[alceurneto]

How to reproduce

1. Import a XML file with transactions (I attached a zip file with sample files);
2. Enter some additional transaction;
3. Select Export (pay attention to suggested time, you may have the issue Export Transactions: wrong time suggestion in first export #464);
4. The generated QIF file will have all transactions even the one previously imported.

If we enter another transaction in GCA and export it again the generated QIF file will have the same previously imported transactions plus the new one.

P.S: We cannot import the QIF file in GnuCash desktop as stated at #466

Additional information

• I've installed GCA 2.0.5 on my Nexus 5 running Android 6.0.1

Sample files
sample-files.zip

[codinguser]

@rivaldi8 this sounds similar to some report we got a while back right?
Could you verify this report? Thanks

[rivaldi8]

I've been trying, but I haven't been able to reproduce. I've tested both with the attached example file and with my own files. It never exports the imported transactions, only the newly entered.

I'm not sure what might be provoking this. Have you changed any setting?

In my case I've done the tests from a bq Aquaris E4 (Android 5.0).

[codinguser]

@rivaldi8 I wonder too, because I can't reproduce it. But a small subset of users seem to be having the issue. I haven't changed any settings with this feature.

Either there is a disconnect between what the users expect and what the feature actually does, or there is a very subtle bug that we will hopefully find one day.

[alceurneto]

Hi guys!

I think we are facing another time zone issue.

My time zone is GMT -02:00 Brasilia Summer Time and the issue can be reproduced using the sample files.

I changed my device to a non negative time zone (I used GMT +00:00 Greenwich Mean Time) and like you the problem could NOT be spotted.

[codinguser]

@alceurneto thanks for that additional info. That helps in fixing the problem

[alceurneto]

Hi guys,

I've digged a little the source code in hope of better understanding of this issue.

I've noticed the trigger responsible for updates on database column modified_at:

  static String createUpdatedAtTrigger(String tableName){
      return "CREATE TRIGGER update_time_trigger "
              + "  AFTER UPDATE ON " + tableName + " FOR EACH ROW"
              + "  BEGIN " + "UPDATE " + tableName
              + "  SET " + CommonColumns.COLUMN_MODIFIED_AT + " = CURRENT_TIMESTAMP"
              + "  WHERE OLD." + CommonColumns.COLUMN_UID + " = NEW." + CommonColumns.COLUMN_UID + ";"
              + "  END;";
  }

and the DEFAULT CURRENT TIMESTAMP for inserts.

On a device with timezone GMT -02:00 Brasilia Summer Time I have seen that:

• Imported transactions have modified_at with the value of 2 hours AFTER local time;
• New transactions have modified_at with correct local time values;

I've struggled a lot to spot the difference between modified_at for imported and new transactions, but they seen to have analogous paths.

Of course the basecode is all new to me and I must be missing something.

P.S.1: I've found this StackOverflow topic about the use of CURRENT_TIMESTAMP x local time (http://stackoverflow.com/questions/381371/sqlite-current-timestamp-is-in-gmt-not-the-timezone-of-the-machine). I've even changed the trigger code to use datetime(CURRENT_TIMESTAMP, 'localtime'), but without success.

P.S.2: Because the mobile device is subject of sudden (and automated) change of timezone, I'm favour of attributes like modfied_at and lastExportTimeStamp having values without dependency on timezones. When showing these values to the end user we could transform them to set current device's timezone.

[alceurneto]

Hi guys,

I think I've understood the issue.

As soon as I've realized that our TIMESTAMP column is a standard TEXT column by afinity (https://www.sqlite.org/datatype3.html) the things started to make sense.

When importing transactions the modified_at column doesn't receive a value and the DEFAULT CURRENT TIMESTAMP stores a timestamp string in UTC time. The use of UTC is correct because we need values without dependency on timezones.

The issue arises when we insert a new transaction in the mobile app. For example, TransactionsDbAdapter.java has this method:

    @Override
    protected @NonNull SQLiteStatement setBindings(@NonNull SQLiteStatement stmt, @NonNull Transaction transaction) {
        stmt.clearBindings();
        // ...
        stmt.bindString(7, transaction.getCreatedTimestamp().toString());
        // ...
        return stmt;
    }

Timestamp.toString() returns a string based on the current user timezone (not UTC most of times) giving us a timezone issue at created_at column. At the link https://docs.oracle.com/javase/7/docs/api/java/sql/Timestamp.html we could see Timestamp is wrapper around Date and http://stackoverflow.com/questions/4199217/what-time-zone-does-date-tostring-display discusses Date.toString() and timezones.

The same occurs for modified_at. For example SplitsDbAdapter.java has this method:

    public void addRecord(@NonNull final Split split, UpdateMethod updateMethod){
        // ...
        //modifying a split means modifying the accompanying transaction as well
        updateRecord(TransactionEntry.TABLE_NAME, transactionId,
                TransactionEntry.COLUMN_MODIFIED_AT, new Timestamp(System.currentTimeMillis()).toString());
    }

Our last export time has analogous issue. For example, QifExporter.java by the end of a successful export operation stores the value this way:

    @Override
    public List<String> generateExport() throws ExporterException {
        // ...
            /// export successful
            String timeStamp = new Timestamp(System.currentTimeMillis()).toString();
            PreferenceManager.getDefaultSharedPreferences(mContext).edit().putString(Exporter.PREF_LAST_EXPORT_TIME, timeStamp).apply();            
        // ...
    }

We need evaluate if there is the same issue with other model entities and exporters.

I'm capable to make a try and work to fix this problem but I need some guidance:

1. Do you think my analysis makes sense or I am missing something?
2. Is there any rules I must know before applying a PR to codinguser/gnucash-android repository?

[codinguser]

@alceurneto I think your analysis makes A LOT of sense. Thanks!

One way I would suggest to fix this, is to work with created_at and modified_at timestamps in UTC completely by using a DateFormat.format() instead of Timestamp.toString(). The timezone of the DateFormat object can be set to SimpleTimeZone.UTC_TIME. When applying the fixes, also stop by DatabaseAdapter.populateBaseModelAttributes(Cursor, BaseModel), which affects all db adapters.

I should really get around to writing more in the wiki about contributing. But basically, contributions should go into the develop branch and also add some (Robolectric) unit tests where applicable.

[alceurneto]

@codinguser Thanks!

I'll work on it ASAP.

[codinguser]

One thing to think about would be the modified_at timestamp of existing transactions in the database though. When importing or creating new transactions, it is set by CURRENT_TIMESTAMP which is already in UTC. However, whenever we have set it by updating some record, we have done so using Timestamp.toString() which is local time. We cannot now differentiate those two. What compromise to make. Or is the difference small enough that there will be no effect on exports? Any ideas?

I have to think about it a little more.

[alceurneto]

That's correct! We need to think about the transition from the old (some transactions in local time) and the new (all transactions in UTC) regarding existing transactions in database.

[alceurneto]

I've started working to fix this issue.

[codinguser]

@alceurneto 👍

[alceurneto]

What compromise to make. Or is the difference small enough that there will be no effect on exports? Any ideas?

Hi guys,

Some thougts here:

If we don't update created_at, modified_at and last_export_time I think we'll see the following outlines:

For users with time zone equals UTC - N

On this picture we can see a time line in UTC. At the 'now' moment, or UTC + 0, occurs a GnuCash Android (GCA) update fixing the current issue without updating our fields. It's pictured in yellow.

When we have an import (export is analogous) occurred before the version update, the modified_at of imported transactions are equal the import time occurrence and our control variable last_export_time is N hours before them. They are all pictured in red.

The same occurs for transactions added or edited in the old version. The modified_at is located before the real event. They are are pictured in blue.

Added or edited data in the new version has equal time occurrence and modified_at. We can see it pictured in green.

This states that, despite the issue has been fixed, the first export operation will have the same wrong result that we see nowadays.

After this first attempt (or after an import) I believe the exports will have correct results.

For users with time zone equals UTC + 0

They fortunately have no problems.

For users with time zone equals UTC + N

As in previous case we can see the time occurrence for GCA update, but now last_export_time is positioned after an import operation (export is analogous). They are in the future and pictured in red again.

In the future are too modified_at for added or edited transactions in the old version (pictured in blue).

That is why users without negative time zones have not noticed the issue.

On the other hand, after the update, all transactions added or edited in the new version will be NOT eligible for export IF they are entered at least N hours after the last import (if they are entered (N+m) hours after last import time, they will be eligible).

Differences between string format of CURRENT_TIMESTAMP and Timestamp.toString()

CURRENT_TIMESTAMP has string format equal to YYYY-MM-DD HH:MM:SS (https://www.sqlite.org/lang_createtable.html).

Timestamp.toString() has string format equal to yyyy-mm-dd hh:mm:ss.fffffffff (https://docs.oracle.com/javase/7/docs/api/java/sql/Timestamp.html#toString%28%29)

I've downloaded the database used for tests and could see the differences:

So a routine to convert the values in the database is feasible, but raises some interesting questions:

• What the routine will do if a strange format is found?
• Will the routine create some form of log?
• Will the routine backup the user data before the conversion (and in case of exceptions restore the data)?
• The entire solution will be more complex and will demand more time?