Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unable to save as complete if _savepoint_timestamp is in the future #64

Closed
linl33 opened this issue Mar 15, 2018 · 5 comments
Closed

Unable to save as complete if _savepoint_timestamp is in the future #64

linl33 opened this issue Mar 15, 2018 · 5 comments
Labels
bug Something isn't working Services ODK-X Services Survey ODK-X Survey Tables ODK-X Tables

Comments

@linl33
Copy link
Member

linl33 commented Mar 15, 2018

Software and hardware versions

Services 2.0.3
Survey 2.0.3
Tables 2.0.3

Problem description

When editing a row in Survey, if the row's _savepoint_timestamp is greater than the latest checkpoint's _savepoint_timestamp the savepoint doesn't get saved as complete.

Steps to reproduce the problem

  1. Write a CSV with a row such that the _savepoint_timestamp column contains a timestamp in the future
  2. Import the CSV with Tables
  3. Edit the row with Survey

Expected behavior

Changes get persisted.

Other information

The problem is in ODKDatabaseImplUtils.java#L3952-L3957

b.setLength(0);
b
  .append(K_DATATABLE_ID_EQUALS_PARAM)
  .append(S_AND)
  .append(DataTableColumns.SAVEPOINT_TIMESTAMP)
  .append(" NOT IN (SELECT MAX(")
  .append(DataTableColumns.SAVEPOINT_TIMESTAMP)
  .append(") FROM ")
  .append(tableId)
  .append(K_WHERE)
  .append(K_DATATABLE_ID_EQUALS_PARAM)
  .append(")");
db.delete(tableId, b.toString(), new Object[] { rowId, rowId });

This translates to

_id = ? AND _savepoint_timestamp NOT IN (SELECT MAX(_savepoint_timestamp) FROM tableId WHERE _id = ?

It keeps the row with the newest _savepoint_timestamp among all rows with the same id, but it should distinguish between checkpoint rows and non-checkpoint rows.

@wbrunette
Copy link
Member

I think we should try to get a better understanding of the use case that you would have a row with a savepoint timestamp in the future. Caroline?

@wbrunette
Copy link
Member

The savepoint timestamp normally refers to when the last time the row was updated so confused why we would have a future time

@linl33
Copy link
Member Author

linl33 commented Mar 15, 2018

This is one of the problems Caroline reported. In her case, she had a typo in the CSV so the timestamp was in the future.

But this could also happen if the timezone changes. For example, a user could pre-populate the timestamp field using the local timezone but Services calculates timezone using UTC.

@wbrunette
Copy link
Member

that is the point of UTC is to convert from the local time to a global time. I am inclined to say that it should be problematic to have future times in savepointtimestamp

@wbrunette wbrunette transferred this issue from getodk/getodk Apr 9, 2019
@wbrunette wbrunette added Services ODK-X Services Survey ODK-X Survey Tables ODK-X Tables bug Something isn't working labels Jul 29, 2019
@clarlars
Copy link
Collaborator

Fixed in odk-x/androidlibrary#130

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Services ODK-X Services Survey ODK-X Survey Tables ODK-X Tables
Projects
None yet
Development

No branches or pull requests

3 participants