Skip to content

Add a check for new entries in the changes table to ensure swift syncing of non-Dexie observable changes.#3591

Merged
bjester merged 2 commits intolearningequality:unstablefrom
rtibbles:ch_ch_ch_ch_changes
Aug 31, 2022
Merged

Add a check for new entries in the changes table to ensure swift syncing of non-Dexie observable changes.#3591
bjester merged 2 commits intolearningequality:unstablefrom
rtibbles:ch_ch_ch_ch_changes

Conversation

@rtibbles
Copy link
Copy Markdown
Member

Summary

Description of the change(s) you made

  • Currently our handleChanges listener only triggers a sync when changes to our IndexedDBResource tables are made that are CREATE, UPDATE, or DELETE (the change types that Dexie Observable natively creates).
  • This means that our custom change operations: MOVE, COPY, PUBLISH, and SYNC, will be ignored, until the sync endpoint polling happens.
  • To remedy this, this PR implements an extra check in the handleChanges listener function to look for newly created changes in our specific changes table, so that we can trigger a sync when one of our custom changes has been created.

Manual verification steps performed

Carried out copy and move operations and confirmed that they were included in an almost immediate sync call to the backend.

Does this introduce any tech-debt items?

I don't think so - ultimately, I think we could drop Dexie Observable and use something closer to the Dexie middleware that it is built on top of to make a more unified solution.

References

Fixes #3586

// MOVE, COPY, PUBLISH, and SYNC changes where we may be executing them inside an IGNORED_SOURCE
// because they also make UPDATE and CREATE changes that we wish to make in the client only.
const newChangeTableEntries = changes.some(
c => c.table === CHANGES_TABLE && c.type === CHANGE_TYPES.CREATED
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

So for @ozer550's websocket implementation, he should also check that synced on the underlying object is also false before sending a change to the backend? Unless I'm misunderstanding, changes coming from the backend would also cause this Dexie create change?

Copy link
Copy Markdown
Member

@bjester bjester Aug 31, 2022

Choose a reason for hiding this comment

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

Actually, we don't save the incoming changes to IndexedDB. But we would apply them of course, so I'm just wondering how we ignore those changes that occur from incoming changes

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I forgot about changes.filter(isSyncableChange); which would prevent the generation of changes from incoming events being applied.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I think still worthwhile to add the synced check here, to make sure we don't try to sync synced changes.

@bjester bjester merged commit aba1e0e into learningequality:unstable Aug 31, 2022
@rtibbles rtibbles deleted the ch_ch_ch_ch_changes branch August 31, 2022 17:42
@bjester bjester mentioned this pull request Sep 2, 2022
24 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Copy, move, and publishing operations do not trigger an immediate sync attempt

2 participants